Make the cleanPodPolicyPointer function global (#1858)

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
kubeflow · Jul 11, 2023 · a3a2972 · a3a2972
1 parent 9e084ff
commit a3a2972
Show file tree

Hide file tree

Showing 14 changed files with 22 additions and 37 deletions.
diff --git a/pkg/apis/kubeflow.org/v1/defaulting_utils.go b/pkg/apis/kubeflow.org/v1/defaulting_utils.go
@@ -58,6 +58,6 @@ func setTypeNameToCamelCase(replicaSpecs map[ReplicaType]*ReplicaSpec, typ Repli
 	}
 }
 
-func cleanPodPolicyPointer(cleanPodPolicy CleanPodPolicy) *CleanPodPolicy {
+func CleanPodPolicyPointer(cleanPodPolicy CleanPodPolicy) *CleanPodPolicy {
 	return &cleanPodPolicy
 }
diff --git a/pkg/apis/kubeflow.org/v1/mpi_defaults.go b/pkg/apis/kubeflow.org/v1/mpi_defaults.go
@@ -25,9 +25,8 @@ func addMPIJobDefaultingFuncs(scheme *runtime.Scheme) error {
 func SetDefaults_MPIJob(mpiJob *MPIJob) {
 	// Set default CleanPodPolicy to None when neither fields specified.
 	if mpiJob.Spec.CleanPodPolicy == nil && mpiJob.Spec.RunPolicy.CleanPodPolicy == nil {
-		none := CleanPodPolicyNone
-		mpiJob.Spec.CleanPodPolicy = &none
-		mpiJob.Spec.RunPolicy.CleanPodPolicy = &none
+		mpiJob.Spec.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
+		mpiJob.Spec.RunPolicy.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
 	}
 
 	// Set default replicas

diff --git a/pkg/apis/kubeflow.org/v1/mpi_defaults_test.go b/pkg/apis/kubeflow.org/v1/mpi_defaults_test.go
@@ -51,7 +51,6 @@ func expectedMPIJob(cleanPodPolicy CleanPodPolicy, restartPolicy RestartPolicy)
 
 func TestSetDefaults_MPIJob(t *testing.T) {
 	customRestartPolicy := RestartPolicyAlways
-	customCleanPodPolicy := CleanPodPolicyRunning
 
 	testCases := map[string]struct {
 		original *MPIJob
@@ -60,9 +59,9 @@ func TestSetDefaults_MPIJob(t *testing.T) {
 		"set default replicas": {
 			original: &MPIJob{
 				Spec: MPIJobSpec{
-					CleanPodPolicy: &customCleanPodPolicy,
+					CleanPodPolicy: CleanPodPolicyPointer(CleanPodPolicyRunning),
 					RunPolicy: RunPolicy{
-						CleanPodPolicy: &customCleanPodPolicy,
+						CleanPodPolicy: CleanPodPolicyPointer(CleanPodPolicyRunning),
 					},
 					MPIReplicaSpecs: map[ReplicaType]*ReplicaSpec{
 						MPIJobReplicaTypeLauncher: {
@@ -94,7 +93,7 @@ func TestSetDefaults_MPIJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedMPIJob(customCleanPodPolicy, customRestartPolicy),
+			expected: expectedMPIJob(CleanPodPolicyRunning, customRestartPolicy),
 		},
 		"set default clean pod policy": {
 			original: &MPIJob{

diff --git a/pkg/apis/kubeflow.org/v1/mxnet_defaults.go b/pkg/apis/kubeflow.org/v1/mxnet_defaults.go
@@ -47,8 +47,7 @@ func setMXNetTypeNamesToCamelCase(mxJob *MXJob) {
 func SetDefaults_MXJob(mxjob *MXJob) {
 	// Set default cleanpod policy to None.
 	if mxjob.Spec.RunPolicy.CleanPodPolicy == nil {
-		all := CleanPodPolicyNone
-		mxjob.Spec.RunPolicy.CleanPodPolicy = &all
+		mxjob.Spec.RunPolicy.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
 	}
 
 	// Update the key of MXReplicaSpecs to camel case.

diff --git a/pkg/apis/kubeflow.org/v1/mxnet_defaults_test.go b/pkg/apis/kubeflow.org/v1/mxnet_defaults_test.go
@@ -202,7 +202,7 @@ func TestSetDefaults_MXJob(t *testing.T) {
 			original: &MXJob{
 				Spec: MXJobSpec{
 					RunPolicy: RunPolicy{
-						CleanPodPolicy: cleanPodPolicyPointer(CleanPodPolicyAll),
+						CleanPodPolicy: CleanPodPolicyPointer(CleanPodPolicyAll),
 					},
 					MXReplicaSpecs: map[ReplicaType]*ReplicaSpec{
 						MXJobReplicaTypeWorker: &ReplicaSpec{

diff --git a/pkg/apis/kubeflow.org/v1/paddlepaddle_defautls.go b/pkg/apis/kubeflow.org/v1/paddlepaddle_defautls.go
@@ -65,8 +65,7 @@ func setPaddleTypeNamesToCamelCase(paddleJob *PaddleJob) {
 func SetDefaults_PaddleJob(job *PaddleJob) {
 	// Set default cleanpod policy to None.
 	if job.Spec.RunPolicy.CleanPodPolicy == nil {
-		policy := CleanPodPolicyNone
-		job.Spec.RunPolicy.CleanPodPolicy = &policy
+		job.Spec.RunPolicy.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
 	}
 
 	// Update the key of PaddleReplicaSpecs to camel case.

diff --git a/pkg/apis/kubeflow.org/v1/pytorch_defaults.go b/pkg/apis/kubeflow.org/v1/pytorch_defaults.go
@@ -65,8 +65,7 @@ func setPytorchTypeNamesToCamelCase(pytorchJob *PyTorchJob) {
 func SetDefaults_PyTorchJob(job *PyTorchJob) {
 	// Set default cleanpod policy to None.
 	if job.Spec.RunPolicy.CleanPodPolicy == nil {
-		policy := CleanPodPolicyNone
-		job.Spec.RunPolicy.CleanPodPolicy = &policy
+		job.Spec.RunPolicy.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
 	}
 
 	// Update the key of PyTorchReplicaSpecs to camel case.

diff --git a/pkg/apis/kubeflow.org/v1/tensorflow_defaults.go b/pkg/apis/kubeflow.org/v1/tensorflow_defaults.go
@@ -50,8 +50,7 @@ func setTensorflowTypeNamesToCamelCase(tfJob *TFJob) {
 func SetDefaults_TFJob(tfJob *TFJob) {
 	// Set default cleanpod policy to None.
 	if tfJob.Spec.RunPolicy.CleanPodPolicy == nil {
-		running := CleanPodPolicyNone
-		tfJob.Spec.RunPolicy.CleanPodPolicy = &running
+		tfJob.Spec.RunPolicy.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
 	}
 	// Set default success policy to "".
 	if tfJob.Spec.SuccessPolicy == nil {

diff --git a/pkg/apis/kubeflow.org/v1/tensorflow_defaults_test.go b/pkg/apis/kubeflow.org/v1/tensorflow_defaults_test.go
@@ -233,7 +233,7 @@ func TestSetDefaultTFJob(t *testing.T) {
 			original: &TFJob{
 				Spec: TFJobSpec{
 					RunPolicy: RunPolicy{
-						CleanPodPolicy: cleanPodPolicyPointer(CleanPodPolicyAll),
+						CleanPodPolicy: CleanPodPolicyPointer(CleanPodPolicyAll),
 					},
 					TFReplicaSpecs: map[ReplicaType]*ReplicaSpec{
 						TFJobReplicaTypeWorker: &ReplicaSpec{

diff --git a/pkg/apis/kubeflow.org/v1/xgboost_defaults.go b/pkg/apis/kubeflow.org/v1/xgboost_defaults.go
@@ -46,8 +46,7 @@ func setXGBoostJobTypeNamesToCamelCase(xgboostJob *XGBoostJob) {
 func SetDefaults_XGBoostJob(xgboostJob *XGBoostJob) {
 	// Set default cleanpod policy to None.
 	if xgboostJob.Spec.RunPolicy.CleanPodPolicy == nil {
-		all := CleanPodPolicyNone
-		xgboostJob.Spec.RunPolicy.CleanPodPolicy = &all
+		xgboostJob.Spec.RunPolicy.CleanPodPolicy = CleanPodPolicyPointer(CleanPodPolicyNone)
 	}
 
 	// Update the key of MXReplicaSpecs to camel case.

diff --git a/pkg/apis/kubeflow.org/v1/xgboost_defaults_test.go b/pkg/apis/kubeflow.org/v1/xgboost_defaults_test.go
@@ -201,7 +201,7 @@ func TestSetDefaults_XGBoostJob(t *testing.T) {
 			original: &XGBoostJob{
 				Spec: XGBoostJobSpec{
 					RunPolicy: RunPolicy{
-						CleanPodPolicy: cleanPodPolicyPointer(CleanPodPolicyAll),
+						CleanPodPolicy: CleanPodPolicyPointer(CleanPodPolicyAll),
 					},
 					XGBReplicaSpecs: map[ReplicaType]*ReplicaSpec{
 						XGBoostJobReplicaTypeWorker: &ReplicaSpec{

diff --git a/pkg/controller.v1/mpi/mpijob_controller_test.go b/pkg/controller.v1/mpi/mpijob_controller_test.go
@@ -39,7 +39,6 @@ const (
 )
 
 func newMPIJobCommon(name string, startTime, completionTime *metav1.Time) *kubeflowv1.MPIJob {
-	cleanPodPolicyAll := common.CleanPodPolicyAll
 	mpiJob := &kubeflowv1.MPIJob{
 		TypeMeta: metav1.TypeMeta{APIVersion: kubeflowv1.SchemeGroupVersion.String()},
 		ObjectMeta: metav1.ObjectMeta{
@@ -48,7 +47,7 @@ func newMPIJobCommon(name string, startTime, completionTime *metav1.Time) *kubef
 		},
 		Spec: kubeflowv1.MPIJobSpec{
 			RunPolicy: common.RunPolicy{
-				CleanPodPolicy: &cleanPodPolicyAll,
+				CleanPodPolicy: kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyAll),
 			},
 			MPIReplicaSpecs: map[common.ReplicaType]*common.ReplicaSpec{
 				kubeflowv1.MPIJobReplicaTypeWorker: {

diff --git a/pkg/controller.v1/tensorflow/testutil/tfjob.go b/pkg/controller.v1/tensorflow/testutil/tfjob.go
@@ -38,29 +38,25 @@ func NewTFJobWithCleanupJobDelay(chief, worker, ps int, ttl *int32) *kubeflowv1.
 	if chief == 1 {
 		tfJob := NewTFJobWithChief(worker, ps)
 		tfJob.Spec.RunPolicy.TTLSecondsAfterFinished = ttl
-		policy := kubeflowv1.CleanPodPolicyNone
-		tfJob.Spec.RunPolicy.CleanPodPolicy = &policy
+		tfJob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyNone)
 		return tfJob
 	}
 	tfJob := NewTFJob(worker, ps)
 	tfJob.Spec.RunPolicy.TTLSecondsAfterFinished = ttl
-	policy := kubeflowv1.CleanPodPolicyNone
-	tfJob.Spec.RunPolicy.CleanPodPolicy = &policy
+	tfJob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyNone)
 	return tfJob
 }
 
 func NewTFJobWithActiveDeadlineSeconds(chief, worker, ps int, ads *int64) *kubeflowv1.TFJob {
 	if chief == 1 {
 		tfJob := NewTFJobWithChief(worker, ps)
 		tfJob.Spec.RunPolicy.ActiveDeadlineSeconds = ads
-		policy := kubeflowv1.CleanPodPolicyAll
-		tfJob.Spec.RunPolicy.CleanPodPolicy = &policy
+		tfJob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyAll)
 		return tfJob
 	}
 	tfJob := NewTFJob(worker, ps)
 	tfJob.Spec.RunPolicy.ActiveDeadlineSeconds = ads
-	policy := kubeflowv1.CleanPodPolicyAll
-	tfJob.Spec.RunPolicy.CleanPodPolicy = &policy
+	tfJob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyAll)
 	return tfJob
 }
 
@@ -69,15 +65,13 @@ func NewTFJobWithBackoffLimit(chief, worker, ps int, backoffLimit *int32) *kubef
 		tfJob := NewTFJobWithChief(worker, ps)
 		tfJob.Spec.RunPolicy.BackoffLimit = backoffLimit
 		tfJob.Spec.TFReplicaSpecs["Worker"].RestartPolicy = "OnFailure"
-		policy := kubeflowv1.CleanPodPolicyAll
-		tfJob.Spec.RunPolicy.CleanPodPolicy = &policy
+		tfJob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyAll)
 		return tfJob
 	}
 	tfJob := NewTFJob(worker, ps)
 	tfJob.Spec.RunPolicy.BackoffLimit = backoffLimit
 	tfJob.Spec.TFReplicaSpecs["Worker"].RestartPolicy = "OnFailure"
-	policy := kubeflowv1.CleanPodPolicyAll
-	tfJob.Spec.RunPolicy.CleanPodPolicy = &policy
+	tfJob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyAll)
 	return tfJob
 }
 

diff --git a/test_job/apis/test_job/v1/defaults.go b/test_job/apis/test_job/v1/defaults.go
@@ -101,8 +101,7 @@ func SetDefaults_TestJob(testjob *TestJob) {
 
 	// Set default cleanpod policy to Running.
 	if testjob.Spec.RunPolicy.CleanPodPolicy == nil {
-		running := kubeflowv1.CleanPodPolicyRunning
-		testjob.Spec.RunPolicy.CleanPodPolicy = &running
+		testjob.Spec.RunPolicy.CleanPodPolicy = kubeflowv1.CleanPodPolicyPointer(kubeflowv1.CleanPodPolicyRunning)
 	}
 
 	// Update the key of TestReplicaSpecs to camel case.