diff --git a/api/jobset/v1alpha2/jobset_webhook.go b/api/jobset/v1alpha2/jobset_webhook.go index 82a38a95..c1605bb6 100644 --- a/api/jobset/v1alpha2/jobset_webhook.go +++ b/api/jobset/v1alpha2/jobset_webhook.go @@ -63,10 +63,6 @@ func (js *JobSet) Default() { if js.Spec.Network.EnableDNSHostnames == nil { js.Spec.Network.EnableDNSHostnames = ptr.To(true) } - // Subdomain defaults to the JobSet name - if js.Spec.Network.Subdomain == "" { - js.Spec.Network.Subdomain = js.Name - } // Default pod restart policy to OnFailure. if js.Spec.ReplicatedJobs[i].Template.Spec.Template.Spec.RestartPolicy == "" { diff --git a/api/jobset/v1alpha2/jobset_webhook_test.go b/api/jobset/v1alpha2/jobset_webhook_test.go index 30c151f7..f5606a6f 100644 --- a/api/jobset/v1alpha2/jobset_webhook_test.go +++ b/api/jobset/v1alpha2/jobset_webhook_test.go @@ -138,49 +138,6 @@ func TestJobSetDefaulting(t *testing.T) { }, }, }, - { - name: "subdomain defaults to jobset name", - js: &JobSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "custom-jobset", - }, - Spec: JobSetSpec{ - SuccessPolicy: defaultSuccessPolicy, - ReplicatedJobs: []ReplicatedJob{ - { - Template: batchv1.JobTemplateSpec{ - Spec: batchv1.JobSpec{ - Template: TestPodTemplate, - CompletionMode: completionModePtr(batchv1.IndexedCompletion), - }, - }, - }, - }, - }, - }, - want: &JobSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "custom-jobset", - }, - Spec: JobSetSpec{ - SuccessPolicy: defaultSuccessPolicy, - Network: &Network{ - EnableDNSHostnames: ptr.To(true), - Subdomain: "custom-jobset", - }, - ReplicatedJobs: []ReplicatedJob{ - { - Template: batchv1.JobTemplateSpec{ - Spec: batchv1.JobSpec{ - Template: TestPodTemplate, - CompletionMode: completionModePtr(batchv1.IndexedCompletion), - }, - }, - }, - }, - }, - }, - }, { name: "enableDNSHostnames is false", js: &JobSet{ diff --git a/pkg/controllers/jobset_controller.go b/pkg/controllers/jobset_controller.go index 0d2a906f..1169a2e9 100644 --- a/pkg/controllers/jobset_controller.go +++ b/pkg/controllers/jobset_controller.go @@ -94,6 +94,14 @@ func (r *JobSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr ctx = ctrl.LoggerInto(ctx, log) log.V(2).Info("Reconciling JobSet") + // If enableDNSHostnames is set, and subdomain is unset, default the subdomain to be the JobSet name. + // This must be done in the controller rather than in the request-time defaulting, since if a JobSet + // uses generateName rather than setting the name explicitly, the JobSet name will still be an empty + // string at that time. + if dnsHostnamesEnabled(&js) && js.Spec.Network.Subdomain == "" { + js.Spec.Network.Subdomain = js.Name + } + // Get Jobs owned by JobSet. ownedJobs, err := r.getChildJobs(ctx, &js) if err != nil { @@ -363,11 +371,6 @@ func (r *JobSetReconciler) createJobs(ctx context.Context, js *jobset.JobSet, ow // If pod DNS hostnames are enabled, create a headless service for the JobSet if dnsHostnamesEnabled(js) { - - // Don't try to create a headless service without a subdomain - if js.Spec.Network.Subdomain == "" { - return fmt.Errorf("a subdomain should be set if dns hostnames are enabled") - } if err := r.createHeadlessSvcIfNotExist(ctx, js); err != nil { return err } diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index 7401de64..d78449d4 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -72,6 +72,14 @@ func (j *JobSetWrapper) SetAnnotations(annotations map[string]string) *JobSetWra return j } +// GenerateName sets the JobSet name. +func (j *JobSetWrapper) SetGenerateName(namePrefix string) *JobSetWrapper { + // Name and GenerateName are mutually exclusive, so we must unset the Name field. + j.Name = "" + j.GenerateName = namePrefix + return j +} + // Obj returns the inner JobSet. func (j *JobSetWrapper) Obj() *jobset.JobSet { return &j.JobSet diff --git a/test/integration/controller/jobset_controller_test.go b/test/integration/controller/jobset_controller_test.go index 083e9b85..7d006001 100644 --- a/test/integration/controller/jobset_controller_test.go +++ b/test/integration/controller/jobset_controller_test.go @@ -628,6 +628,20 @@ var _ = ginkgo.Describe("JobSet controller", func() { }, }, }), + ginkgo.Entry("jobset using generateName with enableDNSHostnames should have headless service name set to the jobset name", &testCase{ + makeJobSet: func(ns *corev1.Namespace) *testing.JobSetWrapper { + return testJobSet(ns).SetGenerateName("name-prefix").EnableDNSHostnames(true) + }, + updates: []*update{ + { + checkJobSetState: func(js *jobset.JobSet) { + gomega.Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: js.Name, Namespace: js.Namespace}, &corev1.Service{}) + }, timeout, interval).Should(gomega.Succeed()) + }, + }, + }, + }), ) // end of DescribeTable }) // end of Describe @@ -885,7 +899,6 @@ func testJobSet(ns *corev1.Namespace) *testing.JobSetWrapper { return testing.MakeJobSet(jobSetName, ns.Name). SuccessPolicy(&jobset.SuccessPolicy{Operator: jobset.OperatorAll, TargetReplicatedJobs: []string{}}). EnableDNSHostnames(true). - NetworkSubdomain(jobSetName). ReplicatedJob(testing.MakeReplicatedJob("replicated-job-a"). Job(testing.MakeJobTemplate("test-job-A", ns.Name).PodSpec(testing.TestPodSpec).Obj()). Replicas(1).