few updates based on review

kubernetes · Dec 20, 2016 · 3419ae7 · 3419ae7
1 parent af9c658
commit 3419ae7
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 92 deletions.
diff --git a/pkg/kobject/kobject.go b/pkg/kobject/kobject.go
@@ -21,7 +21,9 @@ import "k8s.io/kubernetes/pkg/api"
 // KomposeObject holds the generic struct of Kompose transformation
 type KomposeObject struct {
 	ServiceConfigs map[string]ServiceConfig
-	// name of the loader that was created KomposeObject
+	// LoadedFrom is name of the loader that created KomposeObject
+	// Transformer need to know origin format in order to tell user what tag is not supported in origin format
+	// as they can have different names. For example environment variables  are called environment in compose but Env in bundle.
 	LoadedFrom string
 }
 

diff --git a/pkg/loader/bundle/bundle.go b/pkg/loader/bundle/bundle.go
@@ -66,10 +66,10 @@ type Port struct {
 func checkUnsupportedKey(bundleStruct *Bundlefile) []string {
 	// list of all unsupported keys for this loader
 	// this is map to make searching for keys easier
-	// also counts how many times was given key found in service
-	// to make sure that we show warning only once for every key
-	var unsupportedKey = map[string]int{
-		"Networks": 0,
+	// to make sure that unsupported key is not going to be reported twice
+	// by keeping record if already saw this key in another service
+	var unsupportedKey = map[string]bool{
+		"Networks": false,
 	}
 
 	// collect all keys found in project
@@ -80,25 +80,23 @@ func checkUnsupportedKey(bundleStruct *Bundlefile) []string {
 		s := structs.New(service)
 
 		for _, f := range s.Fields() {
-			if f.IsExported() && !f.IsZero() {
-				jsonTagName := strings.Split(f.Tag("json"), ",")[0]
-				if jsonTagName == "" {
-					jsonTagName = f.Name()
-				}
-
-				// IsZero returns false for empty array/slice ([])
-				// this check if field is Slice, and then it checks its size
-				if field := val.FieldByName(f.Name()); field.Kind() == reflect.Slice {
-					if field.Len() == 0 {
-						// array is empty it doesn't metter if it is in unsupportedKey or not
-						continue
+			// Check if given key is among unsupported keys, and skip it if we already saw this key
+			if alreadySaw, ok := unsupportedKey[f.Name()]; ok && !alreadySaw {
+				if f.IsExported() && !f.IsZero() {
+					jsonTagName := strings.Split(f.Tag("json"), ",")[0]
+					if jsonTagName == "" {
+						jsonTagName = f.Name()
 					}
-				}
-				if counter, ok := unsupportedKey[f.Name()]; ok {
-					if counter == 0 {
-						keysFound = append(keysFound, jsonTagName)
+					// IsZero returns false for empty array/slice ([])
+					// this check if field is Slice, and then it checks its size
+					if field := val.FieldByName(f.Name()); field.Kind() == reflect.Slice {
+						if field.Len() == 0 {
+							// array is empty it doesn't matter if it is in unsupportedKey or not
+							continue
+						}
 					}
-					unsupportedKey[f.Name()]++
+					keysFound = append(keysFound, jsonTagName)
+					unsupportedKey[f.Name()] = true
 				}
 			}
 		}

diff --git a/pkg/loader/compose/compose.go b/pkg/loader/compose/compose.go
@@ -45,39 +45,39 @@ func checkUnsupportedKey(composeProject *project.Project) []string {
 
 	// list of all unsupported keys for this loader
 	// this is map to make searching for keys easier
-	// also counts how many times was given key found in service
-	// to make sure that we show warning only once for every key
-	var unsupportedKey = map[string]int{
-		"CgroupParent":  0,
-		"Devices":       0,
-		"DependsOn":     0,
-		"DNS":           0,
-		"DNSSearch":     0,
-		"DomainName":    0,
-		"EnvFile":       0,
-		"Extends":       0,
-		"ExternalLinks": 0,
-		"ExtraHosts":    0,
-		"Hostname":      0,
-		"Ipc":           0,
-		"Logging":       0,
-		"MacAddress":    0,
-		"MemLimit":      0,
-		"MemSwapLimit":  0,
-		"NetworkMode":   0,
-		"Pid":           0,
-		"SecurityOpt":   0,
-		"ShmSize":       0,
-		"StopSignal":    0,
-		"VolumeDriver":  0,
-		"Uts":           0,
-		"ReadOnly":      0,
-		"StdinOpen":     0,
-		"Tty":           0,
-		"Ulimits":       0,
-		"Dockerfile":    0,
-		"Net":           0,
-		"Networks":      0, // there are special checks for Network in checkUnsupportedKey function996607
+	// to make sure that unsupported key is not going to be reported twice
+	// by keeping record if already saw this key in another service
+	var unsupportedKey = map[string]bool{
+		"CgroupParent":  false,
+		"Devices":       false,
+		"DependsOn":     false,
+		"DNS":           false,
+		"DNSSearch":     false,
+		"DomainName":    false,
+		"EnvFile":       false,
+		"Extends":       false,
+		"ExternalLinks": false,
+		"ExtraHosts":    false,
+		"Hostname":      false,
+		"Ipc":           false,
+		"Logging":       false,
+		"MacAddress":    false,
+		"MemLimit":      false,
+		"MemSwapLimit":  false,
+		"NetworkMode":   false,
+		"Pid":           false,
+		"SecurityOpt":   false,
+		"ShmSize":       false,
+		"StopSignal":    false,
+		"VolumeDriver":  false,
+		"Uts":           false,
+		"ReadOnly":      false,
+		"StdinOpen":     false,
+		"Tty":           false,
+		"Ulimits":       false,
+		"Dockerfile":    false,
+		"Net":           false,
+		"Networks":      false, // there are special checks for Network in checkUnsupportedKey function
 	}
 
 	// collect all keys found in project
@@ -98,17 +98,17 @@ func checkUnsupportedKey(composeProject *project.Project) []string {
 		s := structs.New(serviceConfig)
 
 		for _, f := range s.Fields() {
-			if f.IsExported() && !f.IsZero() {
-				// IsZero returns false for empty array/slice ([])
-				// this check if field is Slice, and then it checks its size
-				if field := val.FieldByName(f.Name()); field.Kind() == reflect.Slice {
-					if field.Len() == 0 {
-						// array is empty it doesn't metter if it is in unsupportedKey or not
-						continue
+			// Check if given key is among unsupported keys, and skip it if we already saw this key
+			if alreadySaw, ok := unsupportedKey[f.Name()]; ok && !alreadySaw {
+				if f.IsExported() && !f.IsZero() {
+					// IsZero returns false for empty array/slice ([])
+					// this check if field is Slice, and then it checks its size
+					if field := val.FieldByName(f.Name()); field.Kind() == reflect.Slice {
+						if field.Len() == 0 {
+							// array is empty it doesn't matter if it is in unsupportedKey or not
+							continue
+						}
 					}
-				}
-
-				if counter, ok := unsupportedKey[f.Name()]; ok {
 					//get yaml tag name instad of variable name
 					yamlTagName := strings.Split(f.Tag("yaml"), ",")[0]
 					if f.Name() == "Networks" {
@@ -120,10 +120,8 @@ func checkUnsupportedKey(composeProject *project.Project) []string {
 							yamlTagName = "networks"
 						}
 					}
-					if counter == 0 {
-						keysFound = append(keysFound, yamlTagName)
-					}
-					unsupportedKey[f.Name()]++
+					keysFound = append(keysFound, yamlTagName)
+					unsupportedKey[f.Name()] = true
 				}
 			}
 		}

diff --git a/pkg/loader/compose/compose_test.go b/pkg/loader/compose/compose_test.go
@@ -140,6 +140,21 @@ func TestUnsupportedKeys(t *testing.T) {
 			},
 		},
 	})
+	projectWithNetworks.ServiceConfigs.Add("bar", &config.ServiceConfig{
+		Image: "bar/foo",
+		Build: yaml.Build{
+			Context: "./build",
+		},
+		Hostname: "localhost",
+		Ports:    []string{}, // test empty array
+		Networks: &yaml.Networks{
+			Networks: []*yaml.Network{
+				&yaml.Network{
+					Name: "net1",
+				},
+			},
+		},
+	})
 	projectWithNetworks.VolumeConfigs = map[string]*config.VolumeConfig{
 		"foo": &config.VolumeConfig{
 			Driver: "storage",

diff --git a/pkg/transformer/kubernetes/kubernetes.go b/pkg/transformer/kubernetes/kubernetes.go
@@ -57,17 +57,17 @@ const TIMEOUT = 300
 // list of all unsupported keys for this transformer
 // Keys are names of variables in kobject struct.
 // this is map to make searching for keys easier
-// also counts how many times was given key found in kobject
-// to make sure that we show warning only once for every key
-var unsupportedKey = map[string]int{
-	"Build": 0,
+// to make sure that unsupported key is not going to be reported twice
+// by keeping record if already saw this key in another service
+var unsupportedKey = map[string]bool{
+	"Build": false,
 }
 
 // checkUnsupportedKey checks if given komposeObject contains
 // keys that are not supported by this tranfomer.
 // list of all unsupported keys are stored in unsupportedKey variable
 // returns list of TODO: ....
-func (k *Kubernetes) CheckUnsupportedKey(komposeObject *kobject.KomposeObject, unsupportedKey map[string]int) []string {
+func (k *Kubernetes) CheckUnsupportedKey(komposeObject *kobject.KomposeObject, unsupportedKey map[string]bool) []string {
 	// collect all keys found in project
 	var keysFound []string
 
@@ -77,22 +77,22 @@ func (k *Kubernetes) CheckUnsupportedKey(komposeObject *kobject.KomposeObject, u
 		s := structs.New(serviceConfig)
 
 		for _, f := range s.Fields() {
-			if f.IsExported() && !f.IsZero() {
-				// IsZero returns false for empty array/slice ([])
-				// this check if field is Slice, and then it checks its size
-				if field := val.FieldByName(f.Name()); field.Kind() == reflect.Slice {
-					if field.Len() == 0 {
-						// array is empty it doesn't metter if it is in unsupportedKey or not
-						continue
+			// Check if given key is among unsupported keys, and skip it if we already saw this key
+			if alreadySaw, ok := unsupportedKey[f.Name()]; ok && !alreadySaw {
+
+				if f.IsExported() && !f.IsZero() {
+					// IsZero returns false for empty array/slice ([])
+					// this check if field is Slice, and then it checks its size
+					if field := val.FieldByName(f.Name()); field.Kind() == reflect.Slice {
+						if field.Len() == 0 {
+							// array is empty it doesn't matter if it is in unsupportedKey or not
+							continue
+						}
 					}
-				}
-				if counter, ok := unsupportedKey[f.Name()]; ok {
-					if counter == 0 {
-						//get tag from kobject service configure
-						tag := f.Tag(komposeObject.LoadedFrom)
-						keysFound = append(keysFound, tag)
-					}
-					unsupportedKey[f.Name()]++
+					//get tag from kobject service configure
+					tag := f.Tag(komposeObject.LoadedFrom)
+					keysFound = append(keysFound, tag)
+					unsupportedKey[f.Name()] = true
 				}
 			}
 		}

diff --git a/pkg/transformer/openshift/openshift.go b/pkg/transformer/openshift/openshift.go
@@ -55,9 +55,9 @@ const TIMEOUT = 300
 // list of all unsupported keys for this transformer
 // Keys are names of variables in kobject struct.
 // this is map to make searching for keys easier
-// also counts how many times was given key found in kobject
-// to make sure that we show warning only once for every key
-var unsupportedKey = map[string]int{}
+// to make sure that unsupported key is not going to be reported twice
+// by keeping record if already saw this key in another service
+var unsupportedKey = map[string]bool{}
 
 // getImageTag get tag name from image name
 // if no tag is specified return 'latest'