From 3419ae7fe17e7ff2d8e4fb79d0a9d9b560c7bb35 Mon Sep 17 00:00:00 2001 From: Tomas Kral Date: Mon, 12 Dec 2016 17:32:11 +0100 Subject: [PATCH] few updates based on review --- pkg/kobject/kobject.go | 4 +- pkg/loader/bundle/bundle.go | 40 +++++------ pkg/loader/compose/compose.go | 92 ++++++++++++------------ pkg/loader/compose/compose_test.go | 15 ++++ pkg/transformer/kubernetes/kubernetes.go | 40 +++++------ pkg/transformer/openshift/openshift.go | 6 +- 6 files changed, 105 insertions(+), 92 deletions(-) diff --git a/pkg/kobject/kobject.go b/pkg/kobject/kobject.go index 8bf78451..8f92665b 100644 --- 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 index 3a7f318e..78c57b6c 100644 --- 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 index 1db19a97..43d64ef7 100644 --- 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 index 30e5d0e4..3525bf9e 100644 --- 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 index 1a59ac18..7abf95ec 100644 --- 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 index a1986000..78a6b61e 100644 --- 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'