Add cluster-capi-operator integration #266

mdbooth · 2023-10-05T16:45:30Z

No description provided.

openshift-ci · 2023-10-05T16:45:41Z

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

EmilienM · 2023-11-08T15:46:36Z

openshift/kustomize/infracluster-controller/controller.yaml

+          - /infracluster-controller
+          args:
+            - "--leader-elect"
+            - "--metrics-bind-address=127.0.0.1:8080"


FYI this will very likely change once kubernetes-sigs#1731 has merged and will be backported in 4.15.

EmilienM · 2023-11-08T15:48:43Z

openshift/kustomize/infracluster-controller/controller.yaml

+          imagePullPolicy: Always
+          name: manager
+          ports:
+            - containerPort: 8081


if it's the CAPO controller, it's 9440

It's a separate manager.

EmilienM · 2023-11-08T15:49:45Z

openshift/kustomize/infracluster-controller/controller.yaml

+          ports:
+            - containerPort: 8081
+              name: healthz
+              protocol: TCP


We'll also need :

- containerPort: 8443 name: metrics protocol: TCP

After kubernetes-sigs#1731 has merged and been downstreamed.

EmilienM · 2023-11-08T15:51:46Z

openshift/kustomize/infracluster-controller/kustomization.yaml

+  newName: quay.io/mbooth/cluster-api-provider-openstack
+  newTag: v0.8.0-openshift.3
+
+replacements:


I have no idea if it has an impact but I'll mention it here in case, we might need to apply the new RBAC rules for the authorized diagnostics endpoint: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/pull/1731/files#diff-2873f79a86c0d8b3335cd7731b0ecf7dd4301eb19a82ef7a1cba7589b5252261R148-R150
Once we get new CAPI, see https://github.com/kubernetes-sigs/cluster-api-provider-openstack/pull/1731/files#diff-2873f79a86c0d8b3335cd7731b0ecf7dd4301eb19a82ef7a1cba7589b5252261R148-R150

EmilienM · 2023-11-08T15:54:12Z

I've dropped a few comments with are low value for now, just FYIs I guess. I'm sure all this stuff is generated anyway?

mdbooth · 2023-11-23T20:22:52Z

/hold

mdbooth · 2023-11-24T09:59:22Z

/test images

openshift-ci · 2023-11-24T10:17:01Z

@mdbooth: all tests passed!

Full PR test history. Your PR dashboard.

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.

damdo · 2023-11-27T08:56:11Z

openshift/go.mod

+	k8s.io/api v0.28.2
+	k8s.io/apimachinery v0.28.2
+	k8s.io/client-go v0.28.1


FYI:
For all the other CAPI providers we are forking we are one version lower than the expected k8s version for that OCP next release.
So for example since in 4.15 we are on 1.28, the k8s and controller runtime deps for the CAPI forks are all 0.28-0.01 = k8s 0.27 deps.

This is because not all of them have readily available releases with k8s deps matching the expected k8s version for the OCP release.

The deps are also kept in sync with the cluster-capi-operator for API imports compatibility.

See: https://github.com/openshift/cluster-capi-operator/blob/8acf83f7bf5c1a633715a9169f1c46e23568c04b/go.mod#L15-L28

This also aligns with the k8s and controller-runtime deps that go in tandem with the 1.5.2 CAPI release we (cluster-capi-operator) and CAPO (atm) are pinning to: https://github.com/kubernetes-sigs/cluster-api/blob/3290c5a28ed28c1909713e59e5d481a3e8f68a90/go.mod#L36-L48

So I'd advise to switch this to 0.27 for now.
We are planning to revisit this versioning/pinning mechanism in the future.

This is the infra cluster controller, though. It'll be bumped soon in CAPI and the upgrade is a bit of a PITA, so I decided to use the new version for the new controller.

For CAPO itself, are you suggesting we intentionally stay on an old version with old deps?

For the other CAP* providers we are rebasing/merging on top of explicit upstream releases.
Unfortunately the providers are not always quick at releasing with the most recent k8s dependencies. For example even cluster-api has not yet put out a stable releases which uses 0.28 dependencies (1.5.3 is on k8s 0.27 and c-r 0.15.x).

As such given we are referencing explicit releases for providers, which as of now are using CAPI 1.5.3, which uses 0.27, our approach with them is to keep our deps to 0.27, 1.5.x and 0.15.x.

Hence I'd suggest to keep CAPO to 0.27 et. al. until we figure out a better strategy for this.

damdo · 2023-11-27T09:00:47Z

openshift/pkg/infracluster_controller/openstackcluster.go

