Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v1alpha2] add pod label with job name (without namespace) #672

Closed
jlewi opened this issue Jun 15, 2018 · 0 comments
Closed

[v1alpha2] add pod label with job name (without namespace) #672

jlewi opened this issue Jun 15, 2018 · 0 comments
Assignees
@gaocegege
Labels
area/0.2.0 priority/p1

Comments

@jlewi
Copy link
Contributor

jlewi commented Jun 15, 2018

Right now the pod labels include "tf_job_key" which is ${namespace}-{jobname}.

The namespace feels redundant. I think we should have a label with just the job name and not force
the user to specify namespace. I believe users can already restrict to namespace by specifying the namespace directly so I don't think it makes sense to require the namespace.

If we need the tf_job_key label for some other reason we can keep it and just add a job_name label as well; otherwise I think we can just get rid of it.

Here's an example

  labels:
    group_name: kubeflow.org
    tf-replica-index: "0"
    tf-replica-type: worker
    tf_job_key: kubeflow-jlewi-master-is-chief
@gaocegege gaocegege self-assigned this Jun 15, 2018
This was referenced Jun 15, 2018
Merged
Merged
@jlewi jlewi mentioned this issue Jun 21, 2018
Closed
jlewi added a commit to jlewi/k8s that referenced this issue Jun 21, 2018
@jlewi
Use tf_job_name not tf_job_key as the label name.
85308ed 
* tf_job_name is consistent with v1alpha1; this desirable from a compatibility
  perspective.

* tf_job_key isn't quite right because the job key includes namespace
  but we are no longer using the namespace in the value

Related to kubeflow#672
@jlewi jlewi mentioned this issue Jun 21, 2018
Merged
k8s-ci-robot pushed a commit that referenced this issue Jun 21, 2018
@jlewi @k8s-ci-robot
Use tf_job_name not tf_job_key as the label name. (#689)
bfff36c 
* Use tf_job_name not tf_job_key as the label name.

* tf_job_name is consistent with v1alpha1; this desirable from a compatibility
  perspective.

* tf_job_key isn't quite right because the job key includes namespace
  but we are no longer using the namespace in the value

Related to #672

* Run goimports.

* Fix E2E tests.

* Increase timeout because pods are stuck in pending state.
jetmuffin pushed a commit to jetmuffin/tf-operator that referenced this issue Jul 9, 2018
@jlewi @k8s-ci-robot
Use tf_job_name not tf_job_key as the label name. (kubeflow#689)
640f175 
* Use tf_job_name not tf_job_key as the label name.

* tf_job_name is consistent with v1alpha1; this desirable from a compatibility
  perspective.

* tf_job_key isn't quite right because the job key includes namespace
  but we are no longer using the namespace in the value

Related to kubeflow#672

* Run goimports.

* Fix E2E tests.

* Increase timeout because pods are stuck in pending state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gaocegege gaocegege

Labels
area/0.2.0 priority/p1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jlewi @gaocegege