Skip to content

WINC-805: Add support for Windows Server 2022 in AWS#1091

Closed
jrvaldes wants to merge 4 commits into
openshift:masterfrom
jrvaldes:WINC-805
Closed

WINC-805: Add support for Windows Server 2022 in AWS#1091
jrvaldes wants to merge 4 commits into
openshift:masterfrom
jrvaldes:WINC-805

Conversation

@jrvaldes
Copy link
Copy Markdown
Contributor

@jrvaldes jrvaldes commented Jun 6, 2022

This PR updates the AWS's EC2 AMI to use Windows Server 2022
Base image without Docker container runtime, as the container runtime
(containerd) is installed by WMCO.

Leverage the environment variable introduced in the release job configuration to use Windows Server 2019 as machine and container images for the aws-e2e-operator test

@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 6, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jun 6, 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

@mansikulkarni96
Copy link
Copy Markdown
Member

/test azure-e2e-operator

@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 7, 2022

/test aws-e2e-operator

@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 7, 2022

/test lint

@mansikulkarni96
Copy link
Copy Markdown
Member

/test aws-e2e-upgrade

Copy link
Copy Markdown
Contributor

@saifshaikh48 saifshaikh48 left a comment

Choose a reason for hiding this comment

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

Code changes lgtm, question on docs

Comment thread docs/machineset-aws.md Outdated
Comment thread docs/machineset-aws.md Outdated
@selansen
Copy link
Copy Markdown
Contributor

selansen commented Jun 7, 2022

Can you pls add a few lines in the description section?

@mansikulkarni96
Copy link
Copy Markdown
Member

Don't we need to update the image deployed for networking test here

@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 7, 2022

Don't we need to update the image deployed for networking test here

Yes, addressed in 8abf347

@jrvaldes jrvaldes force-pushed the WINC-805 branch 2 times, most recently from 8abf347 to 7193a6d Compare June 7, 2022 21:52
@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 8, 2022

/test aws-e2e-operator

2 similar comments
@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 8, 2022

/test aws-e2e-operator

@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 9, 2022

/test aws-e2e-operator

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 9, 2022
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 9, 2022
@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Jun 9, 2022

/test aws-e2e-operator

@jrvaldes jrvaldes force-pushed the WINC-805 branch 2 times, most recently from 1968bfd to 004f74b Compare June 13, 2022 16:13
@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-operator

@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-upgrade

1 similar comment
@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-upgrade

@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-operator

Build src image failed

@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Aug 4, 2022

/test aws-e2e-operator

@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Aug 5, 2022

/test aws-e2e-ccm-install

@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-upgrade

@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-upgrade

@jrvaldes jrvaldes marked this pull request as ready for review August 11, 2022 07:46
@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 Aug 11, 2022
@openshift-ci openshift-ci Bot requested review from aravindhp and removed request for selansen August 11, 2022 07:51
@jrvaldes
Copy link
Copy Markdown
Contributor Author

Rebased with:

@jrvaldes jrvaldes force-pushed the WINC-805 branch 3 times, most recently from 92c0a91 to 64f5930 Compare August 13, 2022 20:20
Comment thread pkg/nodeconfig/nodeconfig.go Outdated
patchData, meta.PatchOptions{})
if err != nil {
return errors.Wrapf(err, "error removing version annotation from node %s", nc.node.GetName())
// Check version annotation and remove iff present
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.

How does this commit relate to adding Server 2022 support?

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.

Removed.

Comment thread pkg/instance/instance.go Outdated
@jrvaldes
Copy link
Copy Markdown
Contributor Author

jrvaldes commented Aug 17, 2022

This commit declares support for Windows Server 2022 in the
documentation.
This commit updates the hack/machineset.sh script to use
Windows Server 2022 base image without containers for AWS
machineSets.

In addition, re-arrange the aws cli command that fetch the
AMI id with a multiline for better readability.
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
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-upgrade

@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-operator

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 10, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jrvaldes
Once this PR has been reviewed and has the lgtm label, please assign aravindhp for approval by writing /assign @aravindhp in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

This change introduces a hack to circumvent the routing issue in
Windows Server 2022 running the EC2Launch V2 agent where the
instance metadata endpoint is not available after hybrid-overlay
is running as a Windows service. See ovn-kubernetes/ovn-kubernetes#3119
@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-upgrade

@jrvaldes
Copy link
Copy Markdown
Contributor Author

/test aws-e2e-operator

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 10, 2022

@jrvaldes: The following tests 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/prow/vsphere-e2e-operator c312195 link true /test vsphere-e2e-operator
ci/prow/gcp-e2e-operator c312195 link true /test gcp-e2e-operator
ci/prow/platform-none-vsphere-e2e-operator c312195 link true /test platform-none-vsphere-e2e-operator
ci/prow/aws-e2e-upgrade c77dc9d link true /test aws-e2e-upgrade

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.

@aravindhp
Copy link
Copy Markdown
Contributor

/close

This hack does not work. For one EC2Launch v2 is a service and we should be restarting that and not just executing the binary. Further more restarting the service does not result in the routes being recreated as the services loops waiting for an IP address:

2023-02-03 00:33:31 Console: Network interface configuration is empty...waiting for an IP...
2023-02-03 00:34:35 Console: Network interface configuration is empty...waiting for an IP...

@openshift-merge-robot
Copy link
Copy Markdown
Contributor

PR needs rebase.

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.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Feb 4, 2023

@aravindhp: Closed this PR.

Details

In response to this:

/close

This hack does not work. For one EC2Launch v2 is a service and we should be restarting that and not just executing the binary. Further more restarting the service does not result in the routes being recreated as the services loops waiting for an IP address:

2023-02-03 00:33:31 Console: Network interface configuration is empty...waiting for an IP...
2023-02-03 00:34:35 Console: Network interface configuration is empty...waiting for an IP...

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.

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

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants