Skip to content

WINC-805: Use Windows Server 2019 in aws-e2e-operator test#29791

Merged
openshift-ci[bot] merged 1 commit into
openshift:masterfrom
jrvaldes:add-windows-ami-filter-env-var
Jul 5, 2022
Merged

WINC-805: Use Windows Server 2019 in aws-e2e-operator test#29791
openshift-ci[bot] merged 1 commit into
openshift:masterfrom
jrvaldes:add-windows-ami-filter-env-var

Conversation

@jrvaldes
Copy link
Copy Markdown
Contributor

@jrvaldes jrvaldes commented Jun 23, 2022

Adds an environment variable to the aws-e2e-operator test to
cover Windows Server 2019 in one of the CI jobs, as it is a supported version.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 23, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jun 23, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jrvaldes jrvaldes force-pushed the add-windows-ami-filter-env-var branch from 11642a4 to 8de62f0 Compare June 23, 2022 16:26
@saifshaikh48
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 23, 2022
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jun 27, 2022
This commit leverage the environment variables introduced in openshift/release#29791
to allow a parameter-based configuration for the machine and container
images used in the e2e test suit.
@jrvaldes jrvaldes changed the title [WIP] WINC-805: Use Windows Server 2019 in aws-e2e-operator test WINC-805: Use Windows Server 2019 in aws-e2e-operator test Jun 28, 2022
@jrvaldes jrvaldes marked this pull request as ready for review June 28, 2022 13:12
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2022
@saifshaikh48
Copy link
Copy Markdown
Contributor

/remove-lgtm

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2022
@jrvaldes jrvaldes force-pushed the add-windows-ami-filter-env-var branch from 8de62f0 to ae817eb Compare June 28, 2022 17:54
Comment on lines 91 to 92
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO this is embedding too much logic within the release repo. We shouldn't need to open a release PR if we want to change the container image being used. I'd suggest sticking with a single env var which if set indicates 2019 should be used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Everything is about context and in this context, we need a specific variation of the configuration for one of the tests. The required logic could be explicit (as proposed) or implicit (defined by a flag as you recommend). A single env var should have enough context to explain its usage which in this case may complicate its definition.

For example, the name of the single env var could be WINDOWS_SERVER_2019_ENABLED which is tightened to a specific version of Windows Server (2019). The latter, cannot be extended to other versions, where we may end up with multiple env vars to support multiple versions, followed by several conditional statements in the WMCO's e2e code.

To change the "default" container image being used, no release PR is needed. To adjust the corner case, yes. Both cases, a PR is required.

The introduction of the VM_TEMPLATE env var for vSphere, was the first stepping stone towards moving the Windows Server image definition to the release repo for the e2e test. I feel the proposed env vars align with that approach.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would be more okay with this if it was just describing infrastructure, like the vSphere tests you point to do. But its not just infrastructure, you're moving part of the test workload definition to the release job as well with WINDOWS_CONTAINER_IMAGE. What happens if we need to include more changes to the tests which are different for 2019/2022, such as using runtimeclasses?

I want to avoid more logic being moved to the release repo. So I see two possible options here, either use a simple WINDOWS_SERVER_2019_ENABLED, or use the Windows node status to figure out the Windows server version. The first option is a lot simpler.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be honest, I lean towards the solution that requires no release repo changes. It seems doable to keep everything configured through the WMCO repo's e2e test suite.

One suggestion is to replace tc.hasCustomVXLAN with a flag isWindowsServer2022 in the testContext struct, setting it at the time of machineset creation in the e2e test suite. Might require some parsing of the machine set spec to see the OS version of the image being used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, let me adjust that.

@jrvaldes jrvaldes force-pushed the add-windows-ami-filter-env-var branch from ae817eb to be0539a Compare June 29, 2022 12:13
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jun 30, 2022
This commit leverage the environment variables introduced in openshift/release#29791
to allow a parameter-based configuration for the machine and container
images used in the e2e test suit.
saifshaikh48 pushed a commit to saifshaikh48/windows-machine-config-operator that referenced this pull request Jun 30, 2022
This commit leverage the environment variables introduced in openshift/release#29791
to allow a parameter-based configuration for the machine and container
images used in the e2e test suit.

