-
Notifications
You must be signed in to change notification settings - Fork 55
Update e2e-resources and e2e for local Windows testing #1185
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
base: main
Are you sure you want to change the base?
Conversation
1533135
to
897bcfa
Compare
3631c0d
to
2b6f008
Compare
4c49647
to
35b7b07
Compare
add windows exceptions and specs add windows statefulset and reduce deployment resource reqs update test-specs to remove windows tests (moved to sep file)
35b7b07
to
b9ae878
Compare
| persistentVolumeClaim.enabled | bool | `true` | Create PVCs | | ||
| scraper.enabled | bool | `false` | Deploy the scraper pod | | ||
| statefulSet.enabled | bool | `true` | Deploy a dummy statefulSet | | ||
| windows.is2019 | bool | `false` | Deploy resources on Windows Server 2019 nodes | |
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.
Sorry to keep nit picking on this, but can we not use tolerations here? Really trying to avoid baking in new config options that we'll end up needing to support in the future. If the plan is to drop theses keys in a follow up PR because we're currently testing Windows, I'm okay with this. Just checking.
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'd like to keep the option in e2e-resources because we might not always want to deploy resources for windows. is there an expectation that we need to support e2e-resources for customers?
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 get that, but wouldn't the windows resources not deploy anyways due to tolerations? I don't think we do need to have customer support on this, it's more that I think we should minimize configs that may be "redundant".
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.
because they are deployments they deploy and are stuck in pending because they cant' find a node.
if they were daemonsets they would not deploy
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.
only if the cluster the user is deploying e2e-resources to doesn't have windows nodes! our canaries, for example, have a windows node by default, so it's possible someone's just trying to test something real quick in linux nodes. i'd rather have more levers to pull to target testing, like we do with whether or not we want deployments/statefulsets/daemonsets in general.
Description
This PR adds several resources to the e2e-resources chart that deploy onto Windows-based nodes (Windows Server 2019 & Windows Server 2022) when enabled.
Resource types include
It also includes e2e tests meant to run locally, separately from the standard e2e tests. Our current automated e2e tests run using Minikube, which does not support Windows nodes, so we follow the basic local test script pointing at a different exceptions file & specs file for Windows. Automating this will be a little involved, and will come later.
Note that several tests present for Linux are not present for Windows due to known limitations in available metrics (eg most
netTx
/netRx
/netErrors
), tests that are not relevant for Windows (anything to do withcpuCfs
), & some that I haven't looked into yet (PVC, container pending). This should cover the bulk of the tests, though.Known issues:
Type of change
Checklist: