Add cluster-api integration enhancement #913

alexander-demicev · 2021-09-27T15:52:30Z

No description provided.

elmiko

good start, i have a few questions inline, also i'm curious about how a user might use CAPI on OpenShift. will we allow users to create new clusters from the CAPI on their OpenShift install? (essentially allowing them to use OpenShift as the management cluster for new workload clusters)

enhancements/machine-api/cluster-api-integration.md

enxebre · 2021-10-13T16:10:58Z

enhancements/machine-api/cluster-api-integration.md

+
+In order to maintain the lifecycle of Cluster API related resources, we will create a new operator `cluster-capi-operator`, this name was chosen for avoiding confusion with upstream Cluster API operator.
+This operator will be responsible for all administrative tasks related to the deployment of the Cluster API project within the cluster.
+During tech preview phase, the new operator will also manage all Cluster API related CRDs. All CRD manifests will placed in openshift forks of CAPI and will take from there with no changes.


FWIW in hypershift will install these CRDs in clusters that it runs.

you will need to change the CRD to convert from cert-manager to service-ca (minor). Also not convinced that within in each fork is the best location as it's going to make rebasing harder. So far I have done this in my pr here openshift/cluster-capi-operator#8 .
Argument for this:

the operator controls the version of each provider so having the manifests seems reasonable

reduces the rebase difficulty in each provider

can do translations from cert-manager to service-ca in one place

enhancements/machine-api/cluster-api-integration.md

elmiko

i think this is looking better @alexander-demichev , i left a couple comments. also, it might be helpful to start resolving some of the conversations to make the reviewing a little easier.

enhancements/machine-api/cluster-api-integration.md

elmiko · 2021-10-18T21:06:52Z

enhancements/machine-api/cluster-api-integration.md

+
+- During tech preview `cluster-capi-operator` will have permissions to manage CRDs, this might be a not secure permission for an operator.
+- Note, this permission should be restricted to creating CRDs only, as once installed, the technical preview cannot be uninstalled.
+- CLI usage, once Cluster API is installed command like `oc get machine` will return Cluster API machines, in order to use Machine API users will have to use fully qualified name `oc get machines.machine.openshift.io`.


this makes me wonder if we shouldn't have some sort of warning message associated with oc get machines once the feature gate is active, but i'm not sure if that's even possible.

Yeah, we still have to figure out what to do here.

Where did the conversation end up with API team about changing the priority so we don't make this breaking change?

We have a "hack" to set the preference in openshift. If we do this then any scripts should not break and users will have to use fully qualified names for CAPI resources.

We should make a note of that hack and ideally get something to track that so we don't forget to do it

I've just added a note

JoelSpeed

This is pretty much good to go I think, one nit in there plus I think one last outstanding question, then I think we can get this merged.

