WINC-1869: Drop support for Windows Server 2019 in CI and machineset.sh script#3995
WINC-1869: Drop support for Windows Server 2019 in CI and machineset.sh script#3995jrvaldes wants to merge 2 commits intoopenshift:masterfrom
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Excluded labels (none allowed) (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request removes Windows Server 2019 support across the Windows Machine Config Operator codebase. Changes include: removing 2019 version checks from the machine set configuration script (now only accepts 2022 or 2025), eliminating 2019-specific AMI filters and image configurations for AWS, Azure, and GCP providers, updating container image selection to default to 2022, removing the 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jrvaldes The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
jrvaldes
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@jrvaldes: This pull request references WINC-1869 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@jrvaldes: This pull request references WINC-1869 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/machineset.sh`:
- Around line 429-431: The catch-all case handling invalid CLI options in the
case pattern '\? )' currently exits with 0; change it to exit with non-zero to
indicate error (use exit 1) and print the usage message to stderr (redirect echo
to >&2) so scripts/CI detect failure; update the '\? )' block in
hack/machineset.sh that emits "Usage: $0 -w <2022/2025> -b apply/delete" to echo
to stderr and call exit 1.
- Around line 421-423: The regex in the if check that validates the winver
variable currently uses unanchored alternation and allows partial matches;
update the conditional that tests "$winver" (the if [[ ! "$winver" =~ 2022|2025$
]] block) to use a fully anchored alternation such as ^(2022|2025)$ so only
exact values "2022" or "2025" pass; keep the same quoting and exit behavior but
replace the regex with the anchored pattern.
In `@test/e2e/windows/version.go`:
- Line 14: The help text for the e2e flag that advertises supported server
versions is out of sync with SupportedVersions; locate the flag definition in
test/e2e/main_test.go (the help string around the -server-version/usage text)
and update the message to remove “2019” so it advertises only “2022 or 2025” (or
otherwise reflect Server2022 and Server2025), ensuring the flag usage string
matches the SupportedVersions slice and any related validation logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 98885134-a9b7-4dbc-8284-9d4c61104e9c
📒 Files selected for processing (7)
hack/machineset.shtest/e2e/network_test.gotest/e2e/providers/aws/aws.gotest/e2e/providers/azure/azure.gotest/e2e/providers/gcp/gcp.gotest/e2e/validation_test.gotest/e2e/windows/version.go
💤 Files with no reviewable changes (4)
- test/e2e/providers/azure/azure.go
- test/e2e/providers/gcp/gcp.go
- test/e2e/network_test.go
- test/e2e/providers/aws/aws.go
| if [[ ! "$winver" =~ 2022|2025$ ]]; then | ||
| echo "Invalid -w option $winver. Valid options are 2022 or 2025" | ||
| exit 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the current regex in the script
rg -n 'winver.*=~' hack/machineset.sh
# Demonstrate current behavior of the same regex expression
pattern='2022|2025$'
for v in 2022 2025 abc2022 20220 x2025y; do
if [[ "$v" =~ $pattern ]]; then
echo "accepted: $v"
else
echo "rejected: $v"
fi
done
# Expected: malformed values should be rejected, but current pattern accepts some of them.Repository: openshift/windows-machine-config-operator
Length of output: 216
🏁 Script executed:
sed -n '410,435p' hack/machineset.shRepository: openshift/windows-machine-config-operator
Length of output: 621
Fix -w regex anchoring to reject malformed Windows versions.
Line 421 uses 2022|2025$, which accepts partial matches like abc2022 and 20220. Fully anchor the alternation to ensure only exact matches for valid Windows Server versions.
Proposed fix
- if [[ ! "$winver" =~ 2022|2025$ ]]; then
+ if [[ ! "$winver" =~ ^(2022|2025)$ ]]; then
echo "Invalid -w option $winver. Valid options are 2022 or 2025"
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ ! "$winver" =~ 2022|2025$ ]]; then | |
| echo "Invalid -w option $winver. Valid options are 2022 or 2025" | |
| exit 1 | |
| if [[ ! "$winver" =~ ^(2022|2025)$ ]]; then | |
| echo "Invalid -w option $winver. Valid options are 2022 or 2025" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/machineset.sh` around lines 421 - 423, The regex in the if check that
validates the winver variable currently uses unanchored alternation and allows
partial matches; update the conditional that tests "$winver" (the if [[ !
"$winver" =~ 2022|2025$ ]] block) to use a fully anchored alternation such as
^(2022|2025)$ so only exact values "2022" or "2025" pass; keep the same quoting
and exit behavior but replace the regex with the anchored pattern.
There was a problem hiding this comment.
@jrvaldes, understood — the regex anchoring concern is pre-existing and not related to the 2019 removal in scope here. Would you like me to open a follow-up GitHub issue to track the fix (^(2022|2025)$) so it doesn't get lost?
| \? ) | ||
| echo "Usage: $0 -w <2019/2022> -b apply/delete" | ||
| echo "Usage: $0 -w <2022/2025> -b apply/delete" | ||
| exit 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Static verification of the invalid-option branch behavior.
rg -n -C2 '\\\? \)|Usage: .*2022/2025|exit 0|exit 1' hack/machineset.shRepository: openshift/windows-machine-config-operator
Length of output: 405
Return non-zero exit code for invalid CLI option usage.
Line 431 exits with 0 (success) when invalid options are provided, which masks failures in CI/scripts. Invalid CLI usage should signal an error. Note the inconsistency: line 423 correctly exits with 1 for invalid argument values, but the catchall \? ) pattern exits with 0.
Proposed fix
\? )
echo "Usage: $0 -w <2022/2025> -b apply/delete"
- exit 0
+ exit 1
;;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| \? ) | |
| echo "Usage: $0 -w <2019/2022> -b apply/delete" | |
| echo "Usage: $0 -w <2022/2025> -b apply/delete" | |
| exit 0 | |
| \? ) | |
| echo "Usage: $0 -w <2022/2025> -b apply/delete" | |
| exit 1 | |
| ;; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/machineset.sh` around lines 429 - 431, The catch-all case handling
invalid CLI options in the case pattern '\? )' currently exits with 0; change it
to exit with non-zero to indicate error (use exit 1) and print the usage message
to stderr (redirect echo to >&2) so scripts/CI detect failure; update the '\? )'
block in hack/machineset.sh that emits "Usage: $0 -w <2022/2025> -b
apply/delete" to echo to stderr and call exit 1.
There was a problem hiding this comment.
@jrvaldes, understood — this is a pre-existing issue unrelated to the WS2019 removal. Would you like me to open a follow-up GitHub issue to track fixing the exit 0 → exit 1 behavior in the \? case of hack/machineset.sh?
Remove all 2019 cases from AWS, Azure, GCP, and vSphere MachineSet generation. Update option validation and usage to accept only 2022 and 2025.
This commit removes Server2019 from supported versions, provider image selectors,container image selection, and PowerShell compatibility workarounds. All available Azure Marketplace images for the 2019-datacenter-smalldisk SKU have the OpenSSH regression introduced in KB5044277 (Oct 2024, OS Build 17763.6414). The previously pinned version 17763.6293.240905 has been deprecated and removed. Windows Server 2019 mainstream support ended January 2024.
jrvaldes
force-pushed
the
drop-ws2019-support
branch
from
f319489 to
0011f27
Compare
|
bocking PR merged |
|
/test ? |
|
/test azure-e2e-operator |
1 similar comment
|
/test azure-e2e-operator |
|
@jrvaldes: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
2 participants
This PR removes Server2019 from supported versions, provider image
selectors,container image selection, and PowerShell compatibility workarounds.
All available Azure Marketplace images for the 2019-datacenter-smalldisk
SKU have the OpenSSH regression introduced in KB5044277 (Oct 2024, OS
Build 17763.6414). The previously pinned version 17763.6293.240905 has
been deprecated and removed. Windows Server 2019 mainstream support
ended January 2024.
Summary by CodeRabbit