Skip to content

Conversation

@aravindhp
Copy link
Contributor

Initial enhancement proposal for productization of Windows Containers that will enable running of Windows workloads on an OpenShift cluster.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 3, 2019
@aravindhp
Copy link
Contributor Author

Copy link
Member

@sdodson sdodson left a comment

Choose a reason for hiding this comment

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

lgtm for 4.3, provides flexibility to adapt in the future

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 6, 2019
@aravindhp aravindhp force-pushed the winc-productization branch 2 times, most recently from aa75653 to ae78830 Compare September 9, 2019 19:41
## Summary

The intent of this enhancement is to allow a cluster administrator to add a
Windows worker node with a prescribed configuration to an OpenShift cluster as a
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/worker/compute/, e.g. see the old openshift/installer#1330. Overhauling existing wording it hard, but no reason new stuff can't use the new wording :).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make that change in all places. So is it compute Ignition or worker Ignition?

Copy link
Member

Choose a reason for hiding this comment

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

Unless the machineconfigpool is changing to compute I think we should leave this as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sdodson is your comment against s/worker/compute/ or just the Ignition case?

Copy link
Member

Choose a reason for hiding this comment

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

just in relation to Ignition

### Goals

As part of this enhancement we plan to do the following:
* Prepare an already provisioned Windows worker node to join the cluster
Copy link
Member

Choose a reason for hiding this comment

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

Clarify the boundary between things the end-user does (just create a machine running Windows) and things Red Hat (tooling) does (install a kubelet, etc. etc.)? Currently that's all swept into "provision" and "Prepare". Maybe something like:

As part of this enhancement we plan to provide workflows for installing and upgrading OpenShift compute components (kubelet, OVN, and the Windows Machine Config Bootstrapper) on user-provided Windows machines. It will be up to the cluster administrator to initiate both installs and upgrades.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will go with your suggestion.


The inputs that the WMCB requires are:
* kubelet location on the local disk
* Worker ignition location on the local disk
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/ignition/Ignition/

We plan to have all the repositories associated with this effort fully
integrated with Prow CI and run e2e tests for every PR that is opened. These
e2e tests will involve bringing up a cluster on all supported cloud providers,
instantiating a Windows node and running workloads on it. We also plan to add
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't cover upgrade/downgrade testing. Probably call that out specifically and say whether or not you plan on covering it in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are not planning on covering upgrade/downgrade testing using CI in the 4.3 timeframe. I will mention that.


### Version Skew Strategy

We plan to maintain kubelet major version parity with the Linux counterpart.
Copy link
Member

Choose a reason for hiding this comment

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

You'll have to have skew during upgrades across the boundary though. Maybe say something about having CI coverage when we get to that point so you can ensure that at least some Windows kubelets will successfully handle lagging by one major version behind the rest of the cluster. Or say that before a major bump folks might have to drain/wipe their Windows compute and re-attach them as fresh compute after the major bump?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are planning on advising the customer to exactly what is done in the BYO RHEL scenario i.e. drain the Windows compute nodes using the Ansible playbook. I will add a line about that.

Initial enhancement proposal for productization of Windows Containers
that will enable running of Windows workloads on an OpenShift cluster.
@aravindhp
Copy link
Contributor Author

@wking I have addressed your comments. Please take a look.

@ravisantoshgudimetla
Copy link
Contributor

@crawford - Can you LGTM the PR?

@sdodson
Copy link
Member

sdodson commented Sep 18, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 18, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aravindhp, crawford, sdodson

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-merge-robot openshift-merge-robot merged commit f796b93 into openshift:master Sep 18, 2019

The actions that the WMCB will perform are:
* Install / upgrade and configure the kubelet
* Parse the worker Ignition and extract the bootstrap kubeconfig and the
Copy link
Member

Choose a reason for hiding this comment

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

kubelet configuration varies for linux versus windows hosts, this has to be a separate pool.

Copy link
Member

Choose a reason for hiding this comment

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

It's only used for the cluster coordinates and bootstrapping credentials.

openshift-merge-robot pushed a commit that referenced this pull request Oct 18, 2021
English mistake: sums up => adds up
cgwalters pushed a commit to cgwalters/enhancements that referenced this pull request Oct 19, 2021
Updates for master -> main branch renamings
jaypoulz pushed a commit to jaypoulz/enhancements that referenced this pull request Feb 26, 2025
OCPEDGE-1458: Addressing post-architecture review feedback
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants