Fix creation of aw/job with same name in different namespaces

Author: ChristianZaccaria

go.mod

@@ -18,7 +18,9 @@ require (
 	k8s.io/client-go v0.26.2
 	k8s.io/klog/v2 v2.90.1
 	k8s.io/metrics v0.26.2
+	k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5
 	sigs.k8s.io/custom-metrics-apiserver v0.0.0
+	sigs.k8s.io/structured-merge-diff/v4 v4.2.3
 )
 
 replace sigs.k8s.io/custom-metrics-apiserver => sigs.k8s.io/custom-metrics-apiserver v1.25.1-0.20230306170449-63d8c93851f3
@@ -107,9 +109,7 @@ require (
 	k8s.io/component-base v0.26.2 // indirect
 	k8s.io/kms v0.26.2 // indirect
 	k8s.io/kube-openapi v0.0.0-20230303024457-afdc3dddf62d // indirect
-	k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.35 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
-	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
 	sigs.k8s.io/yaml v1.3.0 // indirect
 )

pkg/controller/queuejobresources/genericresource/genericresource.go

@@ -43,7 +43,7 @@ import (
 	"k8s.io/client-go/restmapper"
 )
 
-var appwrapperJobName = "appwrapper.mcad.ibm.com"
+var appwrapperJobLabelName = "appwrapper.mcad.ibm.com"
 var resourceName = "resourceName"
 var appWrapperKind = arbv1.SchemeGroupVersion.WithKind("AppWrapper")
 
@@ -163,7 +163,7 @@ func (gr *GenericResources) Cleanup(aw *arbv1.AppWrapper, awr *arbv1.AppWrapperG
 	}
 
 	// Get the resource to see if it exists
