-
Notifications
You must be signed in to change notification settings - Fork 1.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
feat: Add windows support to tools-releases #2984
feat: Add windows support to tools-releases #2984
Conversation
Welcome @codablock! |
Hi @codablock. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test I am ok with us adding this one. But is important to highlight that Kubebuilder itself does not support windows besides that be desired, see: #2940 |
/approved |
@jmrodri and @varshaprasad96 wdyt? |
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.
/lgtm
Agreed with @camilamacedo86, it would be nice to explicitly mention in docs that Kubebuilder has not been tested against Windows and its upto the developer to do so. Thanks for the contribution @codablock! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, codablock, jmrodri, varshaprasad96 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 |
Thanks for the review and merge :) |
Hi @codablock, Yes, we will need to enable it into the Google Cloud, see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/RELEASE.md#to-build-the-kubebuilder-tools-artifacts-required-to-use-env-test I change my email and we need help here from others to re-enable our access there. So, asap we will add it. You can follow up: #3005 |
This PR adds support for Windows builds in tools-releases, so that these can be used via setup-envtest. If this is merged, it will require enabling it in the GCP Cloud Build (I have no experience with that part).
I'm currently using the branch behind this PR in https://github.com/kluctl/kubebuilder-tools-releases-windows, which simply clones my kubebuilder fork, checks out the tools-releases-windows branch and manually runs the Docker build for the Windows release. I do this to trick setup-envtest into accepting the downloaded binaries when invoking it with
-i
, which will then later allow me to simply drop the manual downloading. You can find all this here: https://github.com/kluctl/kluctl/blob/main/Makefile#L60.When this PR gets merged and GCP Cloud Build is updated as well, I will then be able to remove the manual downloading and simply rely on setup-envtest.