Renaming labels to common types

dashboard/backend/handler/api_handler.go

@@ -25,7 +25,7 @@ type APIHandler struct {
 // if any and related pods
 type TFJobDetail struct {
 	TFJob *tfv1.TFJob `json:"tfJob"`
-	Pods  []v1.Pod       `json:"pods"`
+	Pods  []v1.Pod    `json:"pods"`
 }
 
 // TFJobList is a list of TFJobs
@@ -160,7 +160,7 @@ func (apiHandler *APIHandler) handleGetTFJobDetail(request *restful.Request, res
 
 	// Get associated pods
 	pods, err := apiHandler.cManager.ClientSet.CoreV1().Pods(namespace).List(metav1.ListOptions{
-		LabelSelector: fmt.Sprintf("group-name=kubeflow.org,tf-job-name=%s", name),
+		LabelSelector: fmt.Sprintf("group-name=kubeflow.org,job-name=%s", name),
 	})
 	if err != nil {
 		log.Warningf("failed to list pods for TFJob %v under namespace %v: %v", name, namespace, err)

pkg/common/jobcontroller/jobcontroller.go

@@ -55,9 +55,6 @@ type ControllerInterface interface {
 	// Returns the Replica Index(value) in the labels of the job
 	GetReplicaIndexLabelKey() string
 
-	// Returns the Job Role(key) in the labels of the job
-	GetJobRoleKey() string
-
 	// Returns the Job from Informer Cache
 	GetJobFromInformerCache(namespace, name string) (metav1.Object, error)
 
@@ -139,6 +136,14 @@ type JobController struct {
 	Recorder record.EventRecorder
 }
 
+const (
+	// JobNameLabel represents the label key for the job name, the value is job name
+	JobNameLabel = "job-name"
+
+	// JobRoleLabel represents the label key for the job role, e.g. the value is master
+	JobRoleLabel = "job-role"
+)
+
 func NewJobController(
 	controllerImpl ControllerInterface,
 	reconcilerSyncPeriod metav1.Duration,
@@ -200,11 +205,13 @@ func (jc *JobController) GenOwnerReference(obj metav1.Object) *metav1.OwnerRefer
 
 func (jc *JobController) GenLabels(jobName string) map[string]string {
 	labelGroupName := jc.Controller.GetGroupNameLabelKey()
-	labelJobName := jc.Controller.GetJobNameLabelKey()
+	// deprecatedLabel is kept for backward compatibility. Has to be removed later
+	deprecatedLabelJobName := jc.Controller.GetJobNameLabelKey()
 	groupName := jc.Controller.GetGroupNameLabelValue()
 	return map[string]string{
-		labelGroupName: groupName,
-		labelJobName:   strings.Replace(jobName, "/", "-", -1),
+		labelGroupName:         groupName,
+		JobNameLabel:           strings.Replace(jobName, "/", "-", -1),
+		deprecatedLabelJobName: strings.Replace(jobName, "/", "-", -1),
 	}
 }
 
@@ -236,8 +243,6 @@ func (jc *JobController) SyncPodGroup(job metav1.Object, minAvailableReplicas in
 
 // SyncPdb will create a PDB for gang scheduling by kube-batch.
 func (jc *JobController) SyncPdb(job metav1.Object, minAvailableReplicas int32) (*v1beta1.PodDisruptionBudget, error) {
-	labelJobName := jc.Controller.GetJobNameLabelKey()
-
 	// Check the pdb exist or not
 	pdb, err := jc.KubeClientSet.PolicyV1beta1().PodDisruptionBudgets(job.GetNamespace()).Get(job.GetName(), metav1.GetOptions{})
 	if err == nil || !k8serrors.IsNotFound(err) {
@@ -260,7 +265,7 @@ func (jc *JobController) SyncPdb(job metav1.Object, minAvailableReplicas int32)
 			MinAvailable: &minAvailable,
 			Selector: &metav1.LabelSelector{
 				MatchLabels: map[string]string{
-					labelJobName: job.GetName(),
+					JobNameLabel: job.GetName(),
 				},
 			},
 		},

pkg/common/util/v1/testutil/util.go

@@ -29,7 +29,9 @@ import (
 
 const (
 	LabelGroupName = "group-name"
-	LabelTFJobName = "tf-job-name"
+	JobNameLabel   = "job-name"
+	// Deprecated label. Has to be removed later
+	DeprecatedLabelTFJobName = "tf-job-name"
 )
 
 var (
@@ -42,8 +44,9 @@ var (
 
 func GenLabels(jobName string) map[string]string {
 	return map[string]string{
-		LabelGroupName: GroupName,
-		LabelTFJobName: strings.Replace(jobName, "/", "-", -1),
+		LabelGroupName:           GroupName,
+		JobNameLabel:             strings.Replace(jobName, "/", "-", -1),
+		DeprecatedLabelTFJobName: strings.Replace(jobName, "/", "-", -1),
 	}
 }
 

pkg/common/util/v1beta2/testutil/util.go

@@ -28,8 +28,9 @@ import (
 )
 
 const (
-	LabelGroupName = "group-name"
-	LabelTFJobName = "tf-job-name"
+	LabelGroupName           = "group-name"
+	JobNameLabel             = "job-name"
+	DeprecatedLabelTFJobName = "tf-job-name"
 )
 
 var (
@@ -42,8 +43,9 @@ var (
 
 func GenLabels(jobName string) map[string]string {
 	return map[string]string{
-		LabelGroupName: GroupName,
-		LabelTFJobName: strings.Replace(jobName, "/", "-", -1),
+		LabelGroupName:           GroupName,
+		JobNameLabel:             strings.Replace(jobName, "/", "-", -1),
+		DeprecatedLabelTFJobName: strings.Replace(jobName, "/", "-", -1),
 	}
 }
 

pkg/controller.v1/tensorflow/controller.go

@@ -55,8 +55,8 @@ const (
 	tfReplicaTypeLabel  = "tf-replica-type"
 	tfReplicaIndexLabel = "tf-replica-index"
 	labelGroupName      = "group-name"
-	labelTFJobName      = "tf-job-name"
-	labelTFJobRole      = "tf-job-role"
+	// Deprecated label for backwards compatibility. Has to be removed
+	labelTFJobName = "tf-job-name"
 )
 
 var (
@@ -570,6 +570,7 @@ func (tc *TFController) GetGroupNameLabelKey() string {
 	return labelGroupName
 }
 
+// Deprecated function for backwards compatibility. Has to be removed later
 func (tc *TFController) GetJobNameLabelKey() string {
 	return labelTFJobName
 }
@@ -586,10 +587,6 @@ func (tc *TFController) GetReplicaIndexLabelKey() string {
 	return tfReplicaIndexLabel
 }
 
-func (tc *TFController) GetJobRoleKey() string {
-	return labelTFJobRole
-}
-
 func (tc *TFController) ControllerName() string {
 	return controllerName
 }

pkg/controller.v1/tensorflow/controller_test.go

@@ -455,7 +455,7 @@ func TestSyncPdb(t *testing.T) {
 					MinAvailable: &minAvailable,
 					Selector: &metav1.LabelSelector{
 						MatchLabels: map[string]string{
-							"tf-job-name": "test-sync-pdb",
+							"job-name": "test-sync-pdb",
 						},
 					},
 				},

pkg/controller.v1/tensorflow/pod.go

@@ -153,7 +153,7 @@ func (tc *TFController) createNewPod(tfjob *tfv1.TFJob, rt, index string, spec *
 	labels[tfReplicaIndexLabel] = index
 
 	if masterRole {
-		labels[labelTFJobRole] = "master"
+		labels[jobcontroller.JobRoleLabel] = "master"
 	}
 
 	podTemplate := spec.Template.DeepCopy()

pkg/controller.v1/tensorflow/util_test.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	tfv1 "github.com/kubeflow/tf-operator/pkg/apis/tensorflow/v1"
+	"github.com/kubeflow/tf-operator/pkg/common/jobcontroller"
 	"github.com/kubeflow/tf-operator/pkg/common/util/v1/testutil"
 )
 
@@ -51,9 +52,10 @@ func TestGenLabels(t *testing.T) {
 	expctedKey := "test-key"
 
 	labels := testutil.GenLabels(testKey)
+	jobNamelabel := jobcontroller.JobNameLabel
 
-	if labels[labelTFJobName] != expctedKey {
-		t.Errorf("Expected %s %s, got %s", labelTFJobName, expctedKey, labels[labelTFJobName])
+	if labels[jobNamelabel] != expctedKey {
+		t.Errorf("Expected %s %s, got %s", jobNamelabel, expctedKey, jobNamelabel)
 	}
 	if labels[labelGroupName] != tfv1.GroupName {
 		t.Errorf("Expected %s %s, got %s", labelGroupName, tfv1.GroupName, labels[labelGroupName])

pkg/controller.v1beta2/tensorflow/controller.go

@@ -53,8 +53,8 @@ const (
 	tfReplicaTypeLabel  = "tf-replica-type"
 	tfReplicaIndexLabel = "tf-replica-index"
 	labelGroupName      = "group-name"
-	labelTFJobName      = "tf-job-name"
-	labelTFJobRole      = "tf-job-role"
+	// Deprecated label for backwards compatibility. Has to be removed
+	labelTFJobName = "tf-job-name"
 )
 
 var (
@@ -559,6 +559,7 @@ func (tc *TFController) GetGroupNameLabelKey() string {
 	return labelGroupName
 }
 
+// Deprecated function for backwards compatibility. Has to be removed later
 func (tc *TFController) GetJobNameLabelKey() string {
 	return labelTFJobName
 }
@@ -575,10 +576,6 @@ func (tc *TFController) GetReplicaIndexLabelKey() string {
 	return tfReplicaIndexLabel
 }
 
-func (tc *TFController) GetJobRoleKey() string {
-	return labelTFJobRole
-}
-
 func (tc *TFController) ControllerName() string {
 	return controllerName
 }

pkg/controller.v1beta2/tensorflow/controller_test.go

@@ -455,7 +455,7 @@ func TestSyncPdb(t *testing.T) {
 					MinAvailable: &minAvailable,
 					Selector: &metav1.LabelSelector{
 						MatchLabels: map[string]string{
-							"tf-job-name": "test-sync-pdb",
+							"job-name": "test-sync-pdb",
 						},
 					},
 				},

pkg/controller.v1beta2/tensorflow/pod.go

@@ -153,7 +153,7 @@ func (tc *TFController) createNewPod(tfjob *tfv1beta2.TFJob, rt, index string, s
 	labels[tfReplicaIndexLabel] = index
 
 	if masterRole {
-		labels[labelTFJobRole] = "master"
+		labels[jobcontroller.JobRoleLabel] = "master"
 	}
 
 	podTemplate := spec.Template.DeepCopy()

pkg/controller.v1beta2/tensorflow/util_test.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	tfv1beta2 "github.com/kubeflow/tf-operator/pkg/apis/tensorflow/v1beta2"
+	"github.com/kubeflow/tf-operator/pkg/common/jobcontroller"
 	"github.com/kubeflow/tf-operator/pkg/common/util/v1beta2/testutil"
 )
 
@@ -51,10 +52,12 @@ func TestGenLabels(t *testing.T) {
 	expctedKey := "test-key"
 
 	labels := testutil.GenLabels(testKey)
+	jobNamelabel := jobcontroller.JobNameLabel
 
-	if labels[labelTFJobName] != expctedKey {
-		t.Errorf("Expected %s %s, got %s", labelTFJobName, expctedKey, labels[labelTFJobName])
+	if labels[jobNamelabel] != expctedKey {
+		t.Errorf("Expected %s %s, got %s", jobNamelabel, expctedKey, jobNamelabel)
 	}
+
 	if labels[labelGroupName] != tfv1beta2.GroupName {
 		t.Errorf("Expected %s %s, got %s", labelGroupName, tfv1beta2.GroupName, labels[labelGroupName])
 	}

py/kubeflow/tf_operator/tf_job_client.py

@@ -15,7 +15,7 @@
 TF_JOB_GROUP = "kubeflow.org"
 TF_JOB_PLURAL = "tfjobs"
 TF_JOB_KIND = "TFJob"
-TF_JOB_NAME_LABEL = "tf-job-name"
+TF_JOB_NAME_LABEL = "job-name"
 
 # How long to wait in seconds for requests to the ApiServer
 TIMEOUT = 120