Skip to content
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.

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

WINC-482: Bring Your Own Windows Host design #608

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

aravindhp
Copy link
Contributor

Enhancement proposal for adding BYOH support to WMCO.

@aravindhp
Copy link
Contributor Author

/cc @sdodson

@aravindhp
Copy link
Contributor Author

/cc @openshift/openshift-team-windows-containers

@aravindhp
Copy link
Contributor Author

/retest

@mansikulkarni96
Copy link
Member

Thanks for getting us started on BYOH @aravindhp
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2021
## Motivation

The world of Windows sees customers treating their instances more as "pets"
rather than "cattle". There is a desire from customers to be able to reuse these
Copy link
Member

Choose a reason for hiding this comment

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

What do we mean by reuse? Are the hosts going to be used for other purposes in addition to being OpenShift nodes? Do we have to worry about having substantial system reserved resources for non Kube managed services?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I understand from @Anandnatraj customers have existing Windows instances that they would like to reuse as Windows workers. But you have a good point about having existing resources reserved for non-k8s serves which could cause issues for the kubelet. However the following stories should allow us to assign priorities to the k8s services:
React to setting kubelet priority flag
React to setting hybrid-overlay priority flag

* Installing the container runtime in the Windows instance
* Managing the Windows operating system(OS) including handling of OS updates
* OpenShift Builds
* OpenShift DeploymentConfigs
Copy link
Member

Choose a reason for hiding this comment

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

Should we call out that this doesn't enable cross provider support, ie: vSphere IPI + Baremetal Windows Nodes? This comes up because cloud provider support is expected for all nodes and if you have some nodes with cloud provider support and some without then your cluster needs to be of platform None. I don't know to what extent windows nodes make use of cloud provider attach/detach of volumes so maybe that's not relevant to Windows nodes?

What about remote Windows worker nodes?

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 call out that we only support adding Windows instances are attached to the same network that other Linux workers are attached to in the goal section. That should cover the cases you are talking about. We are trying hard to not muddy the waters by saying IPI, UPI or Bare Metal by saying BYOWH (Bring Your Own Windows Hosts).

* Delete the payload binaries
* If the list of nodes has IP addresses that do not map to any in the list of
instances, and WMCO is unable to access the instance, it will assume that
the node in question is not managed by it.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should annotate or label the BYO windows nodes as being WMCO managed during the installation process so that we're more sure we have the authority to uninstall.
Same considerations as above with regard to privileges to manage node objects.

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 already have a couple of annotations for version and sshKey. Do you suggest having a separate annotation to indicate it is managed by WMCO?

Copy link
Contributor Author

@aravindhp aravindhp left a comment

Choose a reason for hiding this comment

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

Thanks for the review, @sdodson

## Motivation

The world of Windows sees customers treating their instances more as "pets"
rather than "cattle". There is a desire from customers to be able to reuse these
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I understand from @Anandnatraj customers have existing Windows instances that they would like to reuse as Windows workers. But you have a good point about having existing resources reserved for non-k8s serves which could cause issues for the kubelet. However the following stories should allow us to assign priorities to the k8s services:
React to setting kubelet priority flag
React to setting hybrid-overlay priority flag

* Installing the container runtime in the Windows instance
* Managing the Windows operating system(OS) including handling of OS updates
* OpenShift Builds
* OpenShift DeploymentConfigs
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 call out that we only support adding Windows instances are attached to the same network that other Linux workers are attached to in the goal section. That should cover the cases you are talking about. We are trying hard to not muddy the waters by saying IPI, UPI or Bare Metal by saying BYOWH (Bring Your Own Windows Hosts).

* Delete the payload binaries
* If the list of nodes has IP addresses that do not map to any in the list of
instances, and WMCO is unable to access the instance, it will assume that
the node in question is not managed by it.
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 already have a couple of annotations for version and sshKey. Do you suggest having a separate annotation to indicate it is managed by WMCO?

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2021
@aravindhp aravindhp force-pushed the windows-byoh branch 2 times, most recently from a6b4ad8 to 2c70a00 Compare February 2, 2021 22:06
@aravindhp
Copy link
Contributor Author

/retest

Enhancement proposal for adding BYOH support to WMCO.
@sdodson
Copy link
Member

sdodson commented Feb 5, 2021

/approve
This looks good, thanks.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sdodson

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

The pull request process is described 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 5, 2021
@mansikulkarni96
Copy link
Member

Thanks for working on this @aravindhp
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 5, 2021
@openshift-merge-robot openshift-merge-robot merged commit e591547 into openshift:master Feb 5, 2021
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.

5 participants