From fd7b751564d241bfe4d24a31c32768dc8ee71c69 Mon Sep 17 00:00:00 2001 From: AhmedGrati <48932084+AhmedGrati@users.noreply.github.com> Date: Sun, 13 Nov 2022 14:33:47 +0100 Subject: [PATCH] Fix container name (#1528) * fix: update get container name function * test: add a test for the introduced fix --- pkg/transformer/kubernetes/k8sutils.go | 24 +++++++-------- pkg/transformer/kubernetes/k8sutils_test.go | 34 ++++++++++++++++++--- pkg/transformer/kubernetes/kubernetes.go | 10 +++--- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/pkg/transformer/kubernetes/k8sutils.go b/pkg/transformer/kubernetes/k8sutils.go index fb530d5d..534584e1 100644 --- a/pkg/transformer/kubernetes/k8sutils.go +++ b/pkg/transformer/kubernetes/k8sutils.go @@ -680,15 +680,15 @@ func getServiceGroupID(service kobject.ServiceConfig, mode string) string { // KomposeObjectToServiceConfigGroupMapping returns the service config group by name or by volume // This group function works as following -// 1. Support two mode -// (1): label: use a custom label, the service that contains it will be merged to one workload. -// (2): volume: the service that share to exactly same volume config will be merged to one workload. If use pvc, only -// create one for this group. -// 2. If service containers restart policy and no workload argument provide and it's restart policy looks like a pod, then -// this service should generate a pod. If group mode specified, it should be grouped and ignore the restart policy. -// 3. If group mode specified, port conflict between services in one group will be ignored, and multiple service should be created. -// 4. If `volume` group mode specified, we don't have an appropriate name for this combined service, use the first one for now. -// A warn/info message should be printed to let the user know. +// 1. Support two mode +// (1): label: use a custom label, the service that contains it will be merged to one workload. +// (2): volume: the service that share to exactly same volume config will be merged to one workload. If use pvc, only +// create one for this group. +// 2. If service containers restart policy and no workload argument provide and it's restart policy looks like a pod, then +// this service should generate a pod. If group mode specified, it should be grouped and ignore the restart policy. +// 3. If group mode specified, port conflict between services in one group will be ignored, and multiple service should be created. +// 4. If `volume` group mode specified, we don't have an appropriate name for this combined service, use the first one for now. +// A warn/info message should be printed to let the user know. func KomposeObjectToServiceConfigGroupMapping(komposeObject *kobject.KomposeObject, opt kobject.ConvertOptions) map[string]kobject.ServiceConfigGroup { serviceConfigGroup := make(map[string]kobject.ServiceConfigGroup) @@ -881,7 +881,7 @@ func FormatFileName(name string) string { return strings.Replace(file, "_", "-", -1) } -//FormatContainerName format Container name +// FormatContainerName format Container name func FormatContainerName(name string) string { name = strings.Replace(name, "_", "-", -1) return name @@ -890,9 +890,9 @@ func FormatContainerName(name string) string { func GetContainerName(service kobject.ServiceConfig) string { name := service.Name if len(service.ContainerName) > 0 { - name = FormatContainerName(service.ContainerName) + name = service.ContainerName } - return name + return FormatContainerName(name) } // FormatResourceName generate a valid k8s resource name diff --git a/pkg/transformer/kubernetes/k8sutils_test.go b/pkg/transformer/kubernetes/k8sutils_test.go index 2dd3c841..22aae955 100644 --- a/pkg/transformer/kubernetes/k8sutils_test.go +++ b/pkg/transformer/kubernetes/k8sutils_test.go @@ -32,7 +32,7 @@ import ( ) /* - Test the creation of a service +Test the creation of a service */ func TestCreateService(t *testing.T) { // An example service @@ -179,8 +179,8 @@ func TestCreateServiceWithCPULimit(t *testing.T) { } /* - Test the creation of a service with a specified user. - The expected result is that Kompose will set user in PodSpec +Test the creation of a service with a specified user. +The expected result is that Kompose will set user in PodSpec */ func TestCreateServiceWithServiceUser(t *testing.T) { // An example service @@ -517,7 +517,7 @@ func TestSortedKeys(t *testing.T) { } } -//test conversion from duration string to seconds *int64 +// test conversion from duration string to seconds *int64 func TestDurationStrToSecondsInt(t *testing.T) { testCases := map[string]struct { in string @@ -571,3 +571,29 @@ func TestServiceWithServiceAccount(t *testing.T) { } } } + +func TestCreateServiceWithSpecialName(t *testing.T) { + service := kobject.ServiceConfig{ + ContainerName: "front_end", + Image: "nginx", + } + + // An example object generated via k8s runtime.Objects() + komposeObject := kobject.KomposeObject{ + ServiceConfigs: map[string]kobject.ServiceConfig{"app": service}, + } + k := Kubernetes{} + objects, err := k.Transform(komposeObject, kobject.ConvertOptions{CreateD: true, Replicas: 3}) + if err != nil { + t.Error(errors.Wrap(err, "k.Transform failed")) + } + expectedContainerName := "front-end" + for _, obj := range objects { + if deploy, ok := obj.(*appsv1.Deployment); ok { + containerName := deploy.Spec.Template.Spec.Containers[0].Name + if containerName != "front-end" { + t.Errorf("Error while transforming container name. Expected %s Got %s", expectedContainerName, containerName) + } + } + } +} diff --git a/pkg/transformer/kubernetes/kubernetes.go b/pkg/transformer/kubernetes/kubernetes.go index fa384145..5de64d45 100644 --- a/pkg/transformer/kubernetes/kubernetes.go +++ b/pkg/transformer/kubernetes/kubernetes.go @@ -128,7 +128,7 @@ func (k *Kubernetes) InitPodSpec(name string, image string, pullSecret string) a return pod } -//InitPodSpecWithConfigMap creates the pod specification +// InitPodSpecWithConfigMap creates the pod specification func (k *Kubernetes) InitPodSpecWithConfigMap(name string, image string, service kobject.ServiceConfig) api.PodSpec { var volumeMounts []api.VolumeMount var volumes []api.Volume @@ -245,7 +245,7 @@ func (k *Kubernetes) InitConfigMapForEnv(name string, opt kobject.ConvertOptions // IntiConfigMapFromFileOrDir will create a configmap from dir or file // usage: -// 1. volume +// 1. volume func (k *Kubernetes) IntiConfigMapFromFileOrDir(name, cmName, filePath string, service kobject.ServiceConfig) (*api.ConfigMap, error) { configMap := &api.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -324,7 +324,7 @@ func initConfigMapData(configMap *api.ConfigMap, data map[string]string) { configMap.BinaryData = binData } -//InitConfigMapFromFile initializes a ConfigMap object +// InitConfigMapFromFile initializes a ConfigMap object func (k *Kubernetes) InitConfigMapFromFile(name string, service kobject.ServiceConfig, fileName string) *api.ConfigMap { content, err := GetContentFromFile(fileName) if err != nil { @@ -708,7 +708,7 @@ func (k *Kubernetes) ConfigServicePorts(service kobject.ServiceConfig) []api.Ser return servicePorts } -//ConfigCapabilities configure POSIX capabilities that can be added or removed to a container +// ConfigCapabilities configure POSIX capabilities that can be added or removed to a container func ConfigCapabilities(service kobject.ServiceConfig) *api.Capabilities { capsAdd := []api.Capability{} capsDrop := []api.Capability{} @@ -1012,7 +1012,7 @@ func (k *Kubernetes) ConfigVolumes(name string, service kobject.ServiceConfig) ( } // ConfigEmptyVolumeSource is helper function to create an EmptyDir api.VolumeSource -//either for Tmpfs or for emptyvolumes +// either for Tmpfs or for emptyvolumes func (k *Kubernetes) ConfigEmptyVolumeSource(key string) *api.VolumeSource { //if key is tmpfs if key == "tmpfs" {