Skip to content

Commit cedd7ad

Browse files
ChristianZaccariaChristianZaccaria
committed
Fix creation of aw/job with same name in different namespaces
1 parent b420be0 commit cedd7ad

File tree

10 files changed

+73
-45
lines changed

10 files changed

+73
-45
lines changed

‎go.mod‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ require (
1818
k8s.io/client-go v0.26.2
1919
k8s.io/klog/v2 v2.90.1
2020
k8s.io/metrics v0.26.2
21+
k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5
2122
sigs.k8s.io/custom-metrics-apiserver v0.0.0
23+
sigs.k8s.io/structured-merge-diff/v4 v4.2.3
2224
)
2325

2426
replace sigs.k8s.io/custom-metrics-apiserver => sigs.k8s.io/custom-metrics-apiserver v1.25.1-0.20230306170449-63d8c93851f3
@@ -107,9 +109,7 @@ require (
107109
k8s.io/component-base v0.26.2 // indirect
108110
k8s.io/kms v0.26.2 // indirect
109111
k8s.io/kube-openapi v0.0.0-20230303024457-afdc3dddf62d // indirect
110-
k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 // indirect
111112
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.35 // indirect
112113
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
113-
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
114114
sigs.k8s.io/yaml v1.3.0 // indirect
115115
)

‎pkg/controller/queuejobresources/genericresource/genericresource.go‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ import (
4343
"k8s.io/client-go/restmapper"
4444
)
4545

46-
var appwrapperJobName = "appwrapper.mcad.ibm.com"
46+
var appwrapperJobLabelName = "appwrapper.mcad.ibm.com"
47+
var appwrapperJobLabelNamespace = "appwrapper.mcad.ibm.com/namespace"
4748
var resourceName = "resourceName"
4849
var appWrapperKind = arbv1.SchemeGroupVersion.WithKind("AppWrapper")
4950

@@ -166,7 +167,7 @@ func (gr *GenericResources) Cleanup(aw *arbv1.AppWrapper, awr *arbv1.AppWrapperG
166167
}
167168

168169
// Get the resource to see if it exists in the AppWrapper namespace
169-
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobName, aw.Name, resourceName, unstruct.GetName())
170+
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobLabelName, aw.Name, appwrapperJobLabelNamespace, aw.Namespace)
170171
inEtcd, err := dclient.Resource(rsrc).Namespace(aw.Namespace).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
171172
if err != nil {
172173
return name, gvk, err
@@ -187,7 +188,7 @@ func (gr *GenericResources) Cleanup(aw *arbv1.AppWrapper, awr *arbv1.AppWrapperG
187188
return name, gvk, err
188189
}
189190
} else {
190-
klog.Warningf("[Cleanup] %s/%s not found using label selector: %s.\n", name, namespace, labelSelector)
191+
klog.Warningf("[Cleanup] %s/%s not found using label selector: %s.\n", namespace, name, labelSelector)
191192
}
192193

193194
return name, gvk, err
@@ -297,18 +298,19 @@ func (gr *GenericResources) SyncQueueJob(aw *arbv1.AppWrapper, awr *arbv1.AppWra
297298
} else {
298299
labels = unstruct.GetLabels()
299300
}
300-
labels[appwrapperJobName] = aw.Name
301+
labels[appwrapperJobLabelName] = aw.Name
302+
labels[appwrapperJobLabelNamespace] = aw.Namespace
301303
labels[resourceName] = unstruct.GetName()
302304
unstruct.SetLabels(labels)
303305

304306
// Add labels to pod template if one exists.
305307
podTemplateFound := addLabelsToPodTemplateField(&unstruct, labels)
306308
if !podTemplateFound {
307-
klog.V(4).Infof("[SyncQueueJob] No pod template spec exists for resource: %s to add labels.", name)
309+
klog.V(4).Infof("[SyncQueueJob] No pod template spec exists for resource: %s/%s to add labels.", namespace, name)
308310
}
309311

310-
// Get the resource to see if it exists
311-
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobName, aw.Name, resourceName, unstruct.GetName())
312+
// Get the resource to see if it exists
313+
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobLabelName, aw.Name, appwrapperJobLabelNamespace, aw.Namespace)
312314
inEtcd, err := dclient.Resource(rsrc).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
313315
if err != nil {
314316
return []*v1.Pod{}, err
@@ -329,7 +331,7 @@ func (gr *GenericResources) SyncQueueJob(aw *arbv1.AppWrapper, awr *arbv1.AppWra
329331
if errors.IsAlreadyExists(err) {
330332
klog.V(4).Infof("%v\n", err.Error())
331333
} else {
332-
klog.Errorf("Error creating the object `%v`, the error is `%v`", newName, errors.ReasonForError(err))
334+
klog.Errorf("Error creating the object `%s/%s`, the error is `%v`", namespace, newName, errors.ReasonForError(err))
333335
return []*v1.Pod{}, err
334336
}
335337
}
@@ -499,7 +501,7 @@ func deleteObject(namespaced bool, namespace string, name string, rsrc schema.Gr
499501
}
500502

501503
if err != nil && !errors.IsNotFound(err) {
502-
klog.Errorf("[deleteObject] Error deleting the object `%v`, the error is `%v`.", name, errors.ReasonForError(err))
504+
klog.Errorf("[deleteObject] Error deleting the object `%v`, in namespace %v, the error is `%v`.", name, namespace, errors.ReasonForError(err))
503505
return err
504506
} else {
505507
klog.V(4).Infof("[deleteObject] Resource `%v` deleted.\n", name)
@@ -531,7 +533,7 @@ func GetListOfPodResourcesFromOneGenericItem(awr *arbv1.AppWrapperGenericResourc
531533
klog.V(8).Infof("[GetListOfPodResourcesFromOneGenericItem] Requested total allocation resource from 1 pod `%v`.\n", podTotalresource)
532534
}
533535

534-
// Addd individual pods to results
536+
// Add individual pods to results
535537
var replicaCount int = int(replicas)
536538
for i := 0; i < replicaCount; i++ {
537539
podResourcesList = append(podResourcesList, podTotalresource)
@@ -623,7 +625,7 @@ func getContainerResources(container v1.Container, replicas float64) *clustersta
623625
}
624626

625627
// returns status of an item present in etcd
626-
func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResource, namespace string, appwrapperName string, genericItemName string) (completed bool) {
628+
func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResource, appwrapperNamespace string, appwrapperName string, genericItemName string) (completed bool) {
627629
dd := gr.clients.Discovery()
628630
apigroups, err := restmapper.GetAPIGroupResources(dd)
629631
if err != nil {
@@ -654,8 +656,8 @@ func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResourc
654656
return false
655657
}
656658

657-
labelSelector := fmt.Sprintf("%s=%s", appwrapperJobName, appwrapperName)
658-
inEtcd, err := dclient.Resource(rsrc).Namespace(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
659+
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobLabelName, appwrapperName, appwrapperJobLabelNamespace, appwrapperNamespace)
660+
inEtcd, err := dclient.Resource(rsrc).Namespace(appwrapperNamespace).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
659661
if err != nil {
660662
klog.Errorf("[IsItemCompleted] Error listing object: %v", err)
661663
return false
@@ -675,7 +677,7 @@ func (gr *GenericResources) IsItemCompleted(awgr *arbv1.AppWrapperGenericResourc
675677
}
676678
}
677679
if !validAwOwnerRef {
678-
klog.Warningf("[IsItemCompleted] Item owner name %v does match appwrappper name %v in namespace %v", unstructuredObjectName, appwrapperName, namespace)
680+
klog.Warningf("[IsItemCompleted] Item owner name %v does match appwrappper name %v in namespace %v", unstructuredObjectName, appwrapperName, appwrapperNamespace)
679681
continue
680682
}
681683

‎test/e2e-kuttl-deployment-01/steps/01-assert.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ metadata:
1515
labels:
1616
app: no-quota-deployment-01
1717
appwrapper.mcad.ibm.com: no-quota-deployment-01
18+
appwrapper.mcad.ibm.com/namespace: start-up
1819
resourceName: no-quota-deployment-01
1920
status:
2021
availableReplicas: 1

‎test/e2e-kuttl-deployment-01/steps/03-assert.yaml‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,30 @@ metadata:
1313
namespace: start-up
1414
labels:
1515
appwrapper.mcad.ibm.com: hold-completion-job-03
16+
appwrapper.mcad.ibm.com/namespace: start-up
1617
resourceName: hold-completion-job-03-01
1718
status:
1819
conditions:
1920
- status: "True"
2021
type: Complete
2122
succeeded: 1
2223
---
23-
apiVersion: v1
24-
kind: Pod
25-
metadata:
24+
apiVersion: v1
25+
kind: Pod
26+
metadata:
2627
namespace: start-up
27-
labels:
28-
appwrapper.mcad.ibm.com: hold-completion-job-03
29-
job-name: hold-completion-job-03-01
30-
resourceName: hold-completion-job-03-01
28+
labels:
29+
appwrapper.mcad.ibm.com: hold-completion-job-03
30+
appwrapper.mcad.ibm.com/namespace: start-up
31+
job-name: hold-completion-job-03-01
32+
resourceName: hold-completion-job-03-01
3133
---
32-
apiVersion: v1
33-
kind: Pod
34-
metadata:
34+
apiVersion: v1
35+
kind: Pod
36+
metadata:
3537
namespace: start-up
36-
labels:
37-
appwrapper.mcad.ibm.com: hold-completion-job-03
38-
job-name: hold-completion-job-03-02
39-
resourceName: hold-completion-job-03-02
38+
labels:
39+
appwrapper.mcad.ibm.com: hold-completion-job-03
40+
appwrapper.mcad.ibm.com/namespace: start-up
41+
job-name: hold-completion-job-03-02
42+
resourceName: hold-completion-job-03-02

‎test/e2e-kuttl-deployment-01/steps/07-assert.yaml‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ metadata:
77
status:
88
state: Running
99
---
10-
apiVersion: v1
11-
kind: Pod
12-
metadata:
10+
apiVersion: v1
11+
kind: Pod
12+
metadata:
1313
namespace: start-up
14-
labels:
15-
appwrapper.mcad.ibm.com: no-quota-job-06
16-
job-name: no-quota-job-06
14+
labels:
15+
appwrapper.mcad.ibm.com: no-quota-job-06
16+
appwrapper.mcad.ibm.com/namespace: start-up
17+
job-name: no-quota-job-06
1718
resourceName: no-quota-job-06
18-

‎test/e2e-kuttl-deployment-02/steps/02-assert.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ metadata:
1515
labels:
1616
app: no-quota-deployment-02
1717
appwrapper.mcad.ibm.com: no-quota-deployment-02
18+
appwrapper.mcad.ibm.com/namespace: start-up-02
1819
resourceName: no-quota-deployment-02
1920
status:
2021
availableReplicas: 1

‎test/e2e-kuttl-deployment-03/steps/02-assert.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ metadata:
1515
labels:
1616
app: no-quota-deployment-02
1717
appwrapper.mcad.ibm.com: no-quota-deployment-02
18+
appwrapper.mcad.ibm.com/namespace: start-up-03
1819
resourceName: no-quota-deployment-02
1920
status:
2021
availableReplicas: 1

‎test/e2e-kuttl/quota-errors/03-assert.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ metadata:
1818
labels:
1919
app: deployment-silver-lo-pri-1replica
2020
appwrapper.mcad.ibm.com: deployment-silver-lo-pri-1replica
21-
resourceName: deployment-silver-lo-pri-1replica
21+
appwrapper.mcad.ibm.com/namespace: quota-errors
22+
resourceName: deployment-silver-lo-pri-1replica
2223
status:
2324
availableReplicas: 1
2425
observedGeneration: 1

‎test/e2e/queue.go‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,32 @@ var _ = Describe("AppWrapper E2E Test", func() {
200200
appwrappersPtr := &appwrappers
201201
defer cleanupTestObjectsPtr(context, appwrappersPtr)
202202

203-
aw := createDeploymentAW(context, "aw-deployment-3")
203+
aw := createDeploymentAW(context, "aw-deployment-3", "test")
204204
appwrappers = append(appwrappers, aw)
205205

206206
fmt.Fprintf(GinkgoWriter, "[e2e] Awaiting %d pods running for AW %s.\n", aw.Spec.SchedSpec.MinAvailable, aw.Name)
207207
err := waitAWPodsReady(context, aw)
208208
Expect(err).NotTo(HaveOccurred())
209209
})
210210

211+
It("Create Two AppWrappers Same Name Different Namespaces - Deployment Only - 3 Pods Each", func() {
212+
fmt.Fprintf(os.Stdout, "[e2e] Create Two AppWrappers Same Name Different Namespaces - Deployment Only 3 Pods Each - Started.\n")
213+
context := initTestContext()
214+
var appwrappers []*arbv1.AppWrapper
215+
appwrappersPtr := &appwrappers
216+
defer cleanupTestObjectsPtr(context, appwrappersPtr)
217+
218+
namespaces := []string{"nstest1", "nstest2"}
219+
for _, ns := range namespaces {
220+
aw := createDeploymentAW(context, "aw-deployment-3", ns)
221+
appwrappers = append(appwrappers, aw)
222+
223+
fmt.Fprintf(GinkgoWriter, "[e2e] Awaiting %d pods running for AW %s in namespace %s.\n", aw.Spec.SchedSpec.MinAvailable, aw.Name, ns)
224+
err := waitAWPodsReady(context, aw)
225+
Expect(err).NotTo(HaveOccurred())
226+
}
227+
})
228+
211229
It("Create AppWrapper - Generic Deployment Only - 3 pods", func() {
212230
fmt.Fprintf(os.Stdout, "[e2e] Create AppWrapper - Generic Deployment Only - 3 pods - Started.\n")
213231
context := initTestContext()

‎test/e2e/util.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -646,12 +646,12 @@ func createJobAWWithInitContainer(context *context, name string, requeuingTimeIn
646646
return appwrapper
647647
}
648648

649-
func createDeploymentAW(context *context, name string) *arbv1.AppWrapper {
650-
rb := []byte(`{"apiVersion": "apps/v1",
649+
func createDeploymentAW(context *context, name string, namespace string) *arbv1.AppWrapper {
650+
rb := []byte(fmt.Sprintf(`{"apiVersion": "apps/v1",
651651
"kind": "Deployment",
652652
"metadata": {
653653
"name": "aw-deployment-3",
654-
"namespace": "test",
654+
"namespace": "%s",
655655
"labels": {
656656
"app": "aw-deployment-3"
657657
}
@@ -686,13 +686,13 @@ func createDeploymentAW(context *context, name string) *arbv1.AppWrapper {
686686
]
687687
}
688688
}
689-
}} `)
689+
}} `, namespace))
690690
var schedSpecMin int = 3
691691

692692
aw := &arbv1.AppWrapper{
693693
ObjectMeta: metav1.ObjectMeta{
694694
Name: name,
695-
Namespace: context.namespace,
695+
Namespace: namespace,
696696
},
697697
Spec: arbv1.AppWrapperSpec{
698698
SchedSpec: arbv1.SchedulingSpecTemplate{
@@ -711,7 +711,7 @@ func createDeploymentAW(context *context, name string) *arbv1.AppWrapper {
711711
},
712712
}
713713

714-
appwrapper, err := context.karclient.WorkloadV1beta1().AppWrappers(context.namespace).Create(context.ctx, aw, metav1.CreateOptions{})
714+
appwrapper, err := context.karclient.WorkloadV1beta1().AppWrappers(namespace).Create(context.ctx, aw, metav1.CreateOptions{})
715715
Expect(err).NotTo(HaveOccurred())
716716

717717
return appwrapper
@@ -1512,6 +1512,7 @@ func createGenericServiceAWWithNoStatus(context *context, name string) *arbv1.Ap
15121512
"metadata": {
15131513
"labels": {
15141514
"appwrapper.mcad.ibm.com": "test-dep-job-item",
1515+
"appwrapper.mcad.ibm.com/namespace": "test",
15151516
"resourceName": "test-dep-job-item-svc"
15161517
},
15171518
"name": "test-dep-job-item-svc",

0 commit comments

Comments
 (0)