Set the default value of CleanPodPolicy to None.

Signed-off-by: Syulin7 <[email protected]>
kubeflow · Feb 9, 2023 · af9209a · af9209a
1 parent c85040a
commit af9209a
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 25 deletions.
diff --git a/pkg/apis/kubeflow.org/v1/defaulting_utils.go b/pkg/apis/kubeflow.org/v1/defaulting_utils.go
@@ -58,3 +58,8 @@ func setTypeNameToCamelCase(replicaSpecs map[commonv1.ReplicaType]*commonv1.Repl
 		}
 	}
 }
+
+func cleanPodPolicyPointer(cleanPodPolicy commonv1.CleanPodPolicy) *commonv1.CleanPodPolicy {
+	c := cleanPodPolicy
+	return &c
+}
diff --git a/pkg/apis/kubeflow.org/v1/mxnet_defaults.go b/pkg/apis/kubeflow.org/v1/mxnet_defaults.go
@@ -46,9 +46,9 @@ func setMXNetTypeNamesToCamelCase(mxJob *MXJob) {
 
 // SetDefaults_MXJob sets any unspecified values to defaults.
 func SetDefaults_MXJob(mxjob *MXJob) {
-	// Set default cleanpod policy to All.
+	// Set default cleanpod policy to None.
 	if mxjob.Spec.RunPolicy.CleanPodPolicy == nil {
-		all := commonv1.CleanPodPolicyAll
+		all := commonv1.CleanPodPolicyNone
 		mxjob.Spec.RunPolicy.CleanPodPolicy = &all
 	}
 

diff --git a/pkg/apis/kubeflow.org/v1/mxnet_defaults_test.go b/pkg/apis/kubeflow.org/v1/mxnet_defaults_test.go
@@ -96,7 +96,7 @@ func TestSetDefaults_MXJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedMXNetJob(commonv1.CleanPodPolicyAll, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, MXJobDefaultPort),
+			expected: expectedMXNetJob(commonv1.CleanPodPolicyNone, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, MXJobDefaultPort),
 		},
 		"Set spec with restart policy": {
 			original: &MXJob{
@@ -118,7 +118,7 @@ func TestSetDefaults_MXJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedMXNetJob(commonv1.CleanPodPolicyAll, commonv1.RestartPolicyOnFailure, 1, MXJobDefaultPortName, MXJobDefaultPort),
+			expected: expectedMXNetJob(commonv1.CleanPodPolicyNone, commonv1.RestartPolicyOnFailure, 1, MXJobDefaultPortName, MXJobDefaultPort),
 		},
 		"Set spec with replicas": {
 			original: &MXJob{
@@ -140,7 +140,7 @@ func TestSetDefaults_MXJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedMXNetJob(commonv1.CleanPodPolicyAll, MXJobDefaultRestartPolicy, 3, MXJobDefaultPortName, MXJobDefaultPort),
+			expected: expectedMXNetJob(commonv1.CleanPodPolicyNone, MXJobDefaultRestartPolicy, 3, MXJobDefaultPortName, MXJobDefaultPort),
 		},
 
 		"Set spec with default node port name and port": {
@@ -168,7 +168,7 @@ func TestSetDefaults_MXJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedMXNetJob(commonv1.CleanPodPolicyAll, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, MXJobDefaultPort),
+			expected: expectedMXNetJob(commonv1.CleanPodPolicyNone, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, MXJobDefaultPort),
 		},
 
 		"Set spec with node port": {
@@ -196,7 +196,32 @@ func TestSetDefaults_MXJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedMXNetJob(commonv1.CleanPodPolicyAll, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, 9999),
+			expected: expectedMXNetJob(commonv1.CleanPodPolicyNone, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, 9999),
+		},
+
+		"set spec with cleanpod policy": {
+			original: &MXJob{
+				Spec: MXJobSpec{
+					RunPolicy: commonv1.RunPolicy{
+						CleanPodPolicy: cleanPodPolicyPointer(commonv1.CleanPodPolicyAll),
+					},
+					MXReplicaSpecs: map[commonv1.ReplicaType]*commonv1.ReplicaSpec{
+						MXJobReplicaTypeWorker: &commonv1.ReplicaSpec{
+							Template: corev1.PodTemplateSpec{
+								Spec: corev1.PodSpec{
+									Containers: []corev1.Container{
+										corev1.Container{
+											Name:  MXJobDefaultContainerName,
+											Image: testImage,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: expectedMXNetJob(commonv1.CleanPodPolicyAll, MXJobDefaultRestartPolicy, 1, MXJobDefaultPortName, MXJobDefaultPort),
 		},
 	}
 

diff --git a/pkg/apis/kubeflow.org/v1/tensorflow_defaults.go b/pkg/apis/kubeflow.org/v1/tensorflow_defaults.go
@@ -50,9 +50,9 @@ func setTensorflowTypeNamesToCamelCase(tfJob *TFJob) {
 
 // SetDefaults_TFJob sets any unspecified values to defaults.
 func SetDefaults_TFJob(tfJob *TFJob) {
-	// Set default cleanpod policy to Running.
+	// Set default cleanpod policy to None.
 	if tfJob.Spec.RunPolicy.CleanPodPolicy == nil {
-		running := commonv1.CleanPodPolicyRunning
+		running := commonv1.CleanPodPolicyNone
 		tfJob.Spec.RunPolicy.CleanPodPolicy = &running
 	}
 	// Set default success policy to "".

diff --git a/pkg/apis/kubeflow.org/v1/tensorflow_defaults_test.go b/pkg/apis/kubeflow.org/v1/tensorflow_defaults_test.go
@@ -151,7 +151,7 @@ func TestSetDefaultTFJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedTFJob(commonv1.CleanPodPolicyRunning, customRestartPolicy, TFJobDefaultPortName, TFJobDefaultPort),
+			expected: expectedTFJob(commonv1.CleanPodPolicyNone, customRestartPolicy, TFJobDefaultPortName, TFJobDefaultPort),
 		},
 		"set replicas with default restartpolicy": {
 			original: &TFJob{
@@ -178,7 +178,7 @@ func TestSetDefaultTFJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedTFJob(commonv1.CleanPodPolicyRunning, TFJobDefaultRestartPolicy, TFJobDefaultPortName, TFJobDefaultPort),
+			expected: expectedTFJob(commonv1.CleanPodPolicyNone, TFJobDefaultRestartPolicy, TFJobDefaultPortName, TFJobDefaultPort),
 		},
 		"set replicas with default port": {
 			original: &TFJob{
@@ -201,7 +201,7 @@ func TestSetDefaultTFJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedTFJob(commonv1.CleanPodPolicyRunning, customRestartPolicy, "", 0),
+			expected: expectedTFJob(commonv1.CleanPodPolicyNone, customRestartPolicy, "", 0),
 		},
 		"set replicas adding default port": {
 			original: &TFJob{
@@ -230,7 +230,7 @@ func TestSetDefaultTFJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedTFJob(commonv1.CleanPodPolicyRunning, customRestartPolicy, customPortName, customPort),
+			expected: expectedTFJob(commonv1.CleanPodPolicyNone, customRestartPolicy, customPortName, customPort),
 		},
 		"set custom cleanpod policy": {
 			original: &TFJob{
@@ -273,8 +273,3 @@ func TestSetDefaultTFJob(t *testing.T) {
 		}
 	}
 }
-
-func cleanPodPolicyPointer(cleanPodPolicy commonv1.CleanPodPolicy) *commonv1.CleanPodPolicy {
-	c := cleanPodPolicy
-	return &c
-}
diff --git a/pkg/apis/kubeflow.org/v1/xgboost_defaults.go b/pkg/apis/kubeflow.org/v1/xgboost_defaults.go
@@ -46,9 +46,9 @@ func setXGBoostJobTypeNamesToCamelCase(xgboostJob *XGBoostJob) {
 
 // SetDefaults_XGBoostJob sets any unspecified values to defaults.
 func SetDefaults_XGBoostJob(xgboostJob *XGBoostJob) {
-	// Set default cleanpod policy to All.
+	// Set default cleanpod policy to None.
 	if xgboostJob.Spec.RunPolicy.CleanPodPolicy == nil {
-		all := commonv1.CleanPodPolicyAll
+		all := commonv1.CleanPodPolicyNone
 		xgboostJob.Spec.RunPolicy.CleanPodPolicy = &all
 	}
 

diff --git a/pkg/apis/kubeflow.org/v1/xgboost_defaults_test.go b/pkg/apis/kubeflow.org/v1/xgboost_defaults_test.go
@@ -96,7 +96,7 @@ func TestSetDefaults_XGBoostJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedXGBoostJob(commonv1.CleanPodPolicyAll, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
+			expected: expectedXGBoostJob(commonv1.CleanPodPolicyNone, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
 		},
 		"Set spec with restart policy": {
 			original: &XGBoostJob{
@@ -118,7 +118,7 @@ func TestSetDefaults_XGBoostJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedXGBoostJob(commonv1.CleanPodPolicyAll, commonv1.RestartPolicyOnFailure, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
+			expected: expectedXGBoostJob(commonv1.CleanPodPolicyNone, commonv1.RestartPolicyOnFailure, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
 		},
 		"Set spec with replicas": {
 			original: &XGBoostJob{
@@ -140,7 +140,7 @@ func TestSetDefaults_XGBoostJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedXGBoostJob(commonv1.CleanPodPolicyAll, XGBoostJobDefaultRestartPolicy, 3, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
+			expected: expectedXGBoostJob(commonv1.CleanPodPolicyNone, XGBoostJobDefaultRestartPolicy, 3, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
 		},
 
 		"Set spec with default node port name and port": {
@@ -168,7 +168,7 @@ func TestSetDefaults_XGBoostJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedXGBoostJob(commonv1.CleanPodPolicyAll, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
+			expected: expectedXGBoostJob(commonv1.CleanPodPolicyNone, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
 		},
 
 		"Set spec with node port": {
@@ -196,7 +196,31 @@ func TestSetDefaults_XGBoostJob(t *testing.T) {
 					},
 				},
 			},
-			expected: expectedXGBoostJob(commonv1.CleanPodPolicyAll, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, 9999),
+			expected: expectedXGBoostJob(commonv1.CleanPodPolicyNone, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, 9999),
+		},
+		"set spec with cleanpod policy": {
+			original: &XGBoostJob{
+				Spec: XGBoostJobSpec{
+					RunPolicy: commonv1.RunPolicy{
+						CleanPodPolicy: cleanPodPolicyPointer(commonv1.CleanPodPolicyAll),
+					},
+					XGBReplicaSpecs: map[commonv1.ReplicaType]*commonv1.ReplicaSpec{
+						XGBoostJobReplicaTypeWorker: &commonv1.ReplicaSpec{
+							Template: corev1.PodTemplateSpec{
+								Spec: corev1.PodSpec{
+									Containers: []corev1.Container{
+										corev1.Container{
+											Name:  XGBoostJobDefaultContainerName,
+											Image: testImage,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: expectedXGBoostJob(commonv1.CleanPodPolicyAll, XGBoostJobDefaultRestartPolicy, 1, XGBoostJobDefaultPortName, XGBoostJobDefaultPort),
 		},
 	}