-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 stable names for GH workflow jobs #9552
Conversation
.github/workflows/main.yml
Outdated
- name: make nodeup examples test | ||
working-directory: ${{ env.GOPATH }}/src/k8s.io/kops | ||
run: | | ||
make nodeup examples test |
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.
We can probably skip making nodeup on MacOS.
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.
Good point. Done.
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.
Any idea why we only build nodeup
and not all
?
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 suspect this is just historical about the way these tests were introduced (they're still fairly new); I suspect we were just trying to replace tests in travis.
From that point of view I really like splitting out the tests explicitly. What we really want on macos is coverage of the kops CLI (I don't think we want nodeup tests to run on macos; there's a case for windows in the future but I don't know of anyone trying to get kubelet to run on macos)
For linux, I think trying everything seems reasonable! We should probably be trending towards running tests in bazel, as more and more of our release pipeline is using bazel (I have some exciting news on this front for our next office hours :-) )
I think it's fine to rearrange what these do a bit in a separate PR.
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.
Looking forward to that :).
This removes Go 1.13. Is that because it's currently redundant with the Prow jobs and we expect to drop it as a requirement when we move Prow to 1.14? |
Yes. Also, when Go 1.15 is released we can add an optional build-next job. |
6ce82ad
to
48f50c7
Compare
this looks good to me. I agree we can have optional jobs for the next or previous go versions. I know i've asked this before, but to confirm here, are we safe to merge this with prow configured as-is and will it affect other open PRs? and what will the rollout to enforce these jobs look like exactly? |
@rifelpet open PRs would behave like current release branches, expecting these tests, but will not block for now. |
/approve Github testing isn't entirely happy, so this is also a nice test of whether we're going to break other PRs (I think!) |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman, justinsb 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 |
/retest |
…pstream-release-1.18 Automated cherry pick of #9552: Use stable names for GH workflow jobs
We will need to pass the job names to Tide to make them blocking, so stable names would be required.