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
add go:build no windows for some control-plane related UT for kubeadm #112620
Conversation
@pacoxu: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
When I look into https://storage.googleapis.com/k8s-triage/index.html?text=kubeadm, most are about windows ut. |
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.
/approve
@claudiubelu
could you please help us with review / lgtm here
428bf07
to
f649c5e
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neolit123, pacoxu 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 |
Other fixes for the CI can be found in https://github.com/orgs/kubernetes/projects/99/views/1 |
f649c5e
to
fc80905
Compare
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.
another question here is - should we adapt the tests to work on windows instead of not compiling them?
fc80905
to
2414457
Compare
In #111534, the fix is adding As we always run the work node on windows(no control plane node, correct me if I am wrong), I think some cases should never run on windows, so we can simply not compile them. For other cases, I think we should adopt the tests and some are in progress of #111534 and #110863. |
yes, we do not support windows control planes yet. but actually there some discussions about it with sig windows. one problem if we don't compile the tests is that if we eventually add windows cp support we will forget about these tests. using GOOS might be better here, but i haven't checked how much work it is. if you think we should not compile and better to merge this pr, please add a comment in here |
/lgtm |
It can be easily reflected in code coverage with Windows easily like https://testgrid.k8s.io/sig-testing-canaries#ci-kubernetes-coverage-unit later if we support windows cp. /hold cancel |
What type of PR is this?
/kind failing-test
/kind cleanup
What this PR does / why we need it:
https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-unit-windows-master/1569279943479660544
The failure
Add
//go:build !windows
to files below, as we always run work node on windows(no control plane node)Which issue(s) this PR fixes:
parts of #112619
Special notes for your reviewer:
Some more test failures
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: