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

Propagate labels from Pipeline and Task to PipelineRun and TaskRun #501

Closed
dwnusbaum opened this issue Feb 13, 2019 · 3 comments
Closed

Propagate labels from Pipeline and Task to PipelineRun and TaskRun #501

dwnusbaum opened this issue Feb 13, 2019 · 3 comments

Comments

@dwnusbaum
Copy link
Contributor

Expected Behavior

Custom labels specified on Pipeline and Task resources should be added to PipelineRun and TaskRun objects that reference them. In other words, labels should propagate as follows:

Pipeline -> PipelineRun -> TaskRun -> Pod
Task -> TaskRun -> Pod

In addition, similarly to how the pipeline.knative.dev/pipeline label is added to generated TaskRuns, a pipeline.knative.dev/task label should be added to generated Pods.

Actual Behavior

Custom labels on Pipeline and Task resources are not propagated to other resources. Labels propagate as follows:

PipelineRun -> TaskRun -> Pod
TaskRun -> Pod

Additional Info

See #458. #488 added label propagation from PipelineRun to TaskRun to Pod, but did not handle propagation for Pipeline and Task. Propagating Pipeline and Task labels will require updating a few data structures so that either the ObjectMeta field or the labels field specifically of Pipelines and Tasks is available when TaskRuns and Pods are created so that they can be propagated correctly.

Consistent label propagation is desirable because it allows users to easily find all resources associated with a given Pipeline/Task.

Open questions:

  • Should labels be propagated from/to resources as well?
@dwnusbaum dwnusbaum mentioned this issue Feb 13, 2019
Closed
@dwnusbaum
Copy link
Contributor Author

Any thoughts on the best way to modify ResolvedTaskResources so it contains task metadata as well? I see two obvious options, but I'm not sure if there is a particular reason to prefer either:

  1. Replace TaskSpec *v1alpha1.TaskSpec with Task v1alpha1.TaskInterface
  2. Add a new field TaskMetadata *metav1.ObjectMeta (or just TaskLabels map[string]string if we don't want all of the metadata)

dwnusbaum added a commit to dwnusbaum/pipeline that referenced this issue Feb 18, 2019
@dwnusbaum
Propagate labels from Pipeline/Task to PipelineRun/TaskRun
755862b 
With this change, labels are propagated from Pipeline and Task to
PipelineRun and TaskRun, respectively, giving us full label propagation
from Pipeline to PipelineRun to TaskRun to Pod and Task to TaskRun to
Pod.

This commit also adds a label whose key is pipeline.knative.dev/task
to all TaskRuns that refer to a Task with a TaskRef (the label is not
added to TaskRuns using an embedded TaskSpec) that contains the name of
the Task.

In addition, this commit introduces a builder for Pod values to increase
the readability of the taskrun reconciliation tests.

Fixes tektoncd#501
dwnusbaum added a commit to dwnusbaum/pipeline that referenced this issue Feb 18, 2019
@dwnusbaum
Propagate labels from Pipeline/Task to PipelineRun/TaskRun
6fe3e01 
With this change, labels are propagated from Pipeline and Task to
PipelineRun and TaskRun, respectively, giving us full label propagation
from Pipeline to PipelineRun to TaskRun to Pod and Task to TaskRun to
Pod.

This commit also adds a label whose key is pipeline.knative.dev/task
to all TaskRuns that refer to a Task with a TaskRef (the label is not
added to TaskRuns using an embedded TaskSpec) that contains the name of
the Task.

In addition, this commit introduces a builder for Pod values to increase
the readability of the taskrun reconciliation tests.

Fixes tektoncd#501
@dwnusbaum dwnusbaum mentioned this issue Feb 18, 2019
Merged
3 tasks
@dwnusbaum
Copy link
Contributor Author

Turns out that I was a bit confused, and I don't think we ended up needing to change any data types. I think the main question left is how best to update the PipelineRun/TaskRun with the labels from the Pipeline/Task so that we don't keep doing the same work in the reconciler and so we can keep the core status logic clear. See #517 (comment).

dwnusbaum added a commit to dwnusbaum/pipeline that referenced this issue Feb 19, 2019
@dwnusbaum
Propagate labels from Pipeline/Task to PipelineRun/TaskRun
97c55ea 
With this change, labels are propagated from Pipeline and Task to
PipelineRun and TaskRun, respectively, giving us full label propagation
from Pipeline to PipelineRun to TaskRun to Pod and Task to TaskRun to
Pod.

This commit also adds a label whose key is pipeline.knative.dev/task
to all TaskRuns that refer to a Task with a TaskRef (the label is not
added to TaskRuns using an embedded TaskSpec) that contains the name of
the Task.

Documentation related to labels was moved to labels.md in order
to avoid duplicating similar content across four other pages.

In addition, this commit introduces a builder for Pod values to increase
the readability of the taskrun reconciliation tests.

Fixes tektoncd#501
dwnusbaum added a commit to dwnusbaum/pipeline that referenced this issue Feb 19, 2019
@dwnusbaum
Propagate labels from Pipeline/Task to PipelineRun/TaskRun
3436301 
With this change, labels are propagated from Pipeline and Task to
PipelineRun and TaskRun, respectively, giving us full label propagation
from Pipeline to PipelineRun to TaskRun to Pod and Task to TaskRun to
Pod.

This commit also adds a label whose key is pipeline.knative.dev/task
to all TaskRuns that refer to a Task with a TaskRef (the label is not
added to TaskRuns using an embedded TaskSpec) that contains the name of
the Task.

Documentation related to labels was moved to labels.md in order
to avoid duplicating similar content across four other pages.

Fixes tektoncd#501
knative-prow-robot pushed a commit that referenced this issue Feb 20, 2019
@dwnusbaum @knative-prow-robot
Propagate labels from Pipeline/Task to PipelineRun/TaskRun
69ade03 
With this change, labels are propagated from Pipeline and Task to
PipelineRun and TaskRun, respectively, giving us full label propagation
from Pipeline to PipelineRun to TaskRun to Pod and Task to TaskRun to
Pod.

This commit also adds a label whose key is pipeline.knative.dev/task
to all TaskRuns that refer to a Task with a TaskRef (the label is not
added to TaskRuns using an embedded TaskSpec) that contains the name of
the Task.

Documentation related to labels was moved to labels.md in order
to avoid duplicating similar content across four other pages.

Fixes #501
@bobcatfish
Copy link
Collaborator

Sorry for missing all the thoughts and potential discussion here @dwnusbaum 🙏 but thanks so much for taking this on and fixing it!! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bobcatfish @dwnusbaum