Avoid making copy of runPolicy

Signed-off-by: Michal Szadkowski <[email protected]>
kubeflow · Sep 12, 2024 · 80576f6 · 80576f6
1 parent 353d505
commit 80576f6
Show file tree

Hide file tree

Showing 11 changed files with 13 additions and 13 deletions.
diff --git a/pkg/controller.v1/common/job.go b/pkg/controller.v1/common/job.go
@@ -456,8 +456,8 @@ func (jc *JobController) calcPGMinResources(minMember int32, replicas map[apiv1.
 	return CalcPGMinResources(minMember, replicas, jc.PriorityClassLister.Get)
 }
 
-func (jc *JobController) ManagedByExternalController(rp apiv1.RunPolicy) *string {
-	if controllerName := rp.ManagedBy; controllerName != nil && *controllerName != apiv1.KubeflowJobsController {
+func (jc *JobController) ManagedByExternalController(controllerName *string) *string {
+	if controllerName != nil && *controllerName != apiv1.KubeflowJobsController {
 		return controllerName
 	}
 	return nil

diff --git a/pkg/controller.v1/common/job_test.go b/pkg/controller.v1/common/job_test.go
@@ -253,7 +253,7 @@ func TestManagedByExternalController(T *testing.T) {
 				ManagedBy: tc.managedBy,
 			}
 
-			if got := jobController.ManagedByExternalController(*runPolicy); got != nil {
+			if got := jobController.ManagedByExternalController(runPolicy.ManagedBy); got != nil {
 				if !tc.wantResult {
 					t.Errorf("Unwanted manager controller: %s\n", *got)
 				}

diff --git a/pkg/controller.v1/mpi/mpijob_controller.go b/pkg/controller.v1/mpi/mpijob_controller.go
@@ -135,7 +135,7 @@ func (jc *MPIJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if manager := jc.ManagedByExternalController(mpijob.Spec.RunPolicy); manager != nil {
+	if manager := jc.ManagedByExternalController(mpijob.Spec.RunPolicy.ManagedBy); manager != nil {
 		logger.Info("Skipping MPIJob managed by a custom controller", "managed-by", manager)
 		return ctrl.Result{}, nil
 	}

diff --git a/pkg/controller.v1/paddlepaddle/paddlepaddle_controller.go b/pkg/controller.v1/paddlepaddle/paddlepaddle_controller.go
@@ -131,7 +131,7 @@ func (r *PaddleJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if manager := r.ManagedByExternalController(paddlejob.Spec.RunPolicy); manager != nil {
+	if manager := r.ManagedByExternalController(paddlejob.Spec.RunPolicy.ManagedBy); manager != nil {
 		logger.Info("Skipping PaddleJob managed by a custom controller", "managed-by", manager)
 		return ctrl.Result{}, nil
 	}

diff --git a/pkg/controller.v1/pytorch/pytorchjob_controller.go b/pkg/controller.v1/pytorch/pytorchjob_controller.go
@@ -132,7 +132,7 @@ func (r *PyTorchJobReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if manager := r.ManagedByExternalController(pytorchjob.Spec.RunPolicy); manager != nil {
+	if manager := r.ManagedByExternalController(pytorchjob.Spec.RunPolicy.ManagedBy); manager != nil {
 		logger.Info("Skipping PyTorchJob managed by a custom controller", "managed-by", manager)
 		return ctrl.Result{}, nil
 	}

diff --git a/pkg/controller.v1/tensorflow/tfjob_controller.go b/pkg/controller.v1/tensorflow/tfjob_controller.go
@@ -127,7 +127,7 @@ func (r *TFJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if manager := r.ManagedByExternalController(tfjob.Spec.RunPolicy); manager != nil {
+	if manager := r.ManagedByExternalController(tfjob.Spec.RunPolicy.ManagedBy); manager != nil {
 		logger.Info("Skipping TFJob managed by a custom controller", "managed-by", manager)
 		return ctrl.Result{}, nil
 	}

diff --git a/pkg/controller.v1/xgboost/xgboostjob_controller.go b/pkg/controller.v1/xgboost/xgboostjob_controller.go
@@ -130,7 +130,7 @@ func (r *XGBoostJobReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if manager := r.ManagedByExternalController(xgboostjob.Spec.RunPolicy); manager != nil {
+	if manager := r.ManagedByExternalController(xgboostjob.Spec.RunPolicy.ManagedBy); manager != nil {
 		logger.Info("Skipping XGBoostJob managed by a custom controller", "managed-by", manager)
 		return ctrl.Result{}, nil
 	}

diff --git a/pkg/webhooks/paddlepaddle/paddlepaddle_webhook_test.go b/pkg/webhooks/paddlepaddle/paddlepaddle_webhook_test.go
@@ -173,7 +173,7 @@ func TestValidateV1PaddleJob(t *testing.T) {
 				field.Required(paddleReplicaSpecPath, ""),
 			},
 		},
-		"unsupported managedBy controller name": {
+		"attempt to set unsupported managedBy controller name gets rejected": {
 			paddleJob: &trainingoperator.PaddleJob{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",

diff --git a/pkg/webhooks/pytorch/pytorchjob_webhook_test.go b/pkg/webhooks/pytorch/pytorchjob_webhook_test.go
@@ -290,7 +290,7 @@ func TestValidateV1PyTorchJob(t *testing.T) {
 					specPath.Child("elasticPolicy").Child("nProcPerNode"), specPath.Child("nprocPerNode")),
 			},
 		},
-		"unsupported managedBy controller name": {
+		"attempt to set unsupported managedBy controller name gets rejected": {
 			pytorchJob: &trainingoperator.PyTorchJob{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",
@@ -308,7 +308,7 @@ func TestValidateV1PyTorchJob(t *testing.T) {
 					trainingoperator.KubeflowJobsController))),
 			},
 		},
-		"managedBy field becomes mutable": {
+		"attempt to update the managedBy field gets rejected": {
 			oldPytorchJob: &trainingoperator.PyTorchJob{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",

diff --git a/pkg/webhooks/tensorflow/tfjob_webhook_test.go b/pkg/webhooks/tensorflow/tfjob_webhook_test.go
@@ -184,7 +184,7 @@ func TestValidateTFJob(t *testing.T) {
 				field.Forbidden(tfReplicaSpecPath, ""),
 			},
 		},
-		"unsupported managedBy controller name": {
+		"attempt to set unsupported managedBy controller name gets rejected": {
 			tfJob: &trainingoperator.TFJob{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",

diff --git a/pkg/webhooks/xgboost/xgboostjob_webhook_test.go b/pkg/webhooks/xgboost/xgboostjob_webhook_test.go
@@ -235,7 +235,7 @@ func TestValidateXGBoostJob(t *testing.T) {
 				field.Required(xgbReplicaSpecPath.Key(string(trainingoperator.XGBoostJobReplicaTypeMaster)), ""),
 			},
 		},
-		"unsupported managedBy controller name": {
+		"attempt to set unsupported managedBy controller name gets rejected": {
 			xgboostJob: &trainingoperator.XGBoostJob{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",