force setting subdomain in webhook

I do not like this design - I do not think webhooks are
reliable and it is added unneeded complexity to the
testing, but I have been asked to implement it this way.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

api/jobset/v1alpha2/jobset_types.go

@@ -133,7 +133,6 @@ type Network struct {
 	// EnableDNSHostnames allows pods to be reached via their hostnames.
 	// Pods will be reachable using the fully qualified pod hostname:
 	// <jobSet.name>-<spec.replicatedJob.name>-<job-index>-<pod-index>.<subdomain>
-	// Child jobs will share the same subdomain whether or not Subdomain is set.
 	// +optional
 	EnableDNSHostnames *bool `json:"enableDNSHostnames,omitempty"`
 

api/jobset/v1alpha2/jobset_webhook_test.go

@@ -341,7 +341,6 @@ func TestJobSetDefaulting(t *testing.T) {
 			},
 		},
 	}
-
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			tc.js.Default()

config/components/crd/bases/jobset.x-k8s.io_jobsets.yaml

@@ -66,9 +66,7 @@ spec:
                   enableDNSHostnames:
                     description: 'EnableDNSHostnames allows pods to be reached via
                       their hostnames. Pods will be reachable using the fully qualified
-                      pod hostname: <jobSet.name>-<spec.replicatedJob.name>-<job-index>-<pod-index>.<subdomain>
-                      Child jobs will share the same subdomain whether or not Subdomain
-                      is set.'
+                      pod hostname: <jobSet.name>-<spec.replicatedJob.name>-<job-index>-<pod-index>.<subdomain>'
                     type: boolean
                   subdomain:
                     description: Subdomain is an explicit choice for a network subdomain

pkg/controllers/jobset_controller.go

@@ -389,11 +389,10 @@ func (r *JobSetReconciler) createHeadlessSvcIfNotExist(ctx context.Context, js *
 	// Spec.Network.Subdomain, with default of <jobSetName> set by the webhook.
 	// If the service doesn't exist in the same namespace, create it.
 	var headlessSvc corev1.Service
-	subdomain := GenSubdomain(js)
-	if err := r.Get(ctx, types.NamespacedName{Name: subdomain, Namespace: js.Namespace}, &headlessSvc); err != nil {
+	if err := r.Get(ctx, types.NamespacedName{Name: js.Spec.Network.Subdomain, Namespace: js.Namespace}, &headlessSvc); err != nil {
 		headlessSvc := corev1.Service{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      subdomain,
+				Name:      js.Spec.Network.Subdomain,
 				Namespace: js.Namespace,
 			},
 			Spec: corev1.ServiceSpec{
@@ -575,11 +574,9 @@ func constructJob(js *jobset.JobSet, rjob *jobset.ReplicatedJob, jobIdx int) (*b
 	labelAndAnnotateObject(job, js, rjob, jobIdx)
 	labelAndAnnotateObject(&job.Spec.Template, js, rjob, jobIdx)
 
-	// If enableDNSHostnames is set, update job spec to set subdomain as
-	// the chosen subdomain name or the default name (i.e. <jobsetName>).
-	// A headless service with this name will be created if does not exist.
-	if dnsHostnamesEnabled(js) {
-		job.Spec.Template.Spec.Subdomain = GenSubdomain(js)
+	// If enableDNSHostnames is set, a Subdomain should be set
+	if dnsHostnamesEnabled(js) && js.Spec.Network.Subdomain == "" {
+		return job, fmt.Errorf("a subdomain should be set if dns hostnames are enabled")
 	}
 
 	// If this job should be exclusive per topology, set the pod affinities/anti-affinities accordingly.

pkg/controllers/jobset_controller_test.go

@@ -524,8 +524,10 @@ func TestConstructJobsFromTemplate(t *testing.T) {
 			name: "pod dns hostnames enabled",
 			js: testutils.MakeJobSet(jobSetName, ns).
 				EnableDNSHostnames(true).
+				Subdomain(jobSetName).
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
+					Subdomain(jobSetName).
 					Replicas(1).
 					Obj()).
 				Obj(),
@@ -539,16 +541,18 @@ func TestConstructJobsFromTemplate(t *testing.T) {
 					replicas:          1,
 					jobIdx:            0}).
 					Suspend(false).
-					Subdomain("test-jobset").Obj(),
+					Subdomain(jobSetName).Obj(),
 			},
 		},
 		{
 			name: "suspend job set",
 			js: testutils.MakeJobSet(jobSetName, ns).
 				Suspend(true).
 				EnableDNSHostnames(true).
+				Subdomain(jobSetName).
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
+					Subdomain(jobSetName).
 					Replicas(1).
 					Obj()).
 				Obj(),
@@ -562,16 +566,18 @@ func TestConstructJobsFromTemplate(t *testing.T) {
 					replicas:          1,
 					jobIdx:            0}).
 					Suspend(true).
-					Subdomain("test-jobset").Obj(),
+					Subdomain(jobSetName).Obj(),
 			},
 		},
 		{
 			name: "resume job set",
 			js: testutils.MakeJobSet(jobSetName, ns).
 				Suspend(false).
 				EnableDNSHostnames(true).
+				Subdomain(jobSetName).
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
+					Subdomain(jobSetName).
 					Replicas(1).
 					Obj()).
 				Obj(),
@@ -585,7 +591,7 @@ func TestConstructJobsFromTemplate(t *testing.T) {
 					replicas:          1,
 					jobIdx:            0}).
 					Suspend(false).
-					Subdomain("test-jobset").Obj(),
+					Subdomain(jobSetName).Obj(),
 			},
 		},
 	}

pkg/util/testing/wrappers.go

@@ -72,6 +72,12 @@ func (j *JobSetWrapper) SetAnnotations(annotations map[string]string) *JobSetWra
 	return j
 }
 
+// Subdomain sets the JobSet Network subdomain.
+func (j *JobSetWrapper) Subdomain(subdomain string) *JobSetWrapper {
+	j.Spec.Network.Subdomain = subdomain
+	return j
+}
+
 // Obj returns the inner JobSet.
 func (j *JobSetWrapper) Obj() *jobset.JobSet {
 	return &j.JobSet
@@ -127,6 +133,13 @@ func (r *ReplicatedJobWrapper) Replicas(val int) *ReplicatedJobWrapper {
 	return r
 }
 
+// Subdomain sets the subdomain on the PodSpec
+// We artificially do this because the webhook does not work in testing
+func (r *ReplicatedJobWrapper) Subdomain(subdomain string) *ReplicatedJobWrapper {
+	r.Template.Spec.Template.Spec.Subdomain = subdomain
+	return r
+}
+
 // Obj returns the inner ReplicatedJob.
 func (r *ReplicatedJobWrapper) Obj() jobset.ReplicatedJob {
 	return r.ReplicatedJob