Remove deprecated labels (#200)

pkg/apis/common/v1/constants.go

@@ -1,40 +1,19 @@
 package v1
 
 const (
-	// TODO(#149): Remove deprecated labels.
 
 	// ReplicaIndexLabel represents the label key for the replica-index, e.g. 0, 1, 2.. etc
 	ReplicaIndexLabel = "training.kubeflow.org/replica-index"
 
-	// ReplicaIndexLabelDeprecated represents the label key for the replica-index, e.g. the value is 0, 1, 2.. etc
-	// DEPRECATED: Use ReplicaIndexLabel
-	ReplicaIndexLabelDeprecated = "replica-index"
-
 	// ReplicaTypeLabel represents the label key for the replica-type, e.g. ps, worker etc.
 	ReplicaTypeLabel = "training.kubeflow.org/replica-type"
 
-	// ReplicaTypeLabelDeprecated represents the label key for the replica-type, e.g. the value is ps , worker etc.
-	// DEPRECATED: Use ReplicaTypeLabel
-	ReplicaTypeLabelDeprecated = "replica-type"
-
 	// OperatorNameLabel represents the label key for the operator name, e.g. tf-operator, mpi-operator, etc.
 	OperatorNameLabel = "training.kubeflow.org/operator-name"
 
-	// GroupNameLabelDeprecated represents the label key for group name, e.g. the value is kubeflow.org
-	// DEPRECATED: Use OperatorNameLabel
-	GroupNameLabelDeprecated = "group-name"
-
 	// JobNameLabel represents the label key for the job name, the value is the job name.
 	JobNameLabel = "training.kubeflow.org/job-name"
 
-	// JobNameLabelDeprecated represents the label key for the job name, the value is job name
-	// DEPRECATED: Use JobNameLabel
-	JobNameLabelDeprecated = "job-name"
-
 	// JobRoleLabel represents the label key for the job role, e.g. master.
 	JobRoleLabel = "training.kubeflow.org/job-role"
-
-	// JobRoleLabelDeprecated represents the label key for the job role, e.g. the value is master
-	// DEPRECATED: Use JobRoleLabel
-	JobRoleLabelDeprecated = "job-role"
 )

pkg/controller.v1/common/job_controller.go

@@ -78,10 +78,11 @@ type JobControllerConfiguration struct {
 // reconcile logic of the job controller
 //
 // ReconcileJobs(
-//		job interface{},
-//		replicas map[apiv1.ReplicaType]*apiv1.ReplicaSpec,
-//		jobStatus apiv1.JobStatus,
-//		runPolicy *apiv1.RunPolicy) error
+//
+//	job interface{},
+//	replicas map[apiv1.ReplicaType]*apiv1.ReplicaSpec,
+//	jobStatus apiv1.JobStatus,
+//	runPolicy *apiv1.RunPolicy) error
 type JobController struct {
 	Controller apiv1.ControllerInterface
 
@@ -209,11 +210,8 @@ func (jc *JobController) GenOwnerReference(obj metav1.Object) *metav1.OwnerRefer
 func (jc *JobController) GenLabels(jobName string) map[string]string {
 	jobName = strings.Replace(jobName, "/", "-", -1)
 	return map[string]string{
-		// TODO(#149): Remove deprecated labels.
-		apiv1.OperatorNameLabel:        jc.Controller.ControllerName(),
-		apiv1.GroupNameLabelDeprecated: jc.Controller.GetGroupNameLabelValue(),
-		apiv1.JobNameLabel:             jobName,
-		apiv1.JobNameLabelDeprecated:   jobName,
+		apiv1.OperatorNameLabel: jc.Controller.ControllerName(),
+		apiv1.JobNameLabel:      jobName,
 	}
 }
 
@@ -270,8 +268,7 @@ func (jc *JobController) SyncPdb(job metav1.Object, minAvailableReplicas int32)
 			MinAvailable: &minAvailable,
 			Selector: &metav1.LabelSelector{
 				MatchLabels: map[string]string{
-					// TODO(#149): Change for JobNameLabel after some releases.
-					apiv1.JobNameLabelDeprecated: job.GetName(),
+					apiv1.JobNameLabel: job.GetName(),
 				},
 			},
 		},

pkg/controller.v1/common/pod_test.go

@@ -165,21 +165,20 @@ func TestFilterPodsForReplicaType(t *testing.T) {
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:   "c",
-				Labels: map[string]string{apiv1.ReplicaTypeLabelDeprecated: "foo"},
+				Labels: map[string]string{apiv1.ReplicaTypeLabel: "foo"},
 			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:   "d",
-				Labels: map[string]string{apiv1.ReplicaTypeLabelDeprecated: "bar"},
+				Labels: map[string]string{apiv1.ReplicaTypeLabel: "bar"},
 			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "e",
 				Labels: map[string]string{
-					apiv1.ReplicaTypeLabel:           "foo",
-					apiv1.ReplicaTypeLabelDeprecated: "bar",
+					apiv1.ReplicaTypeLabel: "foo",
 				},
 			},
 		},

pkg/controller.v1/common/service.go

@@ -4,7 +4,7 @@
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
-//      http://www.apache.org/licenses/LICENSE-2.0
+//	http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ -216,7 +216,6 @@ func (jc *JobController) CreateNewService(job metav1.Object, rtype apiv1.Replica
 	}
 
 	rt := strings.ToLower(string(rtype))
