-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Use debug stdout callback in ci rather than manual debug #11793
Use debug stdout callback in ci rather than manual debug #11793
Conversation
This is a followup to 2ba28a3 (Revert "Wait for available API token in a new namespace (kubernetes-sigs#7045)", 2024-10-25). While checking for the serviceaccount token is not effective, there is still a race when creating a Pod directly, because the ServiceAccount itself might not be created yet. More details at kubernetes/kubernetes#66689. This cause very frequent flakes in our CI with spurious failures. Use a Deployment instead ; it will takes cares of creating the Pods and retrying ; it also let us use kubectl rollout status instead of manually checking for the pods.
There is no pods with hostNetwork deployed in this test, and therefore the tasks are skipped / empty output (checked in CI).
/ok-to-test |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: VannTen The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a0dcc09
to
ab53a29
Compare
/label tide/merge-method-merge |
3f16e58
to
b44a731
Compare
This display in a readable (by humans) way the result of most tasks, and should be way more readable that what we have now, which is frequently a bunch of unreadable json. + some small fixes (using delegated_to instead of when <control_plane_host>)
We don't test Flatcar at all in CI, thus remove special handling for it.
b44a731
to
86a949d
Compare
set_fact: | ||
bin_dir: "/usr/local/bin" | ||
when: not ansible_os_family in ["Flatcar", "Flatcar Container Linux by Kinvolk"] | ||
|
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 think the removal of the Flatcar test case is acceptable, although it's somewhat regrettable. It would be great if we could add the Flatcar test case in the future.
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.
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 I don't mind adding a test ; but in the meantime, we should not adjust for things we don't actually test.
This isn't hard to revert later if needed.
/hold cancel |
/lgtm It's worth mentioning that I am also a fan of the yaml stdout callback plugin which is usually quite human readable, but perhaps debug makes more sense for CI. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
CI: Use the debug stdout callback instead of manual debug
This display in a readable (by humans) way the result of most tasks, and
should be way more readable that what we have now, which is frequently a
bunch of unreadable json.
+ some small fixes
This is on top of #11792 so the other one should be merged first
/hold
Does this PR introduce a user-facing change?: