From 4398c32aac63c221627277e9dd9dad17440f5790 Mon Sep 17 00:00:00 2001 From: Syulin7 <735122171@qq.com> Date: Thu, 9 Feb 2023 10:34:01 +0800 Subject: [PATCH] Set the default value of CleanPodPolicy to None. Signed-off-by: Syulin7 <735122171@qq.com> --- pkg/apis/kubeflow.org/v1/defaulting_utils.go | 4 +++ pkg/apis/kubeflow.org/v1/mxnet_defaults.go | 4 +-- .../kubeflow.org/v1/mxnet_defaults_test.go | 35 ++++++++++++++++--- .../kubeflow.org/v1/tensorflow_defaults.go | 4 +-- .../v1/tensorflow_defaults_test.go | 13 +++---- pkg/apis/kubeflow.org/v1/xgboost_defaults.go | 4 +-- .../kubeflow.org/v1/xgboost_defaults_test.go | 34 +++++++++++++++--- 7 files changed, 73 insertions(+), 25 deletions(-) diff --git a/pkg/apis/kubeflow.org/v1/defaulting_utils.go b/pkg/apis/kubeflow.org/v1/defaulting_utils.go index 538eb82d66..8898b5dc07 100644 --- a/pkg/apis/kubeflow.org/v1/defaulting_utils.go +++ b/pkg/apis/kubeflow.org/v1/defaulting_utils.go @@ -58,3 +58,7 @@ func setTypeNameToCamelCase(replicaSpecs map[commonv1.ReplicaType]*commonv1.Repl } } } + +func cleanPodPolicyPointer(cleanPodPolicy commonv1.CleanPodPolicy) *commonv1.CleanPodPolicy { + return &cleanPodPolicy +} diff --git a/pkg/apis/kubeflow.org/v1/mxnet_defaults.go b/pkg/apis/kubeflow.org/v1/mxnet_defaults.go index 235fcf8cad..2c684c9668 100644 --- 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 index 7d2a277e92..0e5ba92581 100644 --- 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 index 9ffa758473..88ae0c1c6b 100644 --- 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 index 49361c84c0..3fa2236490 100644 --- 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 index 227bb90fb7..79cddf65c3 100644 --- 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 index b71093b853..329ad99eda 100644 --- 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), }, }