diff --git a/pkg/controller.v1/common/job.go b/pkg/controller.v1/common/job.go index da67e8a6cb..ae53b4a9b7 100644 --- 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 index cdd34fdb19..712ff6028c 100644 --- 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 index 10682d0981..e59fd15d66 100644 --- 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 index 1f3cf2230f..b453f0eb8e 100644 --- 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 index 9af3e4527d..a5effc5cc5 100644 --- 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 index b14c33f36d..8447d02f55 100644 --- 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 index da3889ce02..4733515bbe 100644 --- 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 index a59ed7d32c..10cc0ea053 100644 --- 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 index 64ebc03251..7757e36b3e 100644 --- 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 index a666ddf53f..759cc1b58b 100644 --- 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 index c8b7ffd811..3c1d410598 100644 --- 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",