-	labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobName, aw.Name, resourceName, unstruct.GetName())
+	labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobLabelName, aw.Name, resourceName, unstruct.GetNamespace() + "-" + unstruct.GetName())
 	inEtcd, err := dclient.Resource(rsrc).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
 	if err != nil {
 		return name, gvk, err
@@ -172,8 +172,8 @@ func (gr *GenericResources) Cleanup(aw *arbv1.AppWrapper, awr *arbv1.AppWrapperG
 	// Check to see if object already exists in etcd, if not, create the object.
 	if inEtcd != nil || len(inEtcd.Items) > 0 {
 		newName := name
-		if len(newName) > 63 {
-			newName = newName[:63]
+		if len(newName + namespace) > 63 {
+			newName = newName[:len(newName) - (len(newName) + len(namespace) - 63)]
 		}
 
 		err = deleteObject(namespaced, namespace, newName, rsrc, dclient)
@@ -184,7 +184,7 @@ func (gr *GenericResources) Cleanup(aw *arbv1.AppWrapper, awr *arbv1.AppWrapperG
 			return name, gvk, err
 		}
 	} else {
-		klog.Warningf("[Cleanup] %s/%s not found using label selector: %s.\n", name, namespace, labelSelector)
+		klog.Warningf("[Cleanup] %s/%s not found using label selector: %s.\n", namespace, name, labelSelector)
 	}
 
 	return name, gvk, err
@@ -291,18 +291,18 @@ func (gr *GenericResources) SyncQueueJob(aw *arbv1.AppWrapper, awr *arbv1.AppWra
 	} else {
 		labels = unstruct.GetLabels()
 	}
-	labels[appwrapperJobName] = aw.Name
-	labels[resourceName] = unstruct.GetName()
+	labels[appwrapperJobLabelName] = aw.Name
+	labels[resourceName] = unstruct.GetNamespace() + "-" + unstruct.GetName()
 	unstruct.SetLabels(labels)
 
 	// Add labels to pod template if one exists.
 	podTemplateFound := addLabelsToPodTemplateField(&unstruct, labels)
 	if !podTemplateFound {
-		klog.V(4).Infof("[SyncQueueJob] No pod template spec exists for resource: %s to add labels.", name)
+		klog.V(4).Infof("[SyncQueueJob] No pod template spec exists for resource: %s/%s to add labels.", namespace, name)
 	}
 
-	// Get the resource  to see if it exists
-	labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobName, aw.Name, resourceName, unstruct.GetName())
+	// Get the resource to see if it exists
+	labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobLabelName, aw.Name, resourceName, unstruct.GetNamespace() + "-" + unstruct.GetName())
 	inEtcd, err := dclient.Resource(rsrc).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
 	if err != nil {
 		return []*v1.Pod{}, err
@@ -311,8 +311,8 @@ func (gr *GenericResources) SyncQueueJob(aw *arbv1.AppWrapper, awr *arbv1.AppWra
 	// Check to see if object already exists in etcd, if not, create the object.
 	if inEtcd == nil || len(inEtcd.Items) < 1 {
 		newName := name
-		if len(newName) > 63 {
-			newName = newName[:63]
+		if len(newName + namespace) > 63 {
+			newName = newName[:len(newName) - (len(newName) + len(namespace) - 63)]
 		}
 		unstruct.SetName(newName)
 		//Asumption object is always namespaced
@@ -323,7 +323,7 @@ func (gr *GenericResources) SyncQueueJob(aw *arbv1.AppWrapper, awr *arbv1.AppWra
 			if errors.IsAlreadyExists(err) {
 				klog.V(4).Infof("%v\n", err.Error())
 			} else {
-				klog.Errorf("Error creating the object `%v`, the error is `%v`", newName, errors.ReasonForError(err))
+				klog.Errorf("Error creating the object `%s/%s`, the error is `%v`", namespace, newName, errors.ReasonForError(err))
 				return []*v1.Pod{}, err
 			}
 		}
@@ -493,7 +493,7 @@ func deleteObject(namespaced bool, namespace string, name string, rsrc schema.Gr
 	}
 
 	if err != nil && !errors.IsNotFound(err) {
-		klog.Errorf("[deleteObject] Error deleting the object `%v`, the error is `%v`.", name, errors.ReasonForError(err))
+		klog.Errorf("[deleteObject] Error deleting the object `%v`, in namespace %v, the error is `%v`.", name, namespace, errors.ReasonForError(err))
 		return err
 	} else {
 		klog.V(4).Infof("[deleteObject] Resource `%v` deleted.\n", name)
@@ -525,7 +525,7 @@ func GetListOfPodResourcesFromOneGenericItem(awr *arbv1.AppWrapperGenericResourc
 			klog.V(8).Infof("[GetListOfPodResourcesFromOneGenericItem] Requested total allocation resource from 1 pod `%v`.\n", podTotalresource)
 		}
 
-		// Addd individual pods to results
+		// Add individual pods to results
 		var replicaCount int = int(replicas)
 		for i := 0; i < replicaCount; i++ {
 			podResourcesList = append(podResourcesList, podTotalresource)
@@ -617,7 +617,7 @@ func getContainerResources(container v1.Container, replicas float64) *clustersta
 }
 
 // returns status of an item present in etcd
-func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResource, namespace string, appwrapperName string, genericItemName string) (completed bool) {
+func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResource, appwrapperNamespace string, appwrapperName string, genericItemName string) (completed bool) {
 	dd := gr.clients.Discovery()
 	apigroups, err := restmapper.GetAPIGroupResources(dd)
 	if err != nil {
@@ -648,8 +648,8 @@ func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResourc
 		return false
 	}
 
-	labelSelector := fmt.Sprintf("%s=%s", appwrapperJobName, appwrapperName)
-	inEtcd, err := dclient.Resource(rsrc).Namespace(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
+	labelSelector := fmt.Sprintf("%s=%s", appwrapperJobLabelName, appwrapperName)
+	inEtcd, err := dclient.Resource(rsrc).Namespace(appwrapperNamespace).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
 	if err != nil {
 		klog.Errorf("[IsItemCompleted] Error listing object: %v", err)
 		return false
@@ -669,7 +669,7 @@ func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResourc
 			}
 		}
 		if !validAwOwnerRef {
-			klog.Warningf("[IsItemCompleted] Item owner name %v does match appwrappper name %v in namespace %v", unstructuredObjectName, appwrapperName, namespace)
+			klog.Warningf("[IsItemCompleted] Item owner name %v does match appwrappper name %v in namespace %v", unstructuredObjectName, appwrapperName, appwrapperNamespace)
 			continue
 		}
 

test/e2e/queue.go

@@ -200,14 +200,32 @@ var _ = Describe("AppWrapper E2E Test", func() {
 		appwrappersPtr := &appwrappers
 		defer cleanupTestObjectsPtr(context, appwrappersPtr)
 
-		aw := createDeploymentAW(context, "aw-deployment-3")
+		aw := createDeploymentAW(context, "aw-deployment-3", "test")
 		appwrappers = append(appwrappers, aw)
 
 		fmt.Fprintf(GinkgoWriter, "[e2e] Awaiting %d pods running for AW %s.\n", aw.Spec.SchedSpec.MinAvailable, aw.Name)
 		err := waitAWPodsReady(context, aw)
 		Expect(err).NotTo(HaveOccurred())
 	})
 
+	It("Create Two AppWrappers Same Name Different Namespaces - Deployment Only - 3 Pods Each", func() {
+		fmt.Fprintf(os.Stdout, "[e2e] Create Two AppWrappers Same Name Different Namespaces - Deployment Only 3 Pods Each - Started.\n")
+		context := initTestContext()
+		var appwrappers []*arbv1.AppWrapper
+		appwrappersPtr := &appwrappers
+		defer cleanupTestObjectsPtr(context, appwrappersPtr)
+
+		namespaces := []string{"nstest1", "nstest2"}
+		for _, ns := range namespaces {
+			aw := createDeploymentAW(context, "aw-deployment-3", ns)
+			appwrappers = append(appwrappers, aw)
+
+			fmt.Fprintf(GinkgoWriter, "[e2e] Awaiting %d pods running for AW %s in namespace %s.\n", aw.Spec.SchedSpec.MinAvailable, aw.Name, ns)
+			err := waitAWPodsReady(context, aw)
+			Expect(err).NotTo(HaveOccurred())
+		}
+	})
+
 	It("Create AppWrapper - Generic Deployment Only - 3 pods", func() {
 		fmt.Fprintf(os.Stdout, "[e2e] Create AppWrapper - Generic Deployment Only - 3 pods - Started.\n")
 		context := initTestContext()

test/e2e/util.go

@@ -646,12 +646,12 @@ func createJobAWWithInitContainer(context *context, name string, requeuingTimeIn
 	return appwrapper
 }
 
-func createDeploymentAW(context *context, name string) *arbv1.AppWrapper {
-	rb := []byte(`{"apiVersion": "apps/v1",
+func createDeploymentAW(context *context, name string, namespace string) *arbv1.AppWrapper {
+	rb := []byte(fmt.Sprintf(`{"apiVersion": "apps/v1",
 		"kind": "Deployment", 
 	"metadata": {
 		"name": "aw-deployment-3",
-		"namespace": "test",
+		"namespace": "%s",
 		"labels": {
 			"app": "aw-deployment-3"
 		}
@@ -686,13 +686,13 @@ func createDeploymentAW(context *context, name string) *arbv1.AppWrapper {
 				]
 			}
 		}
-	}} `)
+	}} `, namespace))
 	var schedSpecMin int = 3
 
 	aw := &arbv1.AppWrapper{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
-			Namespace: context.namespace,
+			Namespace: namespace,
 		},
 		Spec: arbv1.AppWrapperSpec{
 			SchedSpec: arbv1.SchedulingSpecTemplate{
@@ -711,7 +711,7 @@ func createDeploymentAW(context *context, name string) *arbv1.AppWrapper {
 		},
 	}
 
-	appwrapper, err := context.karclient.WorkloadV1beta1().AppWrappers(context.namespace).Create(context.ctx, aw, metav1.CreateOptions{})
+	appwrapper, err := context.karclient.WorkloadV1beta1().AppWrappers(namespace).Create(context.ctx, aw, metav1.CreateOptions{})
 	Expect(err).NotTo(HaveOccurred())
 
 	return appwrapper