Skip to content

capz: remove WINDOWS and TEST_WINDOWS env vars #35701

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

capz: remove WINDOWS and TEST_WINDOWS env vars #35701

wants to merge 1 commit into from
+0 −46

Conversation

jackfrancis
Copy link
Contributor

When not requiring Windows infra for tests in CAPZ, you don't have to explicitly set any configuration. This simplifies the jobs so that explicit usages like WINDOWS=false or TEST_WINDOWS=false are removed.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 14, 2025
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/config Issues or PRs related to code in /config area/jobs area/provider/azure Issues or PRs related to azure provider sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/testing Categorizes an issue or PR as relevant to SIG Testing. wg/device-management Categorizes an issue or PR as relevant to WG Device Management. labels Oct 14, 2025
@jackfrancis jackfrancis force-pushed the capz-windows-remove-for-false branch from 98c4b79 to 1283b02 Compare October 14, 2025 21:17
nojnhuh
nojnhuh reviewed Oct 14, 2025
View reviewed changes
...s/kubernetes-sigs/cluster-api-provider-azure/cluster-api-provider-azure-presubmits-main.yaml
Comment on lines -747 to -748
- name: WINDOWS_SERVER_VERSION
value: "windows-2022"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep this (and below for this same variable) to prevent reverting back to 2019 for these jobs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this particular one, I'd prefer we just address this in CAPZ by making 2022 the default (it would seem to me that this particular test won't need to re-introduce Windows requirements before we do that?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nvm if we're not testing Windows here anyway it doesn't matter.

nojnhuh
nojnhuh approved these changes Oct 14, 2025
View reviewed changes
Copy link
Contributor

@nojnhuh nojnhuh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2025
@jackfrancis
Copy link
Contributor Author

/assign @mborsz @wojtek-t

(just a housekeeping PR but it will help keep our sanity managing all the CAPZ tests across many projects)

mboersma
mboersma approved these changes Oct 16, 2025
View reviewed changes
Copy link
Contributor

@mboersma mboersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jackfrancis, mboersma, nojnhuh
Once this PR has been reviewed and has the lgtm label, please ask for approval from wojtek-t. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mboersma
Copy link
Contributor

/assign @mm4tt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alaypatel07 alaypatel07 Awaiting requested review from alaypatel07

@marosset marosset Awaiting requested review from marosset

2 more reviewers

@mboersma mboersma mboersma approved these changes

@nojnhuh nojnhuh nojnhuh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mboersma mboersma

@mborsz mborsz

@mm4tt mm4tt

@wojtek-t wojtek-t

@nojnhuh nojnhuh

Labels

area/config Issues or PRs related to code in /config area/jobs area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. wg/device-management Categorizes an issue or PR as relevant to WG Device Management.

Projects

Dynamic Resource Allocation
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jackfrancis @k8s-ci-robot @mboersma @nojnhuh @mborsz @mm4tt @wojtek-t