🐛 test/e2e/metrics_test.go: discover operator-controller namespace and improve logging#1547
Conversation
…improve logging Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1547 +/- ##
=======================================
Coverage 74.25% 74.25%
=======================================
Files 42 42
Lines 3329 3329
=======================================
Hits 2472 2472
Misses 676 676
Partials 181 181
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| t.Log("Determining operator-controller namespace") | ||
| cmd := exec.Command(client, "get", "pods", "--all-namespaces", "--selector=control-plane=operator-controller-controller-manager", "--output=jsonpath={.items[0].metadata.namespace}") | ||
| output, err := cmd.CombinedOutput() | ||
| require.NoError(t, err, "Error creating determining operator-controller namespace: %s", string(output)) |
There was a problem hiding this comment.
What if there are multiple outputs (namespaces) found?
There was a problem hiding this comment.
Yeah, I thought about that briefly. Ultimately if there are multiple pods that match the label that exist in different namespaces, we'll pick the first one.
If that's the "correct" namespace (where the SA and metrics service/endpoint) exist, the test will pass.
If it's not the right namespace, the test will fail, likely because the token generation fails because the SA doesn't exist, but if the SA exists in the "incorrect" namespace, it could still fail if there's no service there.
TL;DR: the test will fail if we choose an "incorrect" namespace.
There was a problem hiding this comment.
wouldn’t the output be along the lines of "namespace1\nnamespace2" which would mess up any use of output? I don’t think you’re just picking “one”.
Admittedly, this will totally mess up your --serviceaccount parameter, but we should error out early.
There was a problem hiding this comment.
No, because the output specifically selects the namespace of the first item in the returned list
--output=jsonpath={.items[0].metadata.namespace}
There was a problem hiding this comment.
Ah, yes, this was something you had also changed downstream... So you're going for the first one... hmm... But now I'm wondering how you did that downstream without this commit?
Edit: Ah... drop commit...
…improve logging (#1547) Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
…improve logging (operator-framework#1547) Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
…improve logging (operator-framework#1547) Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
Description
We should make our e2e's resilient to operator-controller being relocated or run in different namespaces. This PR removes the hardcoded 'olmv1-system' namespace assumption and instead discovers the namespace by finding the operator-controller pod via label selector and using its namespace.
This PR also:
Reviewer Checklist