OSASINFRA-3133 - OpenStack support by EmilienM · Pull Request #195 · openshift/cluster-control-plane-machine-set-operator

EmilienM · 2023-04-14T17:10:59Z

OpenStack support in CPMS.

openshift-ci · 2023-04-14T17:11:03Z

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

pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go

odvarkadaniel

Generally, the code LGTM. I would appreciate that someone else would look at this after KubeCon.

cc/ @JoelSpeed @damdo

pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go

test/e2e/framework/framework.go

pkg/controllers/controlplanemachinesetgenerator/controller_test.go

pkg/controllers/controlplanemachinesetgenerator/openstack.go

pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go

EmilienM · 2023-06-30T01:54:17Z

@pierreprinetti much better with the new installer patchset:

Spec:
  Replicas:  3
  Selector:
    Match Labels:
      machine.openshift.io/cluster-api-cluster:       refarch-s26qd
      machine.openshift.io/cluster-api-machine-role:  master
      machine.openshift.io/cluster-api-machine-type:  master
  State:                                              Active
  Strategy:
    Type:  RollingUpdate
  Template:
    Machine Type:  machines_v1beta1_machine_openshift_io
    machines_v1beta1_machine_openshift_io:
      Failure Domains:
        Openstack:
          Availability Zone:  az0
          Availability Zone:  az1
          Availability Zone:  az2
        Platform:             OpenStack
      Metadata:
        Labels:
          machine.openshift.io/cluster-api-cluster:       refarch-s26qd
          machine.openshift.io/cluster-api-machine-role:  master
          machine.openshift.io/cluster-api-machine-type:  master
      Spec:
        Lifecycle Hooks:
        Metadata:
        Provider Spec:
          Value:
            API Version:  machine.openshift.io/v1alpha1
            Cloud Name:   openstack
            Clouds Secret:
              Name:       openstack-cloud-credentials
              Namespace:  openshift-machine-api
            Flavor:       m1.xlarge
            Image:        refarch-s26qd-rhcos
            Kind:         OpenstackProviderSpec
            Metadata:
              Creation Timestamp:  <nil>
            Networks:
              Filter:
              Subnets:
                Filter:
                  Name:  refarch-s26qd-nodes
                  Tags:  openshiftClusterID=refarch-s26qd
            Security Groups:
              Filter:
              Name:             refarch-s26qd-master
            Server Group Name:  refarch-s26qd-master
            Server Metadata:
              Name:                  refarch-s26qd-master
              Openshift Cluster ID:  refarch-s26qd
            Tags:
              openshiftClusterID=refarch-s26qd
            Trunk:  true
            User Data Secret:
              Name:  master-user-data

@damdo @JoelSpeed ready for review when you have time.
Let me know if you want me to squash the commits.

Nothing changed much since your last reviews, except the last 2 commits I would say, which are small enough to be quickly reviewed.

Co-Authored-By: Emilien Macchi <emilien@redhat.com> Co-Authored-By: Pierre Prinetti <pierreprinetti@redhat.com>

test/e2e/helpers/machine.go

If `OPENSTACK_CONTROLPLANE_FLAVOR_ALTERNATE` is found in the environment variables, we'll use that flavor when running the control plane verticale scale tests. Otherwise, roll back on the tags.

JoelSpeed · 2023-06-30T12:49:55Z

docs/user/failure-domains.md

+```yaml
+- availabilityZone: "<nova availability zone>"
+  rootVolume:
+    availabilityZone: "<cinder availability zone>"


Didn't we add another field to the AZ?

if you're talking about the volume type, it'll be handled separately via #217

JoelSpeed · 2023-06-30T12:50:14Z

docs/user/installation.md

+
+#### Configuring a control plane machine set on OpenStack
+
+Two fields are supported for now: `availabilityZone` (instance AZ) and `rootVolume.availabilityZone` (root volume AZ).


Does this need updating for the third field that we added?

volume type will be handled separately via #217

pkg/controllers/controlplanemachinesetgenerator/utils.go

JoelSpeed · 2023-06-30T13:53:30Z

Apart from the nit about the errors, I think this is good to go, thanks for working with us on this 😄

JoelSpeed · 2023-06-30T13:56:06Z

/approve

pierreprinetti

This code seems to be behaving conform to the spec. I believe that there is no reason to further defer testing in vivo.

openshift-ci · 2023-07-03T14:53:42Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

pierreprinetti · 2023-07-03T14:58:15Z

/lgtm

pierreprinetti · 2023-07-03T15:04:41Z

/retest-required

damdo

LGTM
Thanks for all you efforts!

openshift-ci-robot · 2023-07-03T17:14:39Z

/retest-required

Remaining retests: 0 against base HEAD 88ec3eb and 2 for PR HEAD 464c8dc in total

openshift-ci · 2023-07-03T18:35:12Z

@EmilienM: The following test 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/e2e-azure-ovn-etcd-scaling	`464c8dc`	link	false	`/test e2e-azure-ovn-etcd-scaling`

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.

pierreprinetti · 2023-07-03T18:40:58Z

/retest-required

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 14, 2023

EmilienM force-pushed the openstack branch 9 times, most recently from 2add629 to 3c9d625 Compare April 18, 2023 14:55

EmilienM commented Apr 18, 2023

View reviewed changes

pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go Outdated Show resolved Hide resolved

EmilienM force-pushed the openstack branch 7 times, most recently from 41d0792 to ca2234e Compare April 19, 2023 18:49

EmilienM marked this pull request as ready for review April 19, 2023 19:11

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 Apr 19, 2023

openshift-ci bot requested review from elmiko and odvarkadaniel April 19, 2023 19:12

odvarkadaniel reviewed Apr 20, 2023

View reviewed changes

pierreprinetti reviewed Apr 20, 2023

View reviewed changes

pkg/controllers/controlplanemachinesetgenerator/openstack.go Outdated Show resolved Hide resolved

pierreprinetti reviewed Apr 20, 2023

View reviewed changes

pkg/controllers/controlplanemachinesetgenerator/openstack.go Outdated Show resolved Hide resolved

pierreprinetti reviewed Apr 20, 2023

View reviewed changes

pkg/controllers/controlplanemachinesetgenerator/openstack.go Outdated Show resolved Hide resolved

pierreprinetti reviewed Apr 20, 2023

View reviewed changes

pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go Outdated Show resolved Hide resolved

EmilienM force-pushed the openstack branch from ca2234e to 1abd3dc Compare April 20, 2023 15:29

EmilienM force-pushed the openstack branch 2 times, most recently from db07333 to de7743f Compare June 29, 2023 19:48

pierreprinetti force-pushed the openstack branch from de7743f to 318b324 Compare June 30, 2023 12:21

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2023

EmilienM force-pushed the openstack branch from 318b324 to f4c6915 Compare June 30, 2023 12:47

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2023

EmilienM and others added 2 commits June 30, 2023 14:50

OpenStack support in CPMS

c5397e2

Co-Authored-By: Emilien Macchi <emilien@redhat.com> Co-Authored-By: Pierre Prinetti <pierreprinetti@redhat.com>

Add support for no failure domains in OpenStack platform

afbf7c8

pierreprinetti force-pushed the openstack branch from f4c6915 to d43ed9a Compare June 30, 2023 12:51

pierreprinetti reviewed Jun 30, 2023

View reviewed changes

test/e2e/helpers/machine.go Outdated Show resolved Hide resolved

e2e: allow to run regular vertical scale tests in OpenStack

464c8dc

If `OPENSTACK_CONTROLPLANE_FLAVOR_ALTERNATE` is found in the environment variables, we'll use that flavor when running the control plane verticale scale tests. Otherwise, roll back on the tags.

EmilienM force-pushed the openstack branch from d43ed9a to 464c8dc Compare June 30, 2023 12:54

JoelSpeed reviewed Jun 30, 2023

View reviewed changes

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

EmilienM mentioned this pull request Jun 30, 2023

OSASINFRA-3155 - OpenStack: Create ControlPlaneMachineSet CRDs openshift/installer#7280

Merged

pierreprinetti approved these changes Jul 3, 2023

View reviewed changes

openshift-ci bot assigned pierreprinetti Jul 3, 2023

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

damdo reviewed Jul 3, 2023

View reviewed changes

openshift-merge-robot merged commit 74ceace into openshift:main Jul 3, 2023

pierreprinetti deleted the openstack branch July 3, 2023 20:58


		#### Configuring a control plane machine set on OpenStack

		Two fields are supported for now: `availabilityZone` (instance AZ) and `rootVolume.availabilityZone` (root volume AZ).

Comments

Conversation

EmilienM commented Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Apr 14, 2023

Uh oh!

Uh oh!

odvarkadaniel 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!

EmilienM commented Jun 30, 2023

Uh oh!

Uh oh!

JoelSpeed Jun 30, 2023

Choose a reason for hiding this comment

Uh oh!

EmilienM Jun 30, 2023

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Jun 30, 2023

Choose a reason for hiding this comment

Uh oh!

EmilienM Jun 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoelSpeed commented Jun 30, 2023

Uh oh!

JoelSpeed commented Jun 30, 2023

Uh oh!

pierreprinetti left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jul 3, 2023

Uh oh!

pierreprinetti commented Jul 3, 2023

Uh oh!

pierreprinetti commented Jul 3, 2023

Uh oh!

damdo left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Jul 3, 2023

Uh oh!

openshift-ci bot commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierreprinetti commented Jul 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

EmilienM commented Apr 14, 2023 •

edited

Loading

openshift-ci bot commented Jul 3, 2023 •

edited

Loading