Skip to content

Commit

Permalink
Fix creation of aw/job with same name in different namespaces
Browse files Browse the repository at this point in the history
  • Loading branch information
ChristianZaccaria committed Oct 23, 2023
1 parent b420be0 commit cedd7ad
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 45 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)
30 changes: 16 additions & 14 deletions pkg/controller/queuejobresources/genericresource/genericresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ import (
"k8s.io/client-go/restmapper"
)

var appwrapperJobName = "appwrapper.mcad.ibm.com"
var appwrapperJobLabelName = "appwrapper.mcad.ibm.com"
var appwrapperJobLabelNamespace = "appwrapper.mcad.ibm.com/namespace"
var resourceName = "resourceName"
var appWrapperKind = arbv1.SchemeGroupVersion.WithKind("AppWrapper")

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

// Get the resource to see if it exists in the AppWrapper namespace
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobName, aw.Name, resourceName, unstruct.GetName())
labelSelector := fmt.Sprintf("%s=%s, %s=%s", appwrapperJobLabelName, aw.Name, appwrapperJobLabelNamespace, aw.Namespace)
inEtcd, err := dclient.Resource(rsrc).Namespace(aw.Namespace).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
if err != nil {
return name, gvk, err
Expand All @@ -187,7 +188,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
Expand Down Expand Up @@ -297,18 +298,19 @@ func (gr *GenericResources) SyncQueueJob(aw *arbv1.AppWrapper, awr *arbv1.AppWra
} else {
labels = unstruct.GetLabels()
}
labels[appwrapperJobName] = aw.Name
labels[appwrapperJobLabelName] = aw.Name
labels[appwrapperJobLabelNamespace] = aw.Namespace
labels[resourceName] = 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, appwrapperJobLabelNamespace, aw.Namespace)
inEtcd, err := dclient.Resource(rsrc).List(context.Background(), metav1.ListOptions{LabelSelector: labelSelector})
if err != nil {
return []*v1.Pod{}, err
Expand All @@ -329,7 +331,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
}
}
Expand Down Expand Up @@ -499,7 +501,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)
Expand Down Expand Up @@ -531,7 +533,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)
Expand Down Expand Up @@ -623,7 +625,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 {
Expand Down Expand Up @@ -654,8 +656,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, %s=%s", appwrapperJobLabelName, appwrapperName, appwrapperJobLabelNamespace, appwrapperNamespace)
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
Expand All @@ -675,7 +677,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
}

Expand Down
1 change: 1 addition & 0 deletions test/e2e-kuttl-deployment-01/steps/01-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ metadata:
labels:
app: no-quota-deployment-01
appwrapper.mcad.ibm.com: no-quota-deployment-01
appwrapper.mcad.ibm.com/namespace: start-up
resourceName: no-quota-deployment-01
status:
availableReplicas: 1
Expand Down
31 changes: 17 additions & 14 deletions test/e2e-kuttl-deployment-01/steps/03-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,30 @@ metadata:
namespace: start-up
labels:
appwrapper.mcad.ibm.com: hold-completion-job-03
appwrapper.mcad.ibm.com/namespace: start-up
resourceName: hold-completion-job-03-01
status:
conditions:
- status: "True"
type: Complete
succeeded: 1
---
apiVersion: v1
kind: Pod
metadata:
apiVersion: v1
kind: Pod
metadata:
namespace: start-up
labels:
appwrapper.mcad.ibm.com: hold-completion-job-03
job-name: hold-completion-job-03-01
resourceName: hold-completion-job-03-01
labels:
appwrapper.mcad.ibm.com: hold-completion-job-03
appwrapper.mcad.ibm.com/namespace: start-up
job-name: hold-completion-job-03-01
resourceName: hold-completion-job-03-01
---
apiVersion: v1
kind: Pod
metadata:
apiVersion: v1
kind: Pod
metadata:
namespace: start-up
labels:
appwrapper.mcad.ibm.com: hold-completion-job-03
job-name: hold-completion-job-03-02
resourceName: hold-completion-job-03-02
labels:
appwrapper.mcad.ibm.com: hold-completion-job-03
appwrapper.mcad.ibm.com/namespace: start-up
job-name: hold-completion-job-03-02
resourceName: hold-completion-job-03-02
14 changes: 7 additions & 7 deletions test/e2e-kuttl-deployment-01/steps/07-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ metadata:
status:
state: Running
---
apiVersion: v1
kind: Pod
metadata:
apiVersion: v1
kind: Pod
metadata:
namespace: start-up
labels:
appwrapper.mcad.ibm.com: no-quota-job-06
job-name: no-quota-job-06
labels:
appwrapper.mcad.ibm.com: no-quota-job-06
appwrapper.mcad.ibm.com/namespace: start-up
job-name: no-quota-job-06
resourceName: no-quota-job-06

1 change: 1 addition & 0 deletions test/e2e-kuttl-deployment-02/steps/02-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ metadata:
labels:
app: no-quota-deployment-02
appwrapper.mcad.ibm.com: no-quota-deployment-02
appwrapper.mcad.ibm.com/namespace: start-up-02
resourceName: no-quota-deployment-02
status:
availableReplicas: 1
Expand Down
1 change: 1 addition & 0 deletions test/e2e-kuttl-deployment-03/steps/02-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ metadata:
labels:
app: no-quota-deployment-02
appwrapper.mcad.ibm.com: no-quota-deployment-02
appwrapper.mcad.ibm.com/namespace: start-up-03
resourceName: no-quota-deployment-02
status:
availableReplicas: 1
3 changes: 2 additions & 1 deletion test/e2e-kuttl/quota-errors/03-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ metadata:
labels:
app: deployment-silver-lo-pri-1replica
appwrapper.mcad.ibm.com: deployment-silver-lo-pri-1replica
resourceName: deployment-silver-lo-pri-1replica
appwrapper.mcad.ibm.com/namespace: quota-errors
resourceName: deployment-silver-lo-pri-1replica
status:
availableReplicas: 1
observedGeneration: 1
Expand Down
20 changes: 19 additions & 1 deletion test/e2e/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 7 additions & 6 deletions test/e2e/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -1512,6 +1512,7 @@ func createGenericServiceAWWithNoStatus(context *context, name string) *arbv1.Ap
"metadata": {
"labels": {
"appwrapper.mcad.ibm.com": "test-dep-job-item",
"appwrapper.mcad.ibm.com/namespace": "test",
"resourceName": "test-dep-job-item-svc"
},
"name": "test-dep-job-item-svc",
Expand Down

0 comments on commit cedd7ad

Please sign in to comment.