-	// Append ReplicaTypeLabelDeprecated and ReplicaIndexLabelDeprecated labels.
 	labels := jc.GenLabels(job.GetName())
 	utillabels.SetReplicaType(labels, rt)
 	utillabels.SetReplicaIndexStr(labels, index)

pkg/controller.v1/common/service_test.go

@@ -227,21 +227,20 @@ func TestFilterServicesForReplicaType(t *testing.T) {
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:   "c",
-				Labels: map[string]string{apiv1.ReplicaTypeLabelDeprecated: "foo"},
+				Labels: map[string]string{apiv1.ReplicaTypeLabel: "foo"},
 			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:   "d",
-				Labels: map[string]string{apiv1.ReplicaTypeLabelDeprecated: "bar"},
+				Labels: map[string]string{apiv1.ReplicaTypeLabel: "bar"},
 			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "e",
 				Labels: map[string]string{
-					apiv1.ReplicaTypeLabel:           "foo",
-					apiv1.ReplicaTypeLabelDeprecated: "bar",
+					apiv1.ReplicaTypeLabel: "foo",
 				},
 			},
 		},

pkg/core/pod.go

@@ -17,14 +17,9 @@ func FilterPodsForReplicaType(pods []*v1.Pod, replicaType string) ([]*v1.Pod, er
 		apiv1.ReplicaTypeLabel: replicaType,
 	})
 
