From 15d5c151585fd85fd154bdb96ee05b4ec4d3b09a Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 11:01:49 -0500 Subject: [PATCH 1/8] fix deployment and statefulset setup so ownership works --- .../jobs/deployment/deployment_controller.go | 20 ++++++++++++------- .../statefulset/statefulset_controller.go | 20 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/pkg/controller/jobs/deployment/deployment_controller.go b/pkg/controller/jobs/deployment/deployment_controller.go index df3a139305..504b16b571 100644 --- a/pkg/controller/jobs/deployment/deployment_controller.go +++ b/pkg/controller/jobs/deployment/deployment_controller.go @@ -20,6 +20,7 @@ import ( "context" appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -38,13 +39,14 @@ const ( func init() { utilruntime.Must(jobframework.RegisterIntegration(FrameworkName, jobframework.IntegrationCallbacks{ - SetupIndexes: SetupIndexes, - NewReconciler: jobframework.NewNoopReconcilerFactory(gvk), - GVK: gvk, - SetupWebhook: SetupWebhook, - JobType: &appsv1.Deployment{}, - AddToScheme: appsv1.AddToScheme, - DependencyList: []string{"pod"}, + SetupIndexes: SetupIndexes, + NewReconciler: jobframework.NewNoopReconcilerFactory(gvk), + GVK: gvk, + SetupWebhook: SetupWebhook, + JobType: &appsv1.Deployment{}, + AddToScheme: appsv1.AddToScheme, + DependencyList: []string{"pod"}, + IsManagingObjectsOwner: isDeployment, })) } @@ -65,3 +67,7 @@ func (d *Deployment) GVK() schema.GroupVersionKind { func SetupIndexes(context.Context, client.FieldIndexer) error { return nil } + +func isDeployment(owner *metav1.OwnerReference) bool { + return owner.Kind == "Deployment" && owner.APIVersion == gvk.GroupVersion().String() +} diff --git a/pkg/controller/jobs/statefulset/statefulset_controller.go b/pkg/controller/jobs/statefulset/statefulset_controller.go index da6a7a9fa3..e1ae2ae087 100644 --- a/pkg/controller/jobs/statefulset/statefulset_controller.go +++ b/pkg/controller/jobs/statefulset/statefulset_controller.go @@ -20,6 +20,7 @@ import ( "context" appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -38,13 +39,14 @@ const ( func init() { utilruntime.Must(jobframework.RegisterIntegration(FrameworkName, jobframework.IntegrationCallbacks{ - SetupIndexes: SetupIndexes, - NewReconciler: NewReconciler, - SetupWebhook: SetupWebhook, - JobType: &appsv1.StatefulSet{}, - AddToScheme: appsv1.AddToScheme, - DependencyList: []string{"pod"}, - GVK: gvk, + SetupIndexes: SetupIndexes, + NewReconciler: NewReconciler, + SetupWebhook: SetupWebhook, + JobType: &appsv1.StatefulSet{}, + AddToScheme: appsv1.AddToScheme, + DependencyList: []string{"pod"}, + GVK: gvk, + IsManagingObjectsOwner: isStatefulSet, })) } @@ -65,3 +67,7 @@ func (d *StatefulSet) GVK() schema.GroupVersionKind { func SetupIndexes(context.Context, client.FieldIndexer) error { return nil } + +func isStatefulSet(owner *metav1.OwnerReference) bool { + return owner.Kind == "StatefulSet" && owner.APIVersion == gvk.GroupVersion().String() +} From f5b356fe9aea10134a86d13237adb9124414d38d Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 11:37:01 -0500 Subject: [PATCH 2/8] enable deployment/statefulset to be recognized as child jobs --- charts/kueue/templates/rbac/role.yaml | 8 + charts/kueue/templates/webhook/webhook.yaml | 138 +++++++++--------- config/components/rbac/role.yaml | 8 + config/components/webhook/manifests.yaml | 78 +++++----- pkg/controller/jobframework/defaults.go | 18 ++- .../jobs/deployment/deployment_webhook.go | 31 ++-- .../deployment/deployment_webhook_test.go | 5 + pkg/controller/jobs/pod/pod_webhook.go | 13 +- .../jobs/statefulset/statefulset_webhook.go | 47 +++--- .../statefulset/statefulset_webhook_test.go | 4 + 10 files changed, 207 insertions(+), 143 deletions(-) diff --git a/charts/kueue/templates/rbac/role.yaml b/charts/kueue/templates/rbac/role.yaml index 437715c098..7d162ba521 100644 --- a/charts/kueue/templates/rbac/role.yaml +++ b/charts/kueue/templates/rbac/role.yaml @@ -96,6 +96,14 @@ rules: - get - list - watch + - apiGroups: + - apps/v1 + resources: + - replicasets + verbs: + - get + - list + - watch - apiGroups: - autoscaling.x-k8s.io resources: diff --git a/charts/kueue/templates/webhook/webhook.yaml b/charts/kueue/templates/webhook/webhook.yaml index e295c7aaba..f1e421ef9e 100644 --- a/charts/kueue/templates/webhook/webhook.yaml +++ b/charts/kueue/templates/webhook/webhook.yaml @@ -11,6 +11,40 @@ metadata: {{- end }} namespace: '{{ .Release.Namespace }}' webhooks: + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: '{{ include "kueue.fullname" . }}-webhook-service' + namespace: '{{ .Release.Namespace }}' + path: /mutate--v1-pod + {{- if has "pod" $integrationsConfig.frameworks }} + failurePolicy: Fail + {{- else }} + failurePolicy: Ignore + {{- end }} + name: mpod.kb.io + namespaceSelector: + {{- if and (hasKey $integrationsConfig "podOptions") (hasKey ($integrationsConfig.podOptions) "namespaceSelector") }} + {{- toYaml $integrationsConfig.podOptions.namespaceSelector | nindent 6 -}} + {{- else }} + matchExpressions: + - key: kubernetes.io/metadata.name + operator: NotIn + values: + - kube-system + - '{{ .Release.Namespace }}' + {{- end }} + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + resources: + - pods + sideEffects: None - admissionReviewVersions: - v1 clientConfig: @@ -194,40 +228,6 @@ webhooks: resources: - mpijobs sideEffects: None - - admissionReviewVersions: - - v1 - clientConfig: - service: - name: '{{ include "kueue.fullname" . }}-webhook-service' - namespace: '{{ .Release.Namespace }}' - path: /mutate--v1-pod - {{- if has "pod" $integrationsConfig.frameworks }} - failurePolicy: Fail - {{- else }} - failurePolicy: Ignore - {{- end }} - name: mpod.kb.io - namespaceSelector: - {{- if and (hasKey $integrationsConfig "podOptions") (hasKey ($integrationsConfig.podOptions) "namespaceSelector") }} - {{- toYaml $integrationsConfig.podOptions.namespaceSelector | nindent 6 -}} - {{- else }} - matchExpressions: - - key: kubernetes.io/metadata.name - operator: NotIn - values: - - kube-system - - '{{ .Release.Namespace }}' - {{- end }} - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - resources: - - pods - sideEffects: None - admissionReviewVersions: - v1 clientConfig: @@ -356,6 +356,41 @@ metadata: {{- end }} namespace: '{{ .Release.Namespace }}' webhooks: + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: '{{ include "kueue.fullname" . }}-webhook-service' + namespace: '{{ .Release.Namespace }}' + path: /validate--v1-pod + {{- if has "pod" $integrationsConfig.frameworks }} + failurePolicy: Fail + {{- else }} + failurePolicy: Ignore + {{- end }} + name: vpod.kb.io + namespaceSelector: + {{- if and (hasKey $integrationsConfig "podOptions") (hasKey ($integrationsConfig.podOptions) "namespaceSelector") }} + {{- toYaml $integrationsConfig.podOptions.namespaceSelector | nindent 6 -}} + {{- else }} + matchExpressions: + - key: kubernetes.io/metadata.name + operator: NotIn + values: + - kube-system + - '{{ .Release.Namespace }}' + {{- end }} + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - pods + sideEffects: None - admissionReviewVersions: - v1 clientConfig: @@ -547,41 +582,6 @@ webhooks: resources: - mpijobs sideEffects: None - - admissionReviewVersions: - - v1 - clientConfig: - service: - name: '{{ include "kueue.fullname" . }}-webhook-service' - namespace: '{{ .Release.Namespace }}' - path: /validate--v1-pod - {{- if has "pod" $integrationsConfig.frameworks }} - failurePolicy: Fail - {{- else }} - failurePolicy: Ignore - {{- end }} - name: vpod.kb.io - namespaceSelector: - {{- if and (hasKey $integrationsConfig "podOptions") (hasKey ($integrationsConfig.podOptions) "namespaceSelector") }} - {{- toYaml $integrationsConfig.podOptions.namespaceSelector | nindent 6 -}} - {{- else }} - matchExpressions: - - key: kubernetes.io/metadata.name - operator: NotIn - values: - - kube-system - - '{{ .Release.Namespace }}' - {{- end }} - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods - sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/config/components/rbac/role.yaml b/config/components/rbac/role.yaml index 3c2e7eba74..3e79946bde 100644 --- a/config/components/rbac/role.yaml +++ b/config/components/rbac/role.yaml @@ -95,6 +95,14 @@ rules: - get - list - watch +- apiGroups: + - apps/v1 + resources: + - replicasets + verbs: + - get + - list + - watch - apiGroups: - autoscaling.x-k8s.io resources: diff --git a/config/components/webhook/manifests.yaml b/config/components/webhook/manifests.yaml index 7b28e899b2..5d25e25be9 100644 --- a/config/components/webhook/manifests.yaml +++ b/config/components/webhook/manifests.yaml @@ -4,6 +4,25 @@ kind: MutatingWebhookConfiguration metadata: name: mutating-webhook-configuration webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate--v1-pod + failurePolicy: Fail + name: mpod.kb.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + resources: + - pods + sideEffects: None - admissionReviewVersions: - v1 clientConfig: @@ -176,25 +195,6 @@ webhooks: resources: - mpijobs sideEffects: None -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate--v1-pod - failurePolicy: Fail - name: mpod.kb.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - resources: - - pods - sideEffects: None - admissionReviewVersions: - v1 clientConfig: @@ -316,6 +316,26 @@ kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate--v1-pod + failurePolicy: Fail + name: vpod.kb.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - pods + sideEffects: None - admissionReviewVersions: - v1 clientConfig: @@ -496,26 +516,6 @@ webhooks: resources: - mpijobs sideEffects: None -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate--v1-pod - failurePolicy: Fail - name: vpod.kb.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods - sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/pkg/controller/jobframework/defaults.go b/pkg/controller/jobframework/defaults.go index ce6c65c5b6..fb834eb2ed 100644 --- a/pkg/controller/jobframework/defaults.go +++ b/pkg/controller/jobframework/defaults.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -41,12 +42,25 @@ func ApplyDefaultForSuspend(ctx context.Context, job GenericJob, k8sClient clien return nil } +// +kubebuilder:rbac:groups="apps/v1",resources=replicasets,verbs=get;list;watch + // WorkloadShouldBeSuspended determines whether jobObj should be default suspended on creation func WorkloadShouldBeSuspended(ctx context.Context, jobObj client.Object, k8sClient client.Client, manageJobsWithoutQueueName bool, managedJobsNamespaceSelector labels.Selector) (bool, error) { // Do not default suspend a job whose owner is already managed by Kueue - if owner := metav1.GetControllerOf(jobObj); owner != nil && IsOwnerManagedByKueue(owner) { - return false, nil + if owner := metav1.GetControllerOf(jobObj); owner != nil { + if owner.Kind == "ReplicaSet" && owner.APIVersion == "apps/v1" { + // To avoid a full-blown integration for ReplicaSet, we go up one level of ownership here + rs := &appsv1.ReplicaSet{} + err := k8sClient.Get(ctx, client.ObjectKey{Name: owner.Name, Namespace: jobObj.GetNamespace()}, rs) + if err != nil { + return false, fmt.Errorf("failed to get replicaset: %w", err) + } + owner = metav1.GetControllerOf(rs) + } + if owner != nil && IsOwnerManagedByKueue(owner) { + return false, nil + } } // Jobs with queue names whose parents are not managed by Kueue are default suspended diff --git a/pkg/controller/jobs/deployment/deployment_webhook.go b/pkg/controller/jobs/deployment/deployment_webhook.go index 437b580c95..2002529583 100644 --- a/pkg/controller/jobs/deployment/deployment_webhook.go +++ b/pkg/controller/jobs/deployment/deployment_webhook.go @@ -21,6 +21,7 @@ import ( appsv1 "k8s.io/api/apps/v1" apivalidation "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" @@ -30,19 +31,24 @@ import ( "sigs.k8s.io/kueue/pkg/controller/constants" "sigs.k8s.io/kueue/pkg/controller/jobframework" "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" + "sigs.k8s.io/kueue/pkg/controller/jobs/pod" "sigs.k8s.io/kueue/pkg/queue" ) type Webhook struct { - client client.Client - queues *queue.Manager + client client.Client + manageJobsWithoutQueueName bool + managedJobsNamespaceSelector labels.Selector + queues *queue.Manager } func SetupWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { options := jobframework.ProcessOptions(opts...) wh := &Webhook{ - client: mgr.GetClient(), - queues: options.Queues, + client: mgr.GetClient(), + manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, + managedJobsNamespaceSelector: options.ManagedJobsNamespaceSelector, + queues: options.Queues, } obj := &appsv1.Deployment{} return webhook.WebhookManagedBy(mgr). @@ -63,14 +69,19 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { log.V(5).Info("Propagating queue-name") jobframework.ApplyDefaultLocalQueue(deployment.Object(), wh.queues.DefaultLocalQueueExist) - - // Because Deployment is built using a NoOpReconciler handling of jobs without queue names is delegating to the Pod webhook. - queueName := jobframework.QueueNameForObject(deployment.Object()) - if queueName != "" { + suspend, err := jobframework.WorkloadShouldBeSuspended(ctx, deployment.Object(), wh.client, wh.manageJobsWithoutQueueName, wh.managedJobsNamespaceSelector) + if err != nil { + return err + } + if suspend { + queueName := jobframework.QueueNameForObject(deployment.Object()) if deployment.Spec.Template.Labels == nil { - deployment.Spec.Template.Labels = make(map[string]string, 1) + deployment.Spec.Template.Labels = make(map[string]string, 2) + } + if queueName != "" { + deployment.Spec.Template.Labels[constants.QueueLabel] = queueName } - deployment.Spec.Template.Labels[constants.QueueLabel] = queueName + deployment.Spec.Template.Labels[pod.SuspendedByParentLabelKey] = FrameworkName } return nil diff --git a/pkg/controller/jobs/deployment/deployment_webhook_test.go b/pkg/controller/jobs/deployment/deployment_webhook_test.go index 0cfe21c11d..4aec56f65d 100644 --- a/pkg/controller/jobs/deployment/deployment_webhook_test.go +++ b/pkg/controller/jobs/deployment/deployment_webhook_test.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/kueue/pkg/cache" "sigs.k8s.io/kueue/pkg/controller/jobframework" + "sigs.k8s.io/kueue/pkg/controller/jobs/pod" "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/queue" utiltesting "sigs.k8s.io/kueue/pkg/util/testing" @@ -51,6 +52,7 @@ func TestDefault(t *testing.T) { want: testingdeployment.MakeDeployment("test-pod", ""). Queue("test-queue"). PodTemplateSpecQueue("test-queue"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). Obj(), }, "deployment with queue and pod template spec queue": { @@ -61,6 +63,7 @@ func TestDefault(t *testing.T) { want: testingdeployment.MakeDeployment("test-pod", ""). Queue("new-test-queue"). PodTemplateSpecQueue("new-test-queue"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). Obj(), }, "deployment without queue with pod template spec queue": { @@ -74,6 +77,7 @@ func TestDefault(t *testing.T) { want: testingdeployment.MakeDeployment("test-pod", "default"). Queue("default"). PodTemplateSpecQueue("default"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). Obj(), }, "LocalQueueDefaulting enabled, default lq is created, job has queue label": { @@ -83,6 +87,7 @@ func TestDefault(t *testing.T) { want: testingdeployment.MakeDeployment("test-pod", ""). Queue("test-queue"). PodTemplateSpecQueue("test-queue"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). Obj(), }, "LocalQueueDefaulting enabled, default lq isn't created, job doesn't have queue label": { diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index 00b2a43844..664be375a6 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -47,6 +47,7 @@ const ( ManagedLabelKey = constants.ManagedByKueueLabel ManagedLabelValue = "true" PodFinalizer = ManagedLabelKey + SuspendedByParentLabelKey = "kueue.x-k8s.io/pod-suspending-parent" GroupNameLabel = "kueue.x-k8s.io/pod-group-name" GroupTotalCountAnnotation = "kueue.x-k8s.io/pod-group-total-count" GroupFastAdmissionAnnotation = "kueue.x-k8s.io/pod-group-fast-admission" @@ -158,10 +159,14 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { ) } log.V(5).Info("Found pod namespace", "Namespace.Name", ns.GetName()) - jobframework.ApplyDefaultLocalQueue(pod.Object(), w.queues.DefaultLocalQueueExist) - suspend, err := jobframework.WorkloadShouldBeSuspended(ctx, pod.Object(), w.client, w.manageJobsWithoutQueueName, w.managedJobsNamespaceSelector) - if err != nil { - return err + + _, suspend := pod.pod.GetLabels()[SuspendedByParentLabelKey] + if !suspend { + jobframework.ApplyDefaultLocalQueue(pod.Object(), w.queues.DefaultLocalQueueExist) + suspend, err = jobframework.WorkloadShouldBeSuspended(ctx, pod.Object(), w.client, w.manageJobsWithoutQueueName, w.managedJobsNamespaceSelector) + if err != nil { + return err + } } // Backwards compatibility support until podOptions.podSelector and podOptions.namespaceSelector are deprecated. diff --git a/pkg/controller/jobs/statefulset/statefulset_webhook.go b/pkg/controller/jobs/statefulset/statefulset_webhook.go index 30b0c42c55..8c4f7a965a 100644 --- a/pkg/controller/jobs/statefulset/statefulset_webhook.go +++ b/pkg/controller/jobs/statefulset/statefulset_webhook.go @@ -22,6 +22,7 @@ import ( appsv1 "k8s.io/api/apps/v1" apivalidation "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" @@ -38,17 +39,19 @@ import ( ) type Webhook struct { - client client.Client - queues *queue.Manager - manageJobsWithoutQueueName bool + client client.Client + manageJobsWithoutQueueName bool + managedJobsNamespaceSelector labels.Selector + queues *queue.Manager } func SetupWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error { options := jobframework.ProcessOptions(opts...) wh := &Webhook{ - client: mgr.GetClient(), - queues: options.Queues, - manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, + client: mgr.GetClient(), + manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName, + managedJobsNamespaceSelector: options.ManagedJobsNamespaceSelector, + queues: options.Queues, } return ctrl.NewWebhookManagedBy(mgr). For(&appsv1.StatefulSet{}). @@ -66,23 +69,29 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { log := ctrl.LoggerFrom(ctx).WithName("statefulset-webhook") log.V(5).Info("Propagating queue-name") - // Because StatefuleSet is built using a NoOpReconciler handling of jobs without queue names is delegating to the Pod webhook. jobframework.ApplyDefaultLocalQueue(ss.Object(), wh.queues.DefaultLocalQueueExist) - queueName := jobframework.QueueNameForObject(ss.Object()) - if queueName != "" { + suspend, err := jobframework.WorkloadShouldBeSuspended(ctx, ss.Object(), wh.client, wh.manageJobsWithoutQueueName, wh.managedJobsNamespaceSelector) + if err != nil { + return err + } + if suspend { if ss.Spec.Template.Labels == nil { - ss.Spec.Template.Labels = make(map[string]string, 2) + ss.Spec.Template.Labels = make(map[string]string, 3) } - ss.Spec.Template.Labels[constants.QueueLabel] = queueName - ss.Spec.Template.Labels[pod.GroupNameLabel] = GetWorkloadName(ss.Name) - - if ss.Spec.Template.Annotations == nil { - ss.Spec.Template.Annotations = make(map[string]string, 4) + queueName := jobframework.QueueNameForObject(ss.Object()) + if queueName != "" { + ss.Spec.Template.Labels[constants.QueueLabel] = queueName + ss.Spec.Template.Labels[pod.GroupNameLabel] = GetWorkloadName(ss.Name) + + if ss.Spec.Template.Annotations == nil { + ss.Spec.Template.Annotations = make(map[string]string, 4) + } + ss.Spec.Template.Annotations[pod.GroupTotalCountAnnotation] = fmt.Sprint(ptr.Deref(ss.Spec.Replicas, 1)) + ss.Spec.Template.Annotations[pod.GroupFastAdmissionAnnotation] = "true" + ss.Spec.Template.Annotations[pod.GroupServingAnnotation] = "true" + ss.Spec.Template.Annotations[kueuealpha.PodGroupPodIndexLabelAnnotation] = appsv1.PodIndexLabel } - ss.Spec.Template.Annotations[pod.GroupTotalCountAnnotation] = fmt.Sprint(ptr.Deref(ss.Spec.Replicas, 1)) - ss.Spec.Template.Annotations[pod.GroupFastAdmissionAnnotation] = "true" - ss.Spec.Template.Annotations[pod.GroupServingAnnotation] = "true" - ss.Spec.Template.Annotations[kueuealpha.PodGroupPodIndexLabelAnnotation] = appsv1.PodIndexLabel + ss.Spec.Template.Labels[pod.SuspendedByParentLabelKey] = FrameworkName } return nil diff --git a/pkg/controller/jobs/statefulset/statefulset_webhook_test.go b/pkg/controller/jobs/statefulset/statefulset_webhook_test.go index 95dc7f5a34..76a1079d2c 100644 --- a/pkg/controller/jobs/statefulset/statefulset_webhook_test.go +++ b/pkg/controller/jobs/statefulset/statefulset_webhook_test.go @@ -58,6 +58,7 @@ func TestDefault(t *testing.T) { Replicas(10). Queue("test-queue"). PodTemplateSpecQueue("test-queue"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). PodTemplateSpecPodGroupNameLabel("test-pod", "", gvk). PodTemplateSpecPodGroupTotalCountAnnotation(10). PodTemplateSpecPodGroupFastAdmissionAnnotation(true). @@ -75,6 +76,7 @@ func TestDefault(t *testing.T) { PodTemplateSpecPodGroupNameLabel("test-pod", "", gvk). PodTemplateSpecPodGroupTotalCountAnnotation(1). PodTemplateSpecQueue("test-queue"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). PodTemplateSpecPodGroupFastAdmissionAnnotation(true). PodTemplateSpecPodGroupServingAnnotation(true). PodTemplateSpecPodGroupPodIndexLabelAnnotation(appsv1.PodIndexLabel). @@ -87,6 +89,7 @@ func TestDefault(t *testing.T) { want: testingstatefulset.MakeStatefulSet("test-pod", "default"). Queue("default"). PodTemplateSpecQueue("default"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). PodTemplateSpecPodGroupNameLabel("test-pod", "", gvk). PodTemplateSpecPodGroupTotalCountAnnotation(1). PodTemplateSpecPodGroupFastAdmissionAnnotation(true). @@ -101,6 +104,7 @@ func TestDefault(t *testing.T) { want: testingstatefulset.MakeStatefulSet("test-pod", ""). Queue("test-queue"). PodTemplateSpecQueue("test-queue"). + PodTemplateSpecLabel(pod.SuspendedByParentLabelKey, FrameworkName). PodTemplateSpecPodGroupNameLabel("test-pod", "", gvk). PodTemplateSpecPodGroupTotalCountAnnotation(1). PodTemplateSpecPodGroupFastAdmissionAnnotation(true). From 80802922c0a7b1c6b850925b68dba65eb7497110 Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 12:19:18 -0500 Subject: [PATCH 3/8] fix typo in rbacs --- charts/kueue/templates/rbac/role.yaml | 9 +-------- config/components/rbac/role.yaml | 9 +-------- pkg/controller/jobframework/defaults.go | 2 +- 3 files changed, 3 insertions(+), 17 deletions(-) diff --git a/charts/kueue/templates/rbac/role.yaml b/charts/kueue/templates/rbac/role.yaml index 7d162ba521..5bc38b7e52 100644 --- a/charts/kueue/templates/rbac/role.yaml +++ b/charts/kueue/templates/rbac/role.yaml @@ -90,16 +90,9 @@ rules: - watch - apiGroups: - apps - resources: - - statefulsets - verbs: - - get - - list - - watch - - apiGroups: - - apps/v1 resources: - replicasets + - statefulsets verbs: - get - list diff --git a/config/components/rbac/role.yaml b/config/components/rbac/role.yaml index 3e79946bde..9d9308d9b0 100644 --- a/config/components/rbac/role.yaml +++ b/config/components/rbac/role.yaml @@ -90,15 +90,8 @@ rules: - apiGroups: - apps resources: - - statefulsets - verbs: - - get - - list - - watch -- apiGroups: - - apps/v1 - resources: - replicasets + - statefulsets verbs: - get - list diff --git a/pkg/controller/jobframework/defaults.go b/pkg/controller/jobframework/defaults.go index fb834eb2ed..6f96354fe7 100644 --- a/pkg/controller/jobframework/defaults.go +++ b/pkg/controller/jobframework/defaults.go @@ -42,7 +42,7 @@ func ApplyDefaultForSuspend(ctx context.Context, job GenericJob, k8sClient clien return nil } -// +kubebuilder:rbac:groups="apps/v1",resources=replicasets,verbs=get;list;watch +// +kubebuilder:rbac:groups="apps",resources=replicasets,verbs=get;list;watch // WorkloadShouldBeSuspended determines whether jobObj should be default suspended on creation func WorkloadShouldBeSuspended(ctx context.Context, jobObj client.Object, k8sClient client.Client, From 0837f473a9122e86438e145610b587d9733fc759 Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 16:52:53 -0500 Subject: [PATCH 4/8] adjust podwebhook warning for forced suspension --- pkg/controller/jobs/pod/pod_webhook.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index 664be375a6..d7f0295a83 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -288,8 +288,11 @@ func validateManagedLabel(pod *Pod) field.ErrorList { return allErrs } -// warningForPodManagedLabel returns a warning message if the pod has a managed label, and it's parent is managed by kueue +// warningForPodManagedLabel returns a warning message if the pod has a managed label, and it's parent is managed by Kueue and isn' the Deployment or StatefulSet integrations func warningForPodManagedLabel(p *Pod) string { + if _, ok := p.pod.GetLabels()[SuspendedByParentLabelKey]; ok { + return "" + } if managedLabel := p.pod.GetLabels()[ManagedLabelKey]; managedLabel == ManagedLabelValue && IsPodOwnerManagedByKueue(p) { return fmt.Sprintf("pod owner is managed by kueue, label '%s=%s' might lead to unexpected behaviour", ManagedLabelKey, ManagedLabelValue) From b6c7cb2960ec6072546728c4a09ec00731d7a433 Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 21:11:52 -0500 Subject: [PATCH 5/8] redesign special pod/deployment/statefulset ownership logic --- pkg/controller/jobframework/defaults.go | 16 +--- .../jobframework/integrationmanager.go | 4 + .../jobs/deployment/deployment_controller.go | 20 ++--- .../jobs/deployment/deployment_webhook.go | 4 +- pkg/controller/jobs/pod/pod_webhook.go | 76 +++++++++++++------ .../statefulset/statefulset_controller.go | 20 ++--- .../jobs/statefulset/statefulset_webhook.go | 2 +- 7 files changed, 75 insertions(+), 67 deletions(-) diff --git a/pkg/controller/jobframework/defaults.go b/pkg/controller/jobframework/defaults.go index 6f96354fe7..f4aab3efb7 100644 --- a/pkg/controller/jobframework/defaults.go +++ b/pkg/controller/jobframework/defaults.go @@ -20,7 +20,6 @@ import ( "context" "fmt" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -48,19 +47,8 @@ func ApplyDefaultForSuspend(ctx context.Context, job GenericJob, k8sClient clien func WorkloadShouldBeSuspended(ctx context.Context, jobObj client.Object, k8sClient client.Client, manageJobsWithoutQueueName bool, managedJobsNamespaceSelector labels.Selector) (bool, error) { // Do not default suspend a job whose owner is already managed by Kueue - if owner := metav1.GetControllerOf(jobObj); owner != nil { - if owner.Kind == "ReplicaSet" && owner.APIVersion == "apps/v1" { - // To avoid a full-blown integration for ReplicaSet, we go up one level of ownership here - rs := &appsv1.ReplicaSet{} - err := k8sClient.Get(ctx, client.ObjectKey{Name: owner.Name, Namespace: jobObj.GetNamespace()}, rs) - if err != nil { - return false, fmt.Errorf("failed to get replicaset: %w", err) - } - owner = metav1.GetControllerOf(rs) - } - if owner != nil && IsOwnerManagedByKueue(owner) { - return false, nil - } + if owner := metav1.GetControllerOf(jobObj); owner != nil && IsOwnerManagedByKueue(owner) { + return false, nil } // Jobs with queue names whose parents are not managed by Kueue are default suspended diff --git a/pkg/controller/jobframework/integrationmanager.go b/pkg/controller/jobframework/integrationmanager.go index 7418eb02fe..fdb661197a 100644 --- a/pkg/controller/jobframework/integrationmanager.go +++ b/pkg/controller/jobframework/integrationmanager.go @@ -263,6 +263,10 @@ func GetIntegration(name string) (IntegrationCallbacks, bool) { return manager.get(name) } +func IsIntegrationEnabled(name string) bool { + return manager.enabledIntegrations.Has(name) +} + // GetIntegrationByGVK looks-up the framework identified by GroupVersionKind in the currently // registered list of frameworks returning its callbacks and true if found. func GetIntegrationByGVK(gvk schema.GroupVersionKind) (IntegrationCallbacks, bool) { diff --git a/pkg/controller/jobs/deployment/deployment_controller.go b/pkg/controller/jobs/deployment/deployment_controller.go index 504b16b571..df3a139305 100644 --- a/pkg/controller/jobs/deployment/deployment_controller.go +++ b/pkg/controller/jobs/deployment/deployment_controller.go @@ -20,7 +20,6 @@ import ( "context" appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -39,14 +38,13 @@ const ( func init() { utilruntime.Must(jobframework.RegisterIntegration(FrameworkName, jobframework.IntegrationCallbacks{ - SetupIndexes: SetupIndexes, - NewReconciler: jobframework.NewNoopReconcilerFactory(gvk), - GVK: gvk, - SetupWebhook: SetupWebhook, - JobType: &appsv1.Deployment{}, - AddToScheme: appsv1.AddToScheme, - DependencyList: []string{"pod"}, - IsManagingObjectsOwner: isDeployment, + SetupIndexes: SetupIndexes, + NewReconciler: jobframework.NewNoopReconcilerFactory(gvk), + GVK: gvk, + SetupWebhook: SetupWebhook, + JobType: &appsv1.Deployment{}, + AddToScheme: appsv1.AddToScheme, + DependencyList: []string{"pod"}, })) } @@ -67,7 +65,3 @@ func (d *Deployment) GVK() schema.GroupVersionKind { func SetupIndexes(context.Context, client.FieldIndexer) error { return nil } - -func isDeployment(owner *metav1.OwnerReference) bool { - return owner.Kind == "Deployment" && owner.APIVersion == gvk.GroupVersion().String() -} diff --git a/pkg/controller/jobs/deployment/deployment_webhook.go b/pkg/controller/jobs/deployment/deployment_webhook.go index 2002529583..93ddcbbf2a 100644 --- a/pkg/controller/jobs/deployment/deployment_webhook.go +++ b/pkg/controller/jobs/deployment/deployment_webhook.go @@ -74,14 +74,14 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { return err } if suspend { - queueName := jobframework.QueueNameForObject(deployment.Object()) if deployment.Spec.Template.Labels == nil { deployment.Spec.Template.Labels = make(map[string]string, 2) } + deployment.Spec.Template.Labels[pod.SuspendedByParentLabelKey] = FrameworkName + queueName := jobframework.QueueNameForObject(deployment.Object()) if queueName != "" { deployment.Spec.Template.Labels[constants.QueueLabel] = queueName } - deployment.Spec.Template.Labels[pod.SuspendedByParentLabelKey] = FrameworkName } return nil diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index d7f0295a83..e26b20c4bc 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,6 +37,7 @@ import ( configapi "sigs.k8s.io/kueue/apis/config/v1beta1" kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1" "sigs.k8s.io/kueue/pkg/constants" + ctrlconstants "sigs.k8s.io/kueue/pkg/controller/constants" "sigs.k8s.io/kueue/pkg/controller/jobframework" "sigs.k8s.io/kueue/pkg/controller/jobframework/webhook" "sigs.k8s.io/kueue/pkg/features" @@ -150,30 +152,19 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { log := ctrl.LoggerFrom(ctx).WithName("pod-webhook") log.V(5).Info("Applying defaults") - ns := corev1.Namespace{} - err := w.client.Get(ctx, client.ObjectKey{Name: pod.pod.GetNamespace()}, &ns) - if err != nil { - return fmt.Errorf("failed to run mutating webhook on pod %s, error while getting namespace: %w", - pod.pod.GetName(), - err, - ) - } - log.V(5).Info("Found pod namespace", "Namespace.Name", ns.GetName()) - _, suspend := pod.pod.GetLabels()[SuspendedByParentLabelKey] if !suspend { - jobframework.ApplyDefaultLocalQueue(pod.Object(), w.queues.DefaultLocalQueueExist) - suspend, err = jobframework.WorkloadShouldBeSuspended(ctx, pod.Object(), w.client, w.manageJobsWithoutQueueName, w.managedJobsNamespaceSelector) + // Namespace filtering + ns := corev1.Namespace{} + err := w.client.Get(ctx, client.ObjectKey{Name: pod.pod.GetNamespace()}, &ns) if err != nil { - return err + return fmt.Errorf("failed to get namespace: %w", err) + } + if features.Enabled(features.ManagedJobsNamespaceSelector) && !w.managedJobsNamespaceSelector.Matches(labels.Set(ns.GetLabels())) { + return nil } - } - // Backwards compatibility support until podOptions.podSelector and podOptions.namespaceSelector are deprecated. - // When WorkloadShouldBeSuspend determines that suspend is true, also run the podOptions based checks - // and if either of them exempts the Pod from suspension, we return early. - if suspend { - // podOptions.podSelector + // Backwards compatibility: podOptions.podSelector podSelector, err := metav1.LabelSelectorAsSelector(w.podSelector) if err != nil { return fmt.Errorf("failed to parse pod selector: %w", err) @@ -182,7 +173,7 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { return nil } - // podOptions.namespaceSelector + // Backwards compatibility: podOptions.namespaceSelector nsSelector, err := metav1.LabelSelectorAsSelector(w.namespaceSelector) if err != nil { return fmt.Errorf("failed to parse namespace selector: %w", err) @@ -190,6 +181,46 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { if !nsSelector.Matches(labels.Set(ns.GetLabels())) { return nil } + + // Do not suspend a Pod whose owner is already managed by Kueue + if owner := metav1.GetControllerOf(pod.Object()); owner != nil { + if jobframework.IsOwnerManagedByKueue(owner) { + return nil + } + // Special case for Deployment integration which intentionally does not register a IsManagingObjectsOwner + // so that it can use the standaloneJob path in the GenericReconciler for its Pods + if owner.Kind == "ReplicaSet" && owner.APIVersion == "apps/v1" { + rs := &appsv1.ReplicaSet{} + err := w.client.Get(ctx, client.ObjectKey{Name: owner.Name, Namespace: pod.pod.GetNamespace()}, rs) + if err != nil { + return fmt.Errorf("failed to get replicaset: %w", err) + } + owner = metav1.GetControllerOf(rs) + if owner != nil { + if owner.Kind == "Deployment" && owner.APIVersion == "apps/v1" && jobframework.IsIntegrationEnabled("deployment") { + return nil + } + } + } + // Special case for StatefulSet integration which intentionally does not register a IsManagingObjectsOwner + // so that it can use the standaloneJob path in the GenericReconciler for its Pods + if owner.Kind == "StatefulSet" && owner.APIVersion == "apps/v1" && jobframework.IsIntegrationEnabled("statefulset") { + return nil + } + } + + // Local queue defaulting + if features.Enabled(features.LocalQueueDefaulting) && + jobframework.QueueNameForObject(pod.Object()) == "" && + w.queues.DefaultLocalQueueExist(pod.pod.GetNamespace()) { + if pod.pod.Labels == nil { + pod.pod.Labels = make(map[string]string) + } + pod.pod.Labels[ctrlconstants.QueueLabel] = ctrlconstants.DefaultLocalQueueName + } + + suspend = jobframework.QueueNameForObject(pod.Object()) != "" || w.manageJobsWithoutQueueName + } if suspend { @@ -288,11 +319,8 @@ func validateManagedLabel(pod *Pod) field.ErrorList { return allErrs } -// warningForPodManagedLabel returns a warning message if the pod has a managed label, and it's parent is managed by Kueue and isn' the Deployment or StatefulSet integrations +// warningForPodManagedLabel returns a warning message if the pod has a managed label, and it's parent is managed by Kueue func warningForPodManagedLabel(p *Pod) string { - if _, ok := p.pod.GetLabels()[SuspendedByParentLabelKey]; ok { - return "" - } if managedLabel := p.pod.GetLabels()[ManagedLabelKey]; managedLabel == ManagedLabelValue && IsPodOwnerManagedByKueue(p) { return fmt.Sprintf("pod owner is managed by kueue, label '%s=%s' might lead to unexpected behaviour", ManagedLabelKey, ManagedLabelValue) diff --git a/pkg/controller/jobs/statefulset/statefulset_controller.go b/pkg/controller/jobs/statefulset/statefulset_controller.go index e1ae2ae087..da6a7a9fa3 100644 --- a/pkg/controller/jobs/statefulset/statefulset_controller.go +++ b/pkg/controller/jobs/statefulset/statefulset_controller.go @@ -20,7 +20,6 @@ import ( "context" appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -39,14 +38,13 @@ const ( func init() { utilruntime.Must(jobframework.RegisterIntegration(FrameworkName, jobframework.IntegrationCallbacks{ - SetupIndexes: SetupIndexes, - NewReconciler: NewReconciler, - SetupWebhook: SetupWebhook, - JobType: &appsv1.StatefulSet{}, - AddToScheme: appsv1.AddToScheme, - DependencyList: []string{"pod"}, - GVK: gvk, - IsManagingObjectsOwner: isStatefulSet, + SetupIndexes: SetupIndexes, + NewReconciler: NewReconciler, + SetupWebhook: SetupWebhook, + JobType: &appsv1.StatefulSet{}, + AddToScheme: appsv1.AddToScheme, + DependencyList: []string{"pod"}, + GVK: gvk, })) } @@ -67,7 +65,3 @@ func (d *StatefulSet) GVK() schema.GroupVersionKind { func SetupIndexes(context.Context, client.FieldIndexer) error { return nil } - -func isStatefulSet(owner *metav1.OwnerReference) bool { - return owner.Kind == "StatefulSet" && owner.APIVersion == gvk.GroupVersion().String() -} diff --git a/pkg/controller/jobs/statefulset/statefulset_webhook.go b/pkg/controller/jobs/statefulset/statefulset_webhook.go index 8c4f7a965a..b2cdf5ae90 100644 --- a/pkg/controller/jobs/statefulset/statefulset_webhook.go +++ b/pkg/controller/jobs/statefulset/statefulset_webhook.go @@ -78,6 +78,7 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { if ss.Spec.Template.Labels == nil { ss.Spec.Template.Labels = make(map[string]string, 3) } + ss.Spec.Template.Labels[pod.SuspendedByParentLabelKey] = FrameworkName queueName := jobframework.QueueNameForObject(ss.Object()) if queueName != "" { ss.Spec.Template.Labels[constants.QueueLabel] = queueName @@ -91,7 +92,6 @@ func (wh *Webhook) Default(ctx context.Context, obj runtime.Object) error { ss.Spec.Template.Annotations[pod.GroupServingAnnotation] = "true" ss.Spec.Template.Annotations[kueuealpha.PodGroupPodIndexLabelAnnotation] = appsv1.PodIndexLabel } - ss.Spec.Template.Labels[pod.SuspendedByParentLabelKey] = FrameworkName } return nil From 9ab2592f0bb07ded5393143ef6bcc6e62bf693a3 Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 21:17:53 -0500 Subject: [PATCH 6/8] revert trivial change --- pkg/controller/jobs/pod/pod_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index e26b20c4bc..d883905f83 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -319,7 +319,7 @@ func validateManagedLabel(pod *Pod) field.ErrorList { return allErrs } -// warningForPodManagedLabel returns a warning message if the pod has a managed label, and it's parent is managed by Kueue +// warningForPodManagedLabel returns a warning message if the pod has a managed label, and it's parent is managed by kueue func warningForPodManagedLabel(p *Pod) string { if managedLabel := p.pod.GetLabels()[ManagedLabelKey]; managedLabel == ManagedLabelValue && IsPodOwnerManagedByKueue(p) { return fmt.Sprintf("pod owner is managed by kueue, label '%s=%s' might lead to unexpected behaviour", From a733dfd0db321a753efd1774e5d2d3eb57a2aa3e Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 21:31:55 -0500 Subject: [PATCH 7/8] linter fix --- pkg/controller/jobs/pod/pod_webhook.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index d883905f83..436b04452a 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -220,7 +220,6 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { } suspend = jobframework.QueueNameForObject(pod.Object()) != "" || w.manageJobsWithoutQueueName - } if suspend { From 47a8aaf21ccb05090d393a26616db608c4cf7dfe Mon Sep 17 00:00:00 2001 From: David Grove Date: Tue, 10 Dec 2024 22:12:56 -0500 Subject: [PATCH 8/8] nit: grammar --- pkg/controller/jobs/pod/pod_webhook.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controller/jobs/pod/pod_webhook.go b/pkg/controller/jobs/pod/pod_webhook.go index 436b04452a..cc2ca1aa5d 100644 --- a/pkg/controller/jobs/pod/pod_webhook.go +++ b/pkg/controller/jobs/pod/pod_webhook.go @@ -187,8 +187,8 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { if jobframework.IsOwnerManagedByKueue(owner) { return nil } - // Special case for Deployment integration which intentionally does not register a IsManagingObjectsOwner - // so that it can use the standaloneJob path in the GenericReconciler for its Pods + // Special case for Deployment integration which intentionally does not register an IsManagingObjectsOwner + // function so that it can use the standaloneJob path in the GenericReconciler for its Pods. if owner.Kind == "ReplicaSet" && owner.APIVersion == "apps/v1" { rs := &appsv1.ReplicaSet{} err := w.client.Get(ctx, client.ObjectKey{Name: owner.Name, Namespace: pod.pod.GetNamespace()}, rs) @@ -202,8 +202,8 @@ func (w *PodWebhook) Default(ctx context.Context, obj runtime.Object) error { } } } - // Special case for StatefulSet integration which intentionally does not register a IsManagingObjectsOwner - // so that it can use the standaloneJob path in the GenericReconciler for its Pods + // Special case for StatefulSet integration which intentionally does not register an IsManagingObjectsOwner + // function so that it can use the standaloneJob path in the GenericReconciler for its Pods. if owner.Kind == "StatefulSet" && owner.APIVersion == "apps/v1" && jobframework.IsIntegrationEnabled("statefulset") { return nil }