diff --git a/pkg/apis/common/v1/constants.go b/pkg/apis/common/v1/constants.go index e68d7165..1bd3f8aa 100644 --- a/pkg/apis/common/v1/constants.go +++ b/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" ) diff --git a/pkg/controller.v1/common/job_controller.go b/pkg/controller.v1/common/job_controller.go index 59965949..c45fb335 100644 --- a/pkg/controller.v1/common/job_controller.go +++ b/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(), }, }, }, diff --git a/pkg/controller.v1/common/pod_test.go b/pkg/controller.v1/common/pod_test.go index 827515a5..372d37d7 100644 --- a/pkg/controller.v1/common/pod_test.go +++ b/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", }, }, }, diff --git a/pkg/controller.v1/common/service.go b/pkg/controller.v1/common/service.go index 812d0445..6601cb43 100644 --- a/pkg/controller.v1/common/service.go +++ b/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) diff --git a/pkg/controller.v1/common/service_test.go b/pkg/controller.v1/common/service_test.go index 9e607090..be49b888 100644 --- a/pkg/controller.v1/common/service_test.go +++ b/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", }, }, }, diff --git a/pkg/core/pod.go b/pkg/core/pod.go index 0368f5ef..13e90fdf 100644 --- a/pkg/core/pod.go +++ b/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) diff --git a/pkg/core/service.go b/pkg/core/service.go index 97d9c300..3eac2332 100644 --- a/pkg/core/service.go +++ b/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) diff --git a/pkg/reconciler.v1/common/job.go b/pkg/reconciler.v1/common/job.go index 063bc693..08cff94c 100644 --- a/pkg/reconciler.v1/common/job.go +++ b/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, } } diff --git a/pkg/reconciler.v1/common/service_test.go b/pkg/reconciler.v1/common/service_test.go index 203ec661..da6c443e 100644 --- a/pkg/reconciler.v1/common/service_test.go +++ b/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, }, }, } diff --git a/pkg/reconciler.v1/common/utils_test.go b/pkg/reconciler.v1/common/utils_test.go index 4f4b1d7e..454664aa 100644 --- a/pkg/reconciler.v1/common/utils_test.go +++ b/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", }, } }(), diff --git a/pkg/util/labels/labels.go b/pkg/util/labels/labels.go index 8520c0c3..9522e032 100644 --- a/pkg/util/labels/labels.go +++ b/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 } diff --git a/pkg/util/labels/labels_test.go b/pkg/util/labels/labels_test.go index 49b2fe91..11857361 100644 --- a/pkg/util/labels/labels_test.go +++ b/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", },