Removing deprecated Job Labels (#1702)

johnugeorge · web-flow · commit 69813fbc37f3 · 2022-12-22T15:56:13.000Z
* Support for k8s v1.25 in CI

* Support for k8s v1.25 in CI

* Change k8s api to v1.25

* Upgrade golangci-lint version

* Remove deprecated job labels

* Updating dependencies

* Fix Sdk
diff --git a/go.mod b/go.mod
@@ -4,7 +4,7 @@ go 1.19
 
 require (
 	github.com/go-logr/logr v1.2.3
-	github.com/kubeflow/common v0.4.4
+	github.com/kubeflow/common v0.4.5
 	github.com/onsi/ginkgo/v2 v2.1.6
 	github.com/onsi/gomega v1.20.1
 	github.com/prometheus/client_golang v1.12.2
diff --git a/go.sum b/go.sum
@@ -327,8 +327,8 @@ github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
-github.com/kubeflow/common v0.4.4 h1:QG9IgOHIsaS1bq1DtfW/yxtKkBTl5sTjRCyNicjdo10=
-github.com/kubeflow/common v0.4.4/go.mod h1:di43u2m7DyuwnRDb7Kwz1nmA/nhpjnQ+K+gWCV/SPZk=
+github.com/kubeflow/common v0.4.5 h1:W7p+s/4Za1UzIgKP2Z6ormEvsUVHykeaXaOuu8+UgpI=
+github.com/kubeflow/common v0.4.5/go.mod h1:di43u2m7DyuwnRDb7Kwz1nmA/nhpjnQ+K+gWCV/SPZk=
 github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
diff --git a/pkg/common/util/v1/testutil/pod.go b/pkg/common/util/v1/testutil/pod.go
@@ -54,9 +54,7 @@ func NewBasePod(name string, job metav1.Object, refs []metav1.OwnerReference) *c
 
 func NewPod(job metav1.Object, typ string, index int, refs []metav1.OwnerReference) *corev1.Pod {
 	pod := NewBasePod(fmt.Sprintf("%s-%s-%d", job.GetName(), typ, index), job, refs)
-	pod.Labels[commonv1.ReplicaTypeLabelDeprecated] = typ
 	pod.Labels[commonv1.ReplicaTypeLabel] = typ
-	pod.Labels[commonv1.ReplicaIndexLabelDeprecated] = fmt.Sprintf("%d", index)
 	pod.Labels[commonv1.ReplicaIndexLabel] = fmt.Sprintf("%d", index)
 	return pod
 }
diff --git a/pkg/common/util/v1/testutil/service.go b/pkg/common/util/v1/testutil/service.go
@@ -52,9 +52,7 @@ func NewBaseService(name string, job metav1.Object, refs []metav1.OwnerReference
 
 func NewService(job metav1.Object, typ string, index int, refs []metav1.OwnerReference) *corev1.Service {
 	svc := NewBaseService(fmt.Sprintf("%s-%s-%d", job.GetName(), typ, index), job, refs)
-	svc.Labels[commonv1.ReplicaTypeLabelDeprecated] = typ
 	svc.Labels[commonv1.ReplicaTypeLabel] = typ
-	svc.Labels[commonv1.ReplicaIndexLabelDeprecated] = fmt.Sprintf("%d", index)
 	svc.Labels[commonv1.ReplicaIndexLabel] = fmt.Sprintf("%d", index)
 	return svc
 }
diff --git a/pkg/common/util/v1/testutil/util.go b/pkg/common/util/v1/testutil/util.go
@@ -30,8 +30,6 @@ import (
 const (
 	LabelGroupName = "group-name"
 	JobNameLabel   = "job-name"
-	// Deprecated label. Has to be removed later
-	DeprecatedLabelTFJobName = "tf-job-name"
 )
 
 var (
diff --git a/pkg/controller.v1/mpi/mpijob_controller.go b/pkg/controller.v1/mpi/mpijob_controller.go
@@ -272,20 +272,6 @@ func (jc *MPIJobReconciler) ControllerName() string {
 	return controllerName
 }
 
-// GenLabels is overridden for backward compatibility
-// TODO(zw0610): remove this overriding method when backward compatibility is dropped
-func (jc *MPIJobReconciler) GenLabels(jobName string) map[string]string {
-	// Generate basic labels from kubeflow/common
-	basicLabels := jc.JobController.GenLabels(jobName)
-
-	// add "mpi-job-name" label for backward compatibility
-	basicLabels[labelMPIJobName] = basicLabels[commonv1.JobNameLabel]
-	// remove "job-name" as MPIJob never uses
-	delete(basicLabels, commonv1.JobNameLabelDeprecated)
-
-	return basicLabels
-}
-
 func (jc *MPIJobReconciler) GetAPIGroupVersionKind() schema.GroupVersionKind {
 	return kubeflowv1.GroupVersion.WithKind(kubeflowv1.MPIJobKind)
 }
diff --git a/pkg/controller.v1/tensorflow/pod_test.go b/pkg/controller.v1/tensorflow/pod_test.go
@@ -111,9 +111,7 @@ var _ = Describe("TFJob controller", func() {
 
 				jobName := c.tfJob.GetName()
 				labels := reconciler.GenLabels(jobName)
-				labels[commonv1.ReplicaTypeLabelDeprecated] = c.rt
 				labels[commonv1.ReplicaTypeLabel] = c.rt
-				labels[commonv1.ReplicaIndexLabelDeprecated] = c.index
 				labels[commonv1.ReplicaIndexLabel] = c.index
 
 				Expect(reconciler.SetClusterSpec(c.tfJob, podTemplate, c.rt, c.index)).Should(Succeed())
diff --git a/pkg/controller.v1/tensorflow/tfjob_controller.go b/pkg/controller.v1/tensorflow/tfjob_controller.go
@@ -72,9 +72,6 @@ const (
 
 	controllerName = "tfjob-controller"
 
-	// labels for pods and servers.
-	tfReplicaTypeLabel  = "replica-type"
-	tfReplicaIndexLabel = "replica-index"
 	// volcanoTaskSpecKey task spec key used in pod annotation when EnableGangScheduling is true
 	volcanoTaskSpecKey = "volcano.sh/task-spec"
 
@@ -833,8 +830,6 @@ func (r *TFJobReconciler) createNewPod(tfjob *kubeflowv1.TFJob, rt, index string
 
 	// Set type and index for the worker.
 	labels := r.GenLabels(tfjob.Name)
-	labels[tfReplicaTypeLabel] = rt
-	labels[tfReplicaIndexLabel] = index
 	labels[commonv1.ReplicaTypeLabel] = rt
 	labels[commonv1.ReplicaIndexLabel] = index
 
diff --git a/pkg/controller.v1/tensorflow/util_test.go b/pkg/controller.v1/tensorflow/util_test.go
@@ -52,21 +52,11 @@ func TestGenLabels(t *testing.T) {
 
 	labels := reconciler.GenLabels(testJobName)
 	jobNameLabel := commonv1.JobNameLabel
-	JobNameLabelDeprecated := commonv1.JobNameLabelDeprecated
 
 	if labels[jobNameLabel] != expctedVal {
 		t.Errorf("Expected %s %s, got %s", jobNameLabel, expctedVal, jobNameLabel)
 	}
 
-	if labels[JobNameLabelDeprecated] != expctedVal {
-		t.Errorf("Expected %s %s, got %s", JobNameLabelDeprecated, expctedVal, JobNameLabelDeprecated)
-	}
-
-	if labels[commonv1.GroupNameLabelDeprecated] != kubeflowv1.GroupVersion.Group {
-		t.Errorf("Expected %s %s, got %s", commonv1.GroupNameLabelDeprecated, kubeflowv1.GroupVersion.Group,
-			labels[commonv1.GroupNameLabelDeprecated])
-	}
-
 	if labels[commonv1.OperatorNameLabel] != controllerName {
 		t.Errorf("Expected %s %s, got %s", commonv1.OperatorNameLabel, controllerName,
 			labels[commonv1.OperatorNameLabel])
diff --git a/sdk/python/kubeflow/training/utils/utils.py b/sdk/python/kubeflow/training/utils/utils.py
@@ -76,7 +76,6 @@ def get_job_labels(name, master=False, replica_type=None, replica_index=None):
     :return: Dict: Labels
     """
     labels = {
-        constants.JOB_GROUP_LABEL: constants.KUBEFLOW_GROUP,
         constants.JOB_NAME_LABEL: name,
     }
     if master:

-Original file line number
+Diff line change
 require (
 	github.com/go-logr/logr v1.2.3
 -	github.com/kubeflow/common v0.4.4
 +	github.com/kubeflow/common v0.4.5
 	github.com/onsi/ginkgo/v2 v2.1.6
 	github.com/onsi/gomega v1.20.1
 	github.com/prometheus/client_golang v1.12.2
Original file line number	Diff line number	Diff line change
`@@ -54,9 +54,7 @@ func NewBasePod(name string, job metav1.Object, refs []metav1.OwnerReference) *c`
`54`	`54`
`55`	`55`	`func NewPod(job metav1.Object, typ string, index int, refs []metav1.OwnerReference) *corev1.Pod {`
`56`	`56`	`pod := NewBasePod(fmt.Sprintf("%s-%s-%d", job.GetName(), typ, index), job, refs)`
`57`		`- pod.Labels[commonv1.ReplicaTypeLabelDeprecated] = typ`
`58`	`57`	`pod.Labels[commonv1.ReplicaTypeLabel] = typ`
`59`		`- pod.Labels[commonv1.ReplicaIndexLabelDeprecated] = fmt.Sprintf("%d", index)`
`60`	`58`	`pod.Labels[commonv1.ReplicaIndexLabel] = fmt.Sprintf("%d", index)`
`61`	`59`	`return pod`
`62`	`60`	`}`
Original file line number	Diff line number	Diff line change
`@@ -52,9 +52,7 @@ func NewBaseService(name string, job metav1.Object, refs []metav1.OwnerReference`
`52`	`52`
`53`	`53`	`func NewService(job metav1.Object, typ string, index int, refs []metav1.OwnerReference) *corev1.Service {`
`54`	`54`	`svc := NewBaseService(fmt.Sprintf("%s-%s-%d", job.GetName(), typ, index), job, refs)`
`55`		`- svc.Labels[commonv1.ReplicaTypeLabelDeprecated] = typ`
`56`	`55`	`svc.Labels[commonv1.ReplicaTypeLabel] = typ`
`57`		`- svc.Labels[commonv1.ReplicaIndexLabelDeprecated] = fmt.Sprintf("%d", index)`
`58`	`56`	`svc.Labels[commonv1.ReplicaIndexLabel] = fmt.Sprintf("%d", index)`
`59`	`57`	`return svc`
`60`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,8 +30,6 @@ import (`
`30`	`30`	`const (`
`31`	`31`	`LabelGroupName = "group-name"`
`32`	`32`	`JobNameLabel = "job-name"`
`33`		`- // Deprecated label. Has to be removed later`
`34`		`- DeprecatedLabelTFJobName = "tf-job-name"`
`35`	`33`	`)`
`36`	`34`
`37`	`35`	`var (`
Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,6 @@ def get_job_labels(name, master=False, replica_type=None, replica_index=None):`
`76`	`76`	`:return: Dict: Labels`
`77`	`77`	`"""`
`78`	`78`	`labels = {`
`79`		`- constants.JOB_GROUP_LABEL: constants.KUBEFLOW_GROUP,`
`80`	`79`	`constants.JOB_NAME_LABEL: name,`
`81`	`80`	`}`
`82`	`81`	`if master:`