We need to nominate approvers, possible @elmiko and @asalkeld? (I can add the label on their behalf if they can't)

enhancements/machine-api/cluster-api-integration.md

JoelSpeed · 2021-10-29T09:42:53Z

enhancements/machine-api/cluster-api-integration.md

+
+- During tech preview `cluster-capi-operator` will have permissions to manage CRDs, this might be a not secure permission for an operator.
+- Note, this permission should be restricted to creating CRDs only, as once installed, the technical preview cannot be uninstalled.
+- CLI usage, once Cluster API is installed command like `oc get machine` will return Cluster API machines, in order to use Machine API users will have to use fully qualified name `oc get machines.machine.openshift.io`.


Where did the conversation end up with API team about changing the priority so we don't make this breaking change?

openshift-bot · 2022-01-05T04:50:54Z

Rotten enhancement proposals close after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Reopen the proposal by commenting /reopen.
Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Exclude this proposal from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2022-01-05T04:51:26Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten enhancement proposals close after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Reopen the proposal by commenting /reopen.
Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Exclude this proposal from closing again by commenting /lifecycle frozen.

/close

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.

alexander-demicev · 2022-01-05T14:14:01Z

/reopen
/remove-lifecycle rotten

openshift-ci · 2022-01-05T14:14:37Z

@alexander-demichev: Reopened this PR.

Details

In response to this:

/reopen
/remove-lifecycle rotten.

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.

elmiko

i think this generally reads well to me

the only question i have, which i don't think really fits within the enhancement scope is how the capi deployment might interact with the cluster-autoscaler. by default i don't imagine it would make any difference, but there is now the possibility for a user to install the autoscaler into a namespace that could read the capi machinesets and thus work in parallel with the openshift autoscaler.

just a though, i'm not sure we need to mention it here except to maybe say that we won't change the autoscaler behavior.

elmiko · 2022-01-06T22:38:09Z

enhancements/machine-api/cluster-api-integration.md

+
+In order to maintain the lifecycle of Cluster API related resources, we will create a new operator `cluster-capi-operator`, this name was chosen for avoiding confusion with upstream Cluster API operator.
+This operator will be responsible for all administrative tasks related to the deployment of the Cluster API project within the cluster.
+During tech preview phase, the new operator leverage new [CVO feature](https://github.com/openshift/enhancements/blob/master/enhancements/update/cvo-techpreview-manifests.md) for managing all Cluster API related CRDs.


minor nit

Suggested change

During tech preview phase, the new operator leverage new [CVO feature](https://github.com/openshift/enhancements/blob/master/enhancements/update/cvo-techpreview-manifests.md) for managing all Cluster API related CRDs.

During tech preview phase, the new operator will leverage the new [CVO feature](https://github.com/openshift/enhancements/blob/master/enhancements/update/cvo-techpreview-manifests.md) for managing all Cluster API related CRDs.

openshift-bot · 2022-01-13T22:59:01Z

Rotten enhancement proposals close after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Reopen the proposal by commenting /reopen.
Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Exclude this proposal from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2022-01-13T22:59:32Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten enhancement proposals close after 7d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Reopen the proposal by commenting /reopen.
Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Exclude this proposal from closing again by commenting /lifecycle frozen.

/close

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.

alexander-demicev · 2022-01-19T10:36:11Z

/reopen

openshift-ci · 2022-01-19T10:36:43Z

@alexander-demichev: Reopened this PR.

Details

In response to this:

/reopen

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.

alexander-demicev · 2022-01-19T11:09:12Z

@elmiko I added a note about autoscaler to nongoals of this proposal.

openshift-ci · 2022-01-19T11:13:19Z

@alexander-demichev: all tests passed!

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.

elmiko

generally looks good to me @alexander-demichev , i have a couple minor nits
/approve

elmiko · 2022-01-19T14:30:07Z

enhancements/machine-api/cluster-api-integration.md

+- Create both Cluster and InfrastructureCluster objects with externally managed cluster infrastructure annotation.
+- Ensure spec/status of InfrastructureCluster are configured for the OpenShift cluster (infrastructure information can be sourced from resources within the OpenShift Cluster).
+- Patch `Cluster` status to `Ready=true`.
+


might be nice to put this in the non-goals as well

elmiko · 2022-01-19T14:31:58Z

enhancements/machine-api/cluster-api-integration.md

+
+### Risks and Mitigations
+
+- During tech preview `cluster-capi-operator` will have permissions to manage CRDs, this might be a not secure permission for an operator.


minor nit

Suggested change

- During tech preview `cluster-capi-operator` will have permissions to manage CRDs, this might be a not secure permission for an operator.

- During tech preview `cluster-capi-operator` will have permissions to manage CRDs, this might not be a secure permission for an operator.

elmiko · 2022-01-19T14:36:11Z

/remove-lifecycle rotten

JoelSpeed · 2022-01-19T15:52:32Z

This enhancement has been stagnant for a while with no additional feedback and no new discoveries or changes to the plan that we are implementing. Let's merge this and iterate on it if we find new stuff that's needs updating
/lgtm
/approve

openshift-ci · 2022-01-19T15:54:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elmiko, JoelSpeed

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:

~~OWNERS~~ [JoelSpeed]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from abhinavdahiya and adambkaplan September 27, 2021 15:52

alexander-demicev force-pushed the capi branch from 130b51f to fe7a469 Compare September 27, 2021 16:00

elmiko reviewed Sep 27, 2021

View reviewed changes

asalkeld reviewed Sep 28, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Outdated Show resolved Hide resolved

asalkeld reviewed Sep 28, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Outdated Show resolved Hide resolved

asalkeld reviewed Sep 28, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Show resolved Hide resolved

hardys reviewed Sep 28, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Show resolved Hide resolved

flaper87 reviewed Sep 28, 2021

View reviewed changes

alexander-demicev force-pushed the capi branch 2 times, most recently from 6688bc9 to 8c35095 Compare October 13, 2021 13:48

alexander-demicev mentioned this pull request Oct 13, 2021

cluster-capi-operator: setup ci jobs openshift/release#22180

Merged

alexander-demicev force-pushed the capi branch from 8c35095 to 131c5b1 Compare October 13, 2021 14:29

enxebre reviewed Oct 13, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Outdated Show resolved Hide resolved

enxebre reviewed Oct 13, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Show resolved Hide resolved

asalkeld reviewed Oct 13, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Outdated Show resolved Hide resolved

asalkeld reviewed Oct 13, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Show resolved Hide resolved

asalkeld reviewed Oct 13, 2021

View reviewed changes

enhancements/machine-api/cluster-api-integration.md Show resolved Hide resolved

alexander-demicev force-pushed the capi branch from 131c5b1 to 681ee9e Compare October 14, 2021 14:40

elmiko reviewed Oct 18, 2021

View reviewed changes

alexander-demicev force-pushed the capi branch 4 times, most recently from 977577c to afdc763 Compare October 25, 2021 12:03

JoelSpeed reviewed Oct 29, 2021

View reviewed changes

alexander-demicev force-pushed the capi branch 3 times, most recently from 4f52628 to 107bc3d Compare October 29, 2021 12:25

Add cluster-api integration enhancement

bdb9e13

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 28, 2021

openshift-ci bot closed this Jan 5, 2022

openshift-ci bot reopened this Jan 5, 2022

elmiko reviewed Jan 6, 2022

View reviewed changes

openshift-ci bot closed this Jan 13, 2022

alexander-demicev deleted the capi branch January 17, 2022 15:03

alexander-demicev restored the capi branch January 17, 2022 15:03

openshift-ci bot reopened this Jan 19, 2022

Add enxebre as approver

227f355

alexander-demicev force-pushed the capi branch from f5f7b7b to 227f355 Compare January 19, 2022 11:08

elmiko reviewed Jan 19, 2022

View reviewed changes

openshift-ci bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jan 19, 2022

openshift-ci bot assigned JoelSpeed Jan 19, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2022

openshift-merge-robot merged commit d992b0b into openshift:master Jan 19, 2022

	During tech preview phase, the new operator leverage new [CVO feature](https://github.com/openshift/enhancements/blob/master/enhancements/update/cvo-techpreview-manifests.md) for managing all Cluster API related CRDs.
	During tech preview phase, the new operator will leverage the new [CVO feature](https://github.com/openshift/enhancements/blob/master/enhancements/update/cvo-techpreview-manifests.md) for managing all Cluster API related CRDs.


		### Risks and Mitigations

		- During tech preview `cluster-capi-operator` will have permissions to manage CRDs, this might be a not secure permission for an operator.

Add cluster-api integration enhancement #913

Add cluster-api integration enhancement #913

Uh oh!

Conversation

alexander-demicev commented Sep 27, 2021

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Jan 5, 2022

Uh oh!

openshift-ci bot commented Jan 5, 2022

Uh oh!

alexander-demicev commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jan 5, 2022

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Jan 13, 2022

Uh oh!

openshift-ci bot commented Jan 13, 2022

Uh oh!

alexander-demicev commented Jan 19, 2022

Uh oh!

alexander-demicev commented Jan 5, 2022 •

edited

Loading