Allow to delete queue-name on deployment.

pkg/controller/jobs/deployment/deployment_webhook.go

@@ -27,7 +27,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
-	"sigs.k8s.io/kueue/pkg/controller/constants"
+	"sigs.k8s.io/kueue/pkg/constants"
+	controllerconsts "sigs.k8s.io/kueue/pkg/controller/constants"
 	"sigs.k8s.io/kueue/pkg/controller/jobframework"
 	"sigs.k8s.io/kueue/pkg/controller/jobframework/webhook"
 )
@@ -62,7 +63,11 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error {
 		if deployment.Spec.Template.Labels == nil {
 			deployment.Spec.Template.Labels = make(map[string]string, 1)
 		}
-		deployment.Spec.Template.Labels[constants.QueueLabel] = queueName
+		deployment.Labels[constants.ManagedByKueueLabel] = "true"
+		deployment.Spec.Template.Labels[controllerconsts.QueueLabel] = queueName
+	} else if deployment.Labels[constants.ManagedByKueueLabel] == "true" {
+		delete(deployment.Labels, constants.ManagedByKueueLabel)
+		delete(deployment.Spec.Template.Labels, controllerconsts.QueueLabel)
 	}
 
 	return nil
@@ -85,7 +90,7 @@ func (wh *Webhook) ValidateCreate(ctx context.Context, obj runtime.Object) (warn
 
 var (
 	labelsPath         = field.NewPath("metadata", "labels")
-	queueNameLabelPath = labelsPath.Key(constants.QueueLabel)
+	queueNameLabelPath = labelsPath.Key(controllerconsts.QueueLabel)
 )
 
 func (wh *Webhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
@@ -101,9 +106,8 @@ func (wh *Webhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Ob
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, jobframework.ValidateQueueName(newDeployment.Object())...)
 
-	// Prevents updating the queue-name if at least one Pod is not suspended
-	// or if the queue-name has been deleted.
-	if oldDeployment.Status.ReadyReplicas > 0 || newQueueName == "" {
+	// Prevents updating the queue-name if at least one Pod is not suspended.
+	if oldDeployment.Status.ReadyReplicas > 0 {
 		allErrs = append(allErrs, apivalidation.ValidateImmutableField(oldQueueName, newQueueName, queueNameLabelPath)...)
 	}
 

pkg/controller/jobs/deployment/deployment_webhook_test.go

@@ -45,6 +45,7 @@ func TestDefault(t *testing.T) {
 				Obj(),
 			want: testingdeployment.MakeDeployment("test-pod", "").
 				Queue("test-queue").
+				ManagedByKueue().
 				PodTemplateSpecQueue("test-queue").
 				Obj(),
 		},
@@ -55,13 +56,21 @@ func TestDefault(t *testing.T) {
 				Obj(),
 			want: testingdeployment.MakeDeployment("test-pod", "").
 				Queue("new-test-queue").
+				ManagedByKueue().
 				PodTemplateSpecQueue("new-test-queue").
 				Obj(),
 		},
 		"deployment without queue with pod template spec queue": {
 			deployment: testingdeployment.MakeDeployment("test-pod", "").PodTemplateSpecQueue("test-queue").Obj(),
 			want:       testingdeployment.MakeDeployment("test-pod", "").PodTemplateSpecQueue("test-queue").Obj(),
 		},
+		"deployment without queue with pod template spec queue and managed by kueue label": {
+			deployment: testingdeployment.MakeDeployment("test-pod", "").
+				ManagedByKueue().
+				PodTemplateSpecQueue("test-queue").
+				Obj(),
+			want: testingdeployment.MakeDeployment("test-pod", "").Obj(),
+		},
 	}
 
 	for name, tc := range testCases {
@@ -79,7 +88,7 @@ func TestDefault(t *testing.T) {
 			if err := w.Default(ctx, tc.deployment); err != nil {
 				t.Errorf("failed to set defaults for v1/deployment: %s", err)
 			}
-			if diff := cmp.Diff(tc.want, tc.deployment); len(diff) != 0 {
+			if diff := cmp.Diff(tc.want, tc.deployment, cmpopts.EquateEmpty()); len(diff) != 0 {
 				t.Errorf("Default() mismatch (-want,+got):\n%s", diff)
 			}
 		})
@@ -146,11 +155,18 @@ func TestValidateUpdate(t *testing.T) {
 			oldDeployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
 			newDeployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
 		},
-		"without queue": {
+		"without queue and ReadyReplicas = 0": {
 			oldDeployment: testingdeployment.MakeDeployment("test-pod", "").
 				Queue("test-queue").
 				Obj(),
 			newDeployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
+		},
+		"without queue and ReadyReplicas > 0": {
+			oldDeployment: testingdeployment.MakeDeployment("test-pod", "").
+				ReadyReplicas(1).
+				Queue("test-queue").
+				Obj(),
+			newDeployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
 			wantErr: field.ErrorList{
 				&field.Error{
 					Type:  field.ErrorTypeInvalid,

pkg/util/testingjobs/deployment/wrappers.go

@@ -24,7 +24,8 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"sigs.k8s.io/kueue/pkg/controller/constants"
+	"sigs.k8s.io/kueue/pkg/constants"
+	controllerconstants "sigs.k8s.io/kueue/pkg/controller/constants"
 )
 
 // DeploymentWrapper wraps a Deployment.
@@ -82,7 +83,12 @@ func (d *DeploymentWrapper) Label(k, v string) *DeploymentWrapper {
 
 // Queue updates the queue name of the Deployment
 func (d *DeploymentWrapper) Queue(q string) *DeploymentWrapper {
-	return d.Label(constants.QueueLabel, q)
+	return d.Label(controllerconstants.QueueLabel, q)
+}
+
+// ManagedByKueue updates the queue name of the Deployment
+func (d *DeploymentWrapper) ManagedByKueue() *DeploymentWrapper {
+	return d.Label(constants.ManagedByKueueLabel, "true")
 }
 
 // Name updated the name of the Deployment
@@ -130,5 +136,5 @@ func (d *DeploymentWrapper) PodTemplateSpecLabel(k, v string) *DeploymentWrapper
 
 // PodTemplateSpecQueue updates the queue name of the pod template spec of the Deployment
 func (d *DeploymentWrapper) PodTemplateSpecQueue(q string) *DeploymentWrapper {
-	return d.PodTemplateSpecLabel(constants.QueueLabel, q)
+	return d.PodTemplateSpecLabel(controllerconstants.QueueLabel, q)
 }

test/e2e/singlecluster/deployment_test.go

@@ -149,7 +149,7 @@ var _ = ginkgo.Describe("Deployment", func() {
 				g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), createdDeployment)).To(gomega.Succeed())
 				g.Expect(createdDeployment.Status.Replicas).To(gomega.Equal(int32(3)))
 				g.Expect(createdDeployment.Status.UnavailableReplicas).To(gomega.Equal(int32(3)))
-				g.Expect(createdDeployment.Status.AvailableReplicas).To(gomega.Equal(int32(0)))
+				g.Expect(createdDeployment.Status.ReadyReplicas).To(gomega.Equal(int32(0)))
 			}, util.Timeout, util.Interval).Should(gomega.Succeed())
 		})
 

test/integration/webhook/jobs/deployment_webhook_test.go

@@ -114,9 +114,18 @@ var _ = ginkgo.Describe("Deployment Webhook", ginkgo.Ordered, ginkgo.ContinueOnF
 			})
 		})
 
-		ginkgo.It("shouldn't allow to remove the queue label", func() {
+		ginkgo.It("shouldn't allow to remove the queue label (ReadyReplicas > 0)", func() {
 			createdDeployment := &appsv1.Deployment{}
 
+			ginkgo.By("Update status ReadyReplicas = 1", func() {
+				gomega.Eventually(func(g gomega.Gomega) {
+					g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), createdDeployment)).Should(gomega.Succeed())
+					createdDeployment.Status.Replicas = 1
+					createdDeployment.Status.ReadyReplicas = 1
+					gomega.Expect(k8sClient.Status().Update(ctx, createdDeployment)).To(gomega.Succeed())
+				}, util.Timeout, util.Interval).Should(gomega.Succeed())
+			})
+
 			ginkgo.By("Try to remove queue label", func() {
 				gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), createdDeployment)).Should(gomega.Succeed())
 				delete(createdDeployment.Labels, constants.QueueLabel)
@@ -131,6 +140,23 @@ var _ = ginkgo.Describe("Deployment Webhook", ginkgo.Ordered, ginkgo.ContinueOnF
 			})
 		})
 
+		ginkgo.It("should allow to remove the queue label (ReadyReplicas = 0)", func() {
+			createdDeployment := &appsv1.Deployment{}
+
+			ginkgo.By("Try to remove queue label", func() {
+				gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), createdDeployment)).Should(gomega.Succeed())
+				delete(createdDeployment.Labels, constants.QueueLabel)
+				gomega.Expect(k8sClient.Update(ctx, createdDeployment)).To(gomega.Succeed())
+			})
+
+			ginkgo.By("Check that queue label deleted from pod template spec", func() {
+				gomega.Eventually(func(g gomega.Gomega) {
+					g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), createdDeployment)).Should(gomega.Succeed())
+					g.Expect(createdDeployment.Spec.Template.Labels).ShouldNot(gomega.HaveKey(constants.QueueLabel))
+				}, util.Timeout, util.Interval).Should(gomega.Succeed())
+			})
+		})
+
 		ginkgo.It("shouldn't allow to change the queue name (ReadyReplicas > 0)", func() {
 			createdDeployment := &appsv1.Deployment{}