+}
+
+// ensureCAPICluster creates a new CAPI cluster if none exists. It will not modify an existing cluster.
+func (r *OpenShiftClusterReconciler) ensureCAPICluster(ctx context.Context, log logr.Logger, infrastructure *openshiftconfig.Infrastructure, openStackCluster *infrav1.OpenStackCluster, co *openshiftconfig.ClusterOperator) error {


Setting this up is ok for now.
Although we might want to move this into the cluster-capi-operator in the future if we decide to detect the InfraCluster and set the CAPI Cluster up from there, centrally.

mandre

Given that:

this feature is being the TechPreviewNoUpgrade feature flag
the code doesn't do anything by itself
we have time for bug fixes if need be
manual tests showed it is ready for TechPreview

/lgtm

mandre · 2023-11-27T19:15:49Z

/approve

openshift-ci · 2023-11-27T19:16:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mandre

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:

~~DOWNSTREAM_OWNERS~~ [mandre]

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

mdbooth · 2023-11-28T09:27:52Z

/hold cancel

openshift-bot · 2023-11-28T11:41:31Z

[ART PR BUILD NOTIFIER]

This PR has been included in build openstack-cluster-api-controllers-container-v4.15.0-202311281110.p0.ga1d41d1.assembly.stream for distgit openstack-cluster-api-controllers.
All builds following this will include this PR.

SecurityGroupParam

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 Oct 5, 2023

mdbooth force-pushed the cluster-capi-operator-wip branch 4 times, most recently from f1ead72 to b99cbac Compare October 13, 2023 14:01

mdbooth mentioned this pull request Oct 13, 2023

UPSTREAM 1726: Replace kustomize vars with replacements #272

Merged

mdbooth force-pushed the cluster-capi-operator-wip branch 3 times, most recently from d95ebe8 to 51c2e63 Compare October 17, 2023 09:59

EmilienM force-pushed the cluster-capi-operator-wip branch from 0e83084 to d7cfdf6 Compare November 8, 2023 15:38

EmilienM reviewed Nov 8, 2023

View reviewed changes

mdbooth force-pushed the cluster-capi-operator-wip branch 4 times, most recently from ede16ea to bd77a87 Compare November 23, 2023 11:54

Add cluster-capi-operator integration

b86d6ff

mdbooth force-pushed the cluster-capi-operator-wip branch from bd77a87 to b86d6ff Compare November 23, 2023 17:40

mdbooth changed the title ~~WIP: cluster-capi-operator integration~~ Add cluster-capi-operator integration Nov 23, 2023

mdbooth marked this pull request as ready for review November 23, 2023 20:22

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 Nov 23, 2023

openshift-ci bot requested review from pierreprinetti and stephenfin November 23, 2023 20:22

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 23, 2023

damdo reviewed Nov 27, 2023

View reviewed changes

mandre reviewed Nov 27, 2023

View reviewed changes

openshift-ci bot assigned mandre Nov 27, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 27, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2023

openshift-merge-bot bot merged commit a1d41d1 into openshift:main Nov 28, 2023
3 checks passed

mandre deleted the cluster-capi-operator-wip branch November 28, 2023 09:40

pierreprinetti pushed a commit to shiftstack/cluster-api-provider-openstack that referenced this pull request Apr 22, 2024

fix template after change securityGroups from string to (openshift#266)

acaee1f

SecurityGroupParam

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cluster-capi-operator integration #266

Add cluster-capi-operator integration #266

mdbooth commented Oct 5, 2023

openshift-ci bot commented Oct 5, 2023

EmilienM Nov 8, 2023

EmilienM Nov 8, 2023

mdbooth Nov 20, 2023

EmilienM Nov 8, 2023

EmilienM Nov 8, 2023

EmilienM commented Nov 8, 2023

mdbooth commented Nov 23, 2023

mdbooth commented Nov 24, 2023

openshift-ci bot commented Nov 24, 2023

damdo Nov 27, 2023

damdo Nov 27, 2023

mdbooth Nov 27, 2023

mdbooth Nov 27, 2023

damdo Nov 27, 2023

damdo Nov 27, 2023

mandre left a comment

mandre commented Nov 27, 2023

openshift-ci bot commented Nov 27, 2023

mdbooth commented Nov 28, 2023

openshift-bot commented Nov 28, 2023

Add cluster-capi-operator integration #266

Add cluster-capi-operator integration #266

Conversation

mdbooth commented Oct 5, 2023

openshift-ci bot commented Oct 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmilienM commented Nov 8, 2023

mdbooth commented Nov 23, 2023

mdbooth commented Nov 24, 2023

openshift-ci bot commented Nov 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandre left a comment

Choose a reason for hiding this comment

mandre commented Nov 27, 2023

openshift-ci bot commented Nov 27, 2023

mdbooth commented Nov 28, 2023

openshift-bot commented Nov 28, 2023