-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
Check labels in e2e PipelineRun tests and document label propagation #500
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,6 +97,26 @@ of the `TaskRun` resource object. | |
For examples and more information about specifying service accounts, see the | ||
[`ServiceAccount`](./auth.md) reference topic. | ||
|
||
## Labels | ||
|
||
Any labels specified in the metadata field of a `PipelineRun` will be | ||
propagated to the `TaskRuns` created automatically for each `Task` in the | ||
`Pipeline` and then to the `Pods` created for those `TaskRuns`. In addition, | ||
the following labels will be added automatically: | ||
|
||
* `pipeline.knative.dev/pipeline` will contain the name of the `Pipeline` | ||
* `pipeline.knative.dev/pipelineRun` will contain the name of the `PipelineRun` | ||
|
||
These labels make it easier to find the resources that are associated with a | ||
given pipeline. | ||
|
||
For example, to find all `Pods` created by a `Pipeline` named test-pipeline, | ||
you could use the following command: | ||
|
||
```shell | ||
kubectl get pods --all-namespaces -l pipeline.knative.dev/pipeline=test-pipeline | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. niiiice |
||
``` | ||
|
||
## Cancelling a PipelineRun | ||
|
||
In order to cancel a running pipeline (`PipelineRun`), you need to update its | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,7 +138,8 @@ func TestPipelineRun(t *testing.T) { | |
td.testSetup(t, c, namespace, i) | ||
|
||
prName := fmt.Sprintf("%s%d", pipelineRunName, i) | ||
if _, err := c.PipelineRunClient.Create(td.pipelineRunFunc(i, namespace)); err != nil { | ||
pr, err := c.PipelineRunClient.Create(td.pipelineRunFunc(i, namespace)) | ||
if err != nil { | ||
t.Fatalf("Failed to create PipelineRun `%s`: %s", prName, err) | ||
} | ||
|
||
|
@@ -160,16 +161,22 @@ func TestPipelineRun(t *testing.T) { | |
if !r.Status.GetCondition(duckv1alpha1.ConditionSucceeded).IsTrue() { | ||
t.Fatalf("Expected TaskRun %s to have succeeded but Status is %v", taskRunName, r.Status) | ||
} | ||
for name, key := range map[string]string{ | ||
pipelineRunName: pipeline.PipelineRunLabelKey, | ||
pipelineName: pipeline.PipelineLabelKey, | ||
} { | ||
expectedName := getName(name, i) | ||
lbl := pipeline.GroupName + key | ||
if val := r.Labels[lbl]; val != expectedName { | ||
t.Errorf("Expected label %s=%s but got %s", lbl, expectedName, val) | ||
} | ||
|
||
// Check label propagation to Tasks. Presize map with len(pr.ObjectMeta.Labels) | ||
// for custom labels, +2 for the labels added in the PipelineRun controller, | ||
// and +1 for the label added in the TaskRun controller. | ||
labels := make(map[string]string, len(pr.ObjectMeta.Labels)+3) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why +3 here? (maybe explain with a comment or a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment in 37ed488 |
||
for key, val := range pr.ObjectMeta.Labels { | ||
labels[key] = val | ||
} | ||
// These labels are added to every TaskRun by the PipelineRun controller | ||
labels[pipeline.GroupName+pipeline.PipelineLabelKey] = getName(pipelineName, i) | ||
labels[pipeline.GroupName+pipeline.PipelineRunLabelKey] = getName(pipelineRunName, i) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the significance of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah those are static labels added by the controller, but definitely could use a comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment two lines up in 37ed488 |
||
assertLabelsMatch(t, labels, r.ObjectMeta.Labels) | ||
|
||
// Check label propagation to Pods. This label is added to every Pod by the TaskRun controller | ||
labels[pipeline.GroupName + pipeline.TaskRunLabelKey] = taskRunName | ||
assertLabelsMatch(t, labels, getPodForTaskRun(t, c.KubeClient, namespace, r).ObjectMeta.Labels) | ||
} | ||
|
||
matchKinds := map[string][]string{"PipelineRun": {prName}, "TaskRun": expectedTaskRunNames} | ||
|
@@ -322,10 +329,12 @@ func getPipelineRunSecret(suffix int, namespace string) *corev1.Secret { | |
} | ||
|
||
func getHelloWorldPipelineRun(suffix int, namespace string) *v1alpha1.PipelineRun { | ||
return tb.PipelineRun(getName(pipelineRunName, suffix), namespace, tb.PipelineRunSpec( | ||
getName(pipelineName, suffix), | ||
tb.PipelineRunServiceAccount(fmt.Sprintf("%s%d", saName, suffix)), | ||
)) | ||
return tb.PipelineRun(getName(pipelineRunName, suffix), namespace, | ||
tb.PipelineRunLabel("hello-world-key", "hello-world-value"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This just adds a custom label to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks good. Thank you @dwnusbaum |
||
tb.PipelineRunSpec(getName(pipelineName, suffix), | ||
tb.PipelineRunServiceAccount(fmt.Sprintf("%s%d", saName, suffix)), | ||
), | ||
) | ||
} | ||
|
||
func getName(namespace string, suffix int) string { | ||
|
@@ -370,3 +379,25 @@ func collectMatchingEvents(kubeClient *knativetest.KubeClient, namespace string, | |
} | ||
} | ||
} | ||
|
||
func getPodForTaskRun(t *testing.T, kubeClient *knativetest.KubeClient, namespace string, tr *v1alpha1.TaskRun) *corev1.Pod { | ||
// The Pod name has a random suffix, so we filter by label to find the one we care about. | ||
pods, err := kubeClient.Kube.CoreV1().Pods(namespace).List(metav1.ListOptions{ | ||
LabelSelector: pipeline.GroupName + pipeline.TaskRunLabelKey + " = " + tr.Name, | ||
}) | ||
if err != nil { | ||
t.Fatalf("Couldn't get expected Pod for %s: %s", tr.Name, err) | ||
} | ||
if numPods := len(pods.Items); numPods != 1 { | ||
t.Fatalf("Expected 1 Pod for %s, but got %d Pods", tr.Name, numPods) | ||
} | ||
return &pods.Items[0] | ||
} | ||
|
||
func assertLabelsMatch(t *testing.T, expectedLabels, actualLabels map[string]string) { | ||
for key, expectedVal := range expectedLabels { | ||
if actualVal := actualLabels[key]; actualVal != expectedVal { | ||
t.Errorf("Expected labels containing %s=%s but labels were %v", key, expectedVal, actualLabels) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seemed like the least bad place for the docs, but I don't know if it really fits. Any suggestions are welcome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i had trouble thinking of a good place myself! the only other alternative i can think of is to create a new doc like
troubleshooting.md
orfaq.md
ortips.md
or something but none of those options seem great either 🤔 this seems fine for now tho if you don't like any of those ideas!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it seems kind of weird to create a new file just for this information. My preference would be to leave it for now, and then if we end up with other troubleshooting-esque docs we can refactor them into a single place later. Does that seem ok?