Repository: kubernetes/kompose
Pull request #1744 (2023-10-24 15:34:50)

fix: make transforming more deterministic
#### What type of PR is this?

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
-->
/kind bug

#### What this PR does / why we need it:
This PR makes the transformation deterministic by eliminating looping over maps. Instead, we are now looping over sorted arrays of keys.


Patch:
diff --git a/pkg/transformer/kubernetes/k8sutils.go b/pkg/transformer/kubernetes/k8sutils.go
index bb6ac558..03967e49 100644
--- a/pkg/transformer/kubernetes/k8sutils.go
+++ b/pkg/transformer/kubernetes/k8sutils.go
@@ -686,14 +686,16 @@ func getServiceGroupID(service kobject.ServiceConfig, mode string) string {
 //     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)
+	sortedServiceConfigs := SortedKeys(komposeObject.ServiceConfigs)
 
-	for name, service := range komposeObject.ServiceConfigs {
-		groupID := getServiceGroupID(service, opt.ServiceGroupMode)
+	for _, service := range sortedServiceConfigs {
+		serviceConfig := komposeObject.ServiceConfigs[service]
+		groupID := getServiceGroupID(serviceConfig, opt.ServiceGroupMode)
 		if groupID != "" {
-			service.Name = name
-			service.InGroup = true
-			serviceConfigGroup[groupID] = append(serviceConfigGroup[groupID], service)
-			komposeObject.ServiceConfigs[name] = service
+			serviceConfig.Name = service
+			serviceConfig.InGroup = true
+			serviceConfigGroup[groupID] = append(serviceConfigGroup[groupID], serviceConfig)
+			komposeObject.ServiceConfigs[service] = serviceConfig
 		}
 	}
 
@@ -806,9 +808,9 @@ func (k *Kubernetes) RemoveDupObjects(objs *[]runtime.Object) {
 }
 
 // SortedKeys Ensure the kubernetes objects are in a consistent order
-func SortedKeys(komposeObject kobject.KomposeObject) []string {
+func SortedKeys[V kobject.ServiceConfig | kobject.ServiceConfigGroup](serviceConfig map[string]V) []string {
 	var sortedKeys []string
-	for name := range komposeObject.ServiceConfigs {
+	for name := range serviceConfig {
 		sortedKeys = append(sortedKeys, name)
 	}
 	sort.Strings(sortedKeys)
diff --git a/pkg/transformer/kubernetes/kubernetes.go b/pkg/transformer/kubernetes/kubernetes.go
index 57e8ee18..3ced87bf 100644
--- a/pkg/transformer/kubernetes/kubernetes.go
+++ b/pkg/transformer/kubernetes/kubernetes.go
@@ -1455,28 +1455,33 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 	if opt.ServiceGroupMode != "" {
 		log.Debugf("Service group mode is: %s", opt.ServiceGroupMode)
 		komposeObjectToServiceConfigGroupMapping := KomposeObjectToServiceConfigGroupMapping(&komposeObject, opt)
-		for name, group := range komposeObjectToServiceConfigGroupMapping {
+		sortedGroupMappingKeys := SortedKeys(komposeObjectToServiceConfigGroupMapping)
+		for _, group := range sortedGroupMappingKeys {
+			groupMapping := komposeObjectToServiceConfigGroupMapping[group]
 			var objects []runtime.Object
 			podSpec := PodSpec{}
 
+			var groupName string
 			// if using volume group, the name here will be a volume config string. reset to the first service name
 			if opt.ServiceGroupMode == "volume" {
 				if opt.ServiceGroupName != "" {
-					name = opt.ServiceGroupName
+					groupName = opt.ServiceGroupName
 				} else {
 					var names []string
-					for _, svc := range group {
+					for _, svc := range groupMapping {
 						names = append(names, svc.Name)
 					}
-					name = strings.Join(names, "-")
+					groupName = strings.Join(names, "-")
 				}
+			} else {
+				groupName = group
 			}
 
 			// added a container
 			// ports conflict check between services
 			portsUses := map[string]bool{}
 
-			for _, service := range group {
+			for _, service := range groupMapping {
 				// first do ports check
 				ports := ConfigPorts(service)
 				for _, port := range ports {
@@ -1487,7 +1492,7 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 					portsUses[key] = true
 				}
 
-				log.Infof("Group Service %s to [%s]", service.Name, name)
+				log.Infof("Group Service %s to [%s]", service.Name, groupName)
 				service.WithKomposeAnnotation = opt.WithKomposeAnnotation
 				podSpec.Append(AddContainer(service, opt))
 
@@ -1495,17 +1500,17 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 					return nil, err
 				}
 				// override..
-				objects = append(objects, k.CreateWorkloadAndConfigMapObjects(name, service, opt)...)
-				k.configKubeServiceAndIngressForService(service, name, &objects)
+				objects = append(objects, k.CreateWorkloadAndConfigMapObjects(groupName, service, opt)...)
+				k.configKubeServiceAndIngressForService(service, groupName, &objects)
 
 				// Configure the container volumes.
-				volumesMount, volumes, pvc, cms, err := k.ConfigVolumes(name, service)
+				volumesMount, volumes, pvc, cms, err := k.ConfigVolumes(groupName, service)
 				if err != nil {
 					return nil, errors.Wrap(err, "k.ConfigVolumes failed")
 				}
 				// Configure Tmpfs
 				if len(service.TmpFs) > 0 {
-					TmpVolumesMount, TmpVolumes := k.ConfigTmpfs(name, service)
+					TmpVolumesMount, TmpVolumes := k.ConfigTmpfs(groupName, service)
 					volumes = append(volumes, TmpVolumes...)
 					volumesMount = append(volumesMount, TmpVolumesMount...)
 				}
@@ -1527,14 +1532,14 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 
 				podSpec.Append(
 					SetPorts(service),
-					ImagePullPolicy(name, service),
-					RestartPolicy(name, service),
-					SecurityContext(name, service),
+					ImagePullPolicy(groupName, service),
+					RestartPolicy(groupName, service),
+					SecurityContext(groupName, service),
 					HostName(service),
 					DomainName(service),
 					ResourcesLimits(service),
 					ResourcesRequests(service),
-					TerminationGracePeriodSeconds(name, service),
+					TerminationGracePeriodSeconds(groupName, service),
 					TopologySpreadConstraints(service),
 				)
 
@@ -1542,7 +1547,7 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 					podSpec.Append(ServiceAccountName(serviceAccountName))
 				}
 
-				err = k.UpdateKubernetesObjectsMultipleContainers(name, service, &objects, podSpec)
+				err = k.UpdateKubernetesObjectsMultipleContainers(groupName, service, &objects, podSpec)
 				if err != nil {
 					return nil, errors.Wrap(err, "Error transforming Kubernetes objects")
 				}
@@ -1557,7 +1562,7 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
 			allobjects = append(allobjects, objects...)
 		}
 	}
-	sortedKeys := SortedKeys(komposeObject)
+	sortedKeys := SortedKeys(komposeObject.ServiceConfigs)
 	for _, name := range sortedKeys {
 		service := komposeObject.ServiceConfigs[name]
 
diff --git a/pkg/transformer/openshift/openshift.go b/pkg/transformer/openshift/openshift.go
index ba53409b..04f5a155 100644
--- a/pkg/transformer/openshift/openshift.go
+++ b/pkg/transformer/openshift/openshift.go
@@ -280,7 +280,7 @@ func (o *OpenShift) Transform(komposeObject kobject.KomposeObject, opt kobject.C
 		}
 	}
 
-	sortedKeys := kubernetes.SortedKeys(komposeObject)
+	sortedKeys := kubernetes.SortedKeys(komposeObject.ServiceConfigs)
 	for _, name := range sortedKeys {
 		service := komposeObject.ServiceConfigs[name]
 		var objects []runtime.Object


Test patch:
diff --git a/pkg/transformer/kubernetes/k8sutils_test.go b/pkg/transformer/kubernetes/k8sutils_test.go
index 15cfd87f..d050a610 100644
--- a/pkg/transformer/kubernetes/k8sutils_test.go
+++ b/pkg/transformer/kubernetes/k8sutils_test.go
@@ -511,7 +511,7 @@ func TestSortedKeys(t *testing.T) {
 	komposeObject := kobject.KomposeObject{
 		ServiceConfigs: map[string]kobject.ServiceConfig{"b": service, "a": service1},
 	}
-	a := SortedKeys(komposeObject)
+	a := SortedKeys(komposeObject.ServiceConfigs)
 	if !reflect.DeepEqual(a, c) {
 		t.Logf("Test Fail output should be %s", c)
 	}
diff --git a/script/test/fixtures/service-group/output-k8s.yaml b/script/test/fixtures/service-group/output-k8s.yaml
index e22fdd17..57929661 100644
--- a/script/test/fixtures/service-group/output-k8s.yaml
+++ b/script/test/fixtures/service-group/output-k8s.yaml
@@ -41,25 +41,25 @@ spec:
       containers:
         - env:
             - name: TZ
-          image: librenms/librenms:latest
-          name: librenms
-          ports:
-            - containerPort: 8000
-              hostPort: 8000
-              protocol: TCP
+          image: librenms/dispatcher:latest
+          name: dispatcher
           resources: {}
           volumeMounts:
             - mountPath: /data
               name: librenms-dispatcher-claim0
         - env:
             - name: TZ
-          image: librenms/dispatcher:latest
-          name: dispatcher
+          image: librenms/librenms:latest
+          name: librenms
+          ports:
+            - containerPort: 8000
+              hostPort: 8000
+              protocol: TCP
           resources: {}
           volumeMounts:
             - mountPath: /data
               name: librenms-dispatcher-claim0
-      hostname: dispatcher
+      hostname: librenms
       restartPolicy: Always
       volumes:
         - name: librenms-dispatcher-claim0