-	// TODO(#149): Remove deprecated selector.
-	deprecatedSelector := labels.SelectorFromValidatedSet(labels.Set{
-		apiv1.ReplicaTypeLabelDeprecated: replicaType,
-	})
-
 	for _, pod := range pods {
 		set := labels.Set(pod.Labels)
-		if !selector.Matches(set) && !deprecatedSelector.Matches(set) {
+		if !selector.Matches(set) {
 			continue
 		}
 		result = append(result, pod)

pkg/core/service.go

@@ -18,14 +18,9 @@ func FilterServicesForReplicaType(services []*v1.Service, replicaType string) ([
 		apiv1.ReplicaTypeLabel: replicaType,
 	})
 
-	// TODO(#149): Remove deprecated selector.
-	deprecatedSelector := labels.SelectorFromValidatedSet(labels.Set{
-		apiv1.ReplicaTypeLabelDeprecated: replicaType,
-	})
-
 	for _, service := range services {
 		set := labels.Set(service.Labels)
-		if !selector.Matches(set) && !deprecatedSelector.Matches(set) {
+		if !selector.Matches(set) {
 			continue
 		}
 		result = append(result, service)

pkg/reconciler.v1/common/job.go

@@ -102,11 +102,8 @@ func (r *JobReconciler) OverrideForJobInterface(ui ReconcilerUtilInterface, pi P
 func (r *JobReconciler) GenLabels(jobName string) map[string]string {
 	jobName = strings.Replace(jobName, "/", "-", -1)
 	return map[string]string{
-		// TODO(#149): Remove deprecated labels.
-		commonv1.OperatorNameLabel:        r.GetReconcilerName(),
-		commonv1.GroupNameLabelDeprecated: r.GetGroupNameLabelValue(),
-		commonv1.JobNameLabel:             jobName,
-		commonv1.JobNameLabelDeprecated:   jobName,
+		commonv1.OperatorNameLabel: r.GetReconcilerName(),
+		commonv1.JobNameLabel:      jobName,
 	}
 }
 

pkg/reconciler.v1/common/service_test.go

@@ -56,12 +56,10 @@ func TestCreateNewService(t *testing.T) {
 					},
 					ClusterIP: corev1.ClusterIPNone,
 					Selector: map[string]string{
-						commonv1.GroupNameLabelDeprecated: testjobv1.GroupName,
-						commonv1.OperatorNameLabel:        "Test Reconciler",
-						commonv1.JobNameLabelDeprecated:   jobName,
-						commonv1.JobNameLabel:             jobName,
-						commonv1.ReplicaTypeLabel:         strings.ToLower(string(testjobv1.TestReplicaTypeWorker)),
-						commonv1.ReplicaIndexLabel:        idx,
+						commonv1.OperatorNameLabel: "Test Reconciler",
+						commonv1.JobNameLabel:      jobName,
+						commonv1.ReplicaTypeLabel:  strings.ToLower(string(testjobv1.TestReplicaTypeWorker)),
+						commonv1.ReplicaIndexLabel: idx,
 					},
 				},
 			}

pkg/reconciler.v1/common/utils_test.go

@@ -18,7 +18,6 @@ import (
 	"testing"
 
 	commonv1 "github.com/kubeflow/common/pkg/apis/common/v1"
-	testjobv1 "github.com/kubeflow/common/test_job/apis/test_job/v1"
 
 	"github.com/kubeflow/common/test_job/reconciler.v1/test_job"
 )
@@ -33,10 +32,8 @@ func TestGenLabels(t *testing.T) {
 			return tc{
 				testJobName: "test/job1",
 				expectedLabel: map[string]string{
-					commonv1.GroupNameLabelDeprecated: testjobv1.GroupName,
-					commonv1.JobNameLabel:             "test-job1",
-					commonv1.JobNameLabelDeprecated:   "test-job1",
-					commonv1.OperatorNameLabel:        "Test Reconciler",
+					commonv1.JobNameLabel:      "test-job1",
+					commonv1.OperatorNameLabel: "Test Reconciler",
 				},
 			}
 		}(),

pkg/util/labels/labels.go

@@ -21,15 +21,10 @@ import (
 	v1 "github.com/kubeflow/common/pkg/apis/common/v1"
 )
 
-// TODO(#149): Remove deprecated labels.
-
 func ReplicaIndex(labels map[string]string) (int, error) {
 	v, ok := labels[v1.ReplicaIndexLabel]
 	if !ok {
-		v, ok = labels[v1.ReplicaIndexLabelDeprecated]
-		if !ok {
-			return 0, errors.New("replica index label not found")
-		}
+		return 0, errors.New("replica index label not found")
 	}
 	return strconv.Atoi(v)
 }
@@ -40,31 +35,25 @@ func SetReplicaIndex(labels map[string]string, idx int) {
 
 func SetReplicaIndexStr(labels map[string]string, idx string) {
 	labels[v1.ReplicaIndexLabel] = idx
-	labels[v1.ReplicaIndexLabelDeprecated] = idx
 }
 
 func ReplicaType(labels map[string]string) (v1.ReplicaType, error) {
 	v, ok := labels[v1.ReplicaTypeLabel]
 	if !ok {
-		v, ok = labels[v1.ReplicaTypeLabelDeprecated]
-		if !ok {
-			return "", errors.New("replica type label not found")
-		}
+		return "", errors.New("replica type label not found")
 	}
 	return v1.ReplicaType(v), nil
 }
 
 func SetReplicaType(labels map[string]string, rt string) {
 	labels[v1.ReplicaTypeLabel] = rt
-	labels[v1.ReplicaTypeLabelDeprecated] = rt
 }
 
 func HasKnownLabels(labels map[string]string, groupName string) bool {
 	_, has := labels[v1.OperatorNameLabel]
-	return has || labels[v1.GroupNameLabelDeprecated] == groupName
+	return has
 }
 
 func SetJobRole(labels map[string]string, role string) {
 	labels[v1.JobRoleLabel] = role
-	labels[v1.JobRoleLabelDeprecated] = role
 }

pkg/util/labels/labels_test.go

@@ -20,7 +20,7 @@ func TestReplicaIndex(t *testing.T) {
 		},
 		"old": {
 			labels: map[string]string{
-				v1.ReplicaIndexLabelDeprecated: "3",
+				v1.ReplicaIndexLabel: "3",
 			},
 			want: 3,
 		},
@@ -30,8 +30,7 @@ func TestReplicaIndex(t *testing.T) {
 		},
 		"both": {
 			labels: map[string]string{
-				v1.ReplicaIndexLabel:           "4",
-				v1.ReplicaIndexLabelDeprecated: "5",
+				v1.ReplicaIndexLabel: "4",
 			},
 			want: 4,
 		},
@@ -63,7 +62,7 @@ func TestReplicaType(t *testing.T) {
 		},
 		"old": {
 			labels: map[string]string{
-				v1.ReplicaTypeLabelDeprecated: "Bar",
+				v1.ReplicaTypeLabel: "Bar",
 			},
 			want: "Bar",
 		},
@@ -73,8 +72,7 @@ func TestReplicaType(t *testing.T) {
 		},
 		"both": {
 			labels: map[string]string{
-				v1.ReplicaTypeLabel:           "Baz",
-				v1.ReplicaTypeLabelDeprecated: "Foo",
+				v1.ReplicaTypeLabel: "Baz",
 			},
 			want: "Baz",
 		},