Get subdomain via a func instead of defaulting it on the jobset object

pkg/controllers/jobset_controller.go

@@ -94,14 +94,6 @@ 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 {
@@ -423,10 +415,11 @@ 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
-	if err := r.Get(ctx, types.NamespacedName{Name: js.Spec.Network.Subdomain, Namespace: js.Namespace}, &headlessSvc); err != nil {
+	subdomain := GetSubdomain(js)
+	if err := r.Get(ctx, types.NamespacedName{Name: subdomain, Namespace: js.Namespace}, &headlessSvc); err != nil {
 		headlessSvc := corev1.Service{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      js.Spec.Network.Subdomain,
+				Name:      subdomain,
 				Namespace: js.Namespace,
 			},
 			Spec: corev1.ServiceSpec{
@@ -586,7 +579,7 @@ func updateCondition(js *jobset.JobSet, condition metav1.Condition) bool {
 func constructJobsFromTemplate(js *jobset.JobSet, rjob *jobset.ReplicatedJob, ownedJobs *childJobs) ([]*batchv1.Job, error) {
 	var jobs []*batchv1.Job
 	for jobIdx := 0; jobIdx < int(rjob.Replicas); jobIdx++ {
-		jobName := placement.GenJobName(js.Name, rjob.Name, jobIdx)
+		jobName := placement.GetJobName(js.Name, rjob.Name, jobIdx)
 		if create := shouldCreateJob(jobName, ownedJobs); !create {
 			continue
 		}
@@ -616,7 +609,7 @@ func constructJob(js *jobset.JobSet, rjob *jobset.ReplicatedJob, jobIdx int) (*b
 	// If enableDNSHostnames is set, update job spec to set subdomain as
 	// job name (a headless service with same name as job will be created later).
 	if dnsHostnamesEnabled(js) {
-		job.Spec.Template.Spec.Subdomain = js.Spec.Network.Subdomain
+		job.Spec.Template.Spec.Subdomain = GetSubdomain(js)
 	}
 
 	// If this job is using the nodeSelectorStrategy implementation of exclusive placement,
@@ -707,8 +700,11 @@ func JobFinished(job *batchv1.Job) (bool, batchv1.JobConditionType) {
 	return false, ""
 }
 
-func GenSubdomain(js *jobset.JobSet) string {
-	// If we have selected an explicit network name, use it
+func GetSubdomain(js *jobset.JobSet) string {
+	// 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 js.Spec.Network.Subdomain != "" {
 		return js.Spec.Network.Subdomain
 	}

test/integration/controller/jobset_controller_test.go

@@ -589,7 +589,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
 					// Fetch headless service created for replicated job and delete it.
 					jobSetUpdateFn: func(js *jobset.JobSet) {
 						var svc corev1.Service
-						gomega.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: controllers.GenSubdomain(js), Namespace: js.Namespace}, &svc)).To(gomega.Succeed())
+						gomega.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: controllers.GetSubdomain(js), Namespace: js.Namespace}, &svc)).To(gomega.Succeed())
 						gomega.Expect(k8sClient.Delete(ctx, &svc)).To(gomega.Succeed())
 					},
 					// Service should be recreated during reconciliation.