(cherry picked from commit 419d49b)
@jrvaldes jrvaldes marked this pull request as draft July 1, 2022 13:50
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 1, 2022
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 5, 2022
This commit leverage the environment variables introduced in openshift/release#29791
to allow a parameter-based configuration for the machine and container
images used in the e2e test suit.
@jrvaldes jrvaldes force-pushed the add-windows-ami-filter-env-var branch from be0539a to d28f001 Compare July 5, 2022 16:18
This commit adds an environment variable to the aws-e2e-operator test to
cover Windows Server 2019 in one of the CI jobs, as it is a supported version.
@jrvaldes jrvaldes force-pushed the add-windows-ami-filter-env-var branch from d28f001 to f27f329 Compare July 5, 2022 17:39
@jrvaldes jrvaldes marked this pull request as ready for review July 5, 2022 17:40
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 5, 2022
@openshift-ci openshift-ci Bot requested a review from selansen July 5, 2022 17:47
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 5, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 5, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
@sebsoto
Copy link
Copy Markdown
Contributor

sebsoto commented Jul 5, 2022

/approve

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 5, 2022
@saifshaikh48
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2022
@mansikulkarni96
Copy link
Copy Markdown
Member

/lgtm

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jrvaldes, mansikulkarni96, saifshaikh48, sebsoto

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci Bot merged commit 85b6a04 into openshift:master Jul 5, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 5, 2022

@jrvaldes: Updated the following 4 configmaps:

  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-windows-machine-config-operator-master.yaml using file ci-operator/config/openshift/windows-machine-config-operator/openshift-windows-machine-config-operator-master.yaml
  • ci-operator-4.11-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-windows-machine-config-operator-release-4.11.yaml using file ci-operator/config/openshift/windows-machine-config-operator/openshift-windows-machine-config-operator-release-4.11.yaml
  • ci-operator-4.12-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-windows-machine-config-operator-release-4.12.yaml using file ci-operator/config/openshift/windows-machine-config-operator/openshift-windows-machine-config-operator-release-4.12.yaml
  • step-registry configmap in namespace ci at cluster app.ci using the following files:
    • key windows-e2e-operator-test-ref.yaml using file ci-operator/step-registry/windows/e2e/operator/test/windows-e2e-operator-test-ref.yaml
Details

In response to this:

Adds two environment variables to the aws-e2e-operator test to
cover Windows Server 2019 in one of the CI jobs, as it is a supported version.

  • WINDOWS_AWS_AMI_FILTER is the filter used to find the Windows AMI
  • WINDOWS_CONTAINER_IMAGE is the Windows container image, must be compatible
    with the selected Windows AMI

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.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 5, 2022

@jrvaldes: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/rehearse/openshift/windows-machine-config-operator/release-4.12/aws-e2e-operator f27f329 link unknown /test pj-rehearse

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 5, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 6, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 9, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 19, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Jul 25, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 4, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 11, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 11, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 12, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 13, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without Docker container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 17, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Aug 17, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suit.

In addition, updates the logic to select the container image based
on the cloud provider, where the default container image must be
compatible with Windows Server 2022.

Finally, adjusts the PowerShell cmdlets for WS2022 to
avoid the escaping the double-quotes when default shell is
Powershell.
jrvaldes added a commit to jrvaldes/windows-machine-config-operator that referenced this pull request Nov 10, 2022
This commit updates the AWS's EC2 AMI to use Windows Server 2022
base image without container runtime. The container runtime
(containerd) is installed by WMCO. It also leverages the environment
variable introduced in openshift/release#29791
to allow a parameter-based configuration for Windows Server version,
where the machine and container images can be adjusted in the e2e
test suite.

In addition, adjusts the logic for PowerShell cmdlets to avoid escaping
the double-quotes when default shell is PowerShell to taking into account
the Windows Server version.
@jrvaldes jrvaldes deleted the add-windows-ami-filter-env-var branch April 2, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants