CORS-2405: Create ControlPlaneMachineSet CRDs #6632

rna-afk · 2022-11-28T18:06:37Z

Creating an extra CRD asset for the ControlPlaneMachineSet which is required for the machine api operator for more control over the control-plane nodes that come up on Azure.

rna-afk · 2022-11-29T15:21:52Z

@JoelSpeed can you help with the Azure CPMS CRD? This is the output of the manifest file that I tested locally. Can you check to see if this is the right piece of information the operator needs?

apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
metadata:
  creationTimestamp: null
  labels:
    machine.openshift.io/cluster-api-cluster: anarayan-fr485
  name: cluster
  namespace: openshift-machine-api
spec:
  replicas: 3
  selector:
    matchLabels:
      machine.openshift.io/cluster-api-cluster: anarayan-fr485
      machine.openshift.io/cluster-api-machine-role: master
      machine.openshift.io/cluster-api-machine-type: master
  strategy: {}
  template:
    machineType: machines_v1beta1_machine_openshift_io
    machines_v1beta1_machine_openshift_io:
      failureDomains:
        azure:
        - zone: "3"
        - zone: "1"
        - zone: "2"
        platform: Azure
      metadata:
        labels:
          machine.openshift.io/cluster-api-cluster: anarayan-fr485
          machine.openshift.io/cluster-api-machine-role: master
          machine.openshift.io/cluster-api-machine-type: master
      spec:
        lifecycleHooks: {}
        metadata: {}
        providerSpec:
          value:
            acceleratedNetworking: true
            apiVersion: machine.openshift.io/v1beta1
            credentialsSecret:
              name: azure-cloud-credentials
              namespace: openshift-machine-api
            diagnostics: {}
            image:
              offer: ""
              publisher: ""
              resourceID: /resourceGroups/anarayan-fr485-rg/providers/Microsoft.Compute/galleries/gallery_anarayan_fr485/images/anarayan-fr485-gen2/versions/412.86.20220930
              sku: ""
              version: ""
            kind: AzureMachineProviderSpec
            location: eastus
            managedIdentity: anarayan-fr485-identity
            metadata:
              creationTimestamp: null
            networkResourceGroup: anarayan-fr485-rg
            osDisk:
              diskSettings: {}
              diskSizeGB: 1024
              managedDisk:
                storageAccountType: Premium_LRS
              osType: Linux
            publicIP: false
            publicLoadBalancer: anarayan-fr485
            resourceGroup: anarayan-fr485-rg
            subnet: anarayan-fr485-master-subnet
            userDataSecret:
              name: master-user-data
            vmSize: Standard_D8s_v3
            vnet: anarayan-fr485-vnet
            zone: "2"
status: {}

JoelSpeed · 2022-11-29T16:12:51Z

failureDomains:
        azure:
        - zone: "3"
        - zone: "1"
        - zone: "2"
        platform: Azure

How much effort would it be to sort the zones? 🤔 Purely for aesthetic reasons

           userDataSecret:
             name: master-user-data
           vmSize: Standard_D8s_v3
           vnet: anarayan-fr485-vnet
           zone: "2"

You should clear the zone from this spec

Otherwise LGTM

rna-afk · 2022-11-29T16:29:43Z

How much effort would it be to sort the zones? thinking Purely for aesthetic reasons

Not much. Fixed it.

You should clear the zone from this spec

Done.

Here's the new CRD.

apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
metadata:
  creationTimestamp: null
  labels:
    machine.openshift.io/cluster-api-cluster: anarayan-fhzz6
  name: cluster
  namespace: openshift-machine-api
spec:
  replicas: 3
  selector:
    matchLabels:
      machine.openshift.io/cluster-api-cluster: anarayan-fhzz6
      machine.openshift.io/cluster-api-machine-role: master
      machine.openshift.io/cluster-api-machine-type: master
  strategy: {}
  template:
    machineType: machines_v1beta1_machine_openshift_io
    machines_v1beta1_machine_openshift_io:
      failureDomains:
        azure:
        - zone: "1"
        - zone: "2"
        - zone: "3"
        platform: Azure
      metadata:
        labels:
          machine.openshift.io/cluster-api-cluster: anarayan-fhzz6
          machine.openshift.io/cluster-api-machine-role: master
          machine.openshift.io/cluster-api-machine-type: master
      spec:
        lifecycleHooks: {}
        metadata: {}
        providerSpec:
          value:
            acceleratedNetworking: true
            apiVersion: machine.openshift.io/v1beta1
            credentialsSecret:
              name: azure-cloud-credentials
              namespace: openshift-machine-api
            diagnostics: {}
            image:
              offer: ""
              publisher: ""
              resourceID: /resourceGroups/anarayan-fhzz6-rg/providers/Microsoft.Compute/galleries/gallery_anarayan_fhzz6/images/anarayan-fhzz6-gen2/versions/412.86.20220930
              sku: ""
              version: ""
            kind: AzureMachineProviderSpec
            location: eastus
            managedIdentity: anarayan-fhzz6-identity
            metadata:
              creationTimestamp: null
            networkResourceGroup: anarayan-fhzz6-rg
            osDisk:
              diskSettings: {}
              diskSizeGB: 1024
              managedDisk:
                storageAccountType: Premium_LRS
              osType: Linux
            publicIP: false
            publicLoadBalancer: anarayan-fhzz6
            resourceGroup: anarayan-fhzz6-rg
            subnet: anarayan-fhzz6-master-subnet
            userDataSecret:
              name: master-user-data
            vmSize: Standard_D8s_v3
            vnet: anarayan-fhzz6-vnet
status: {}

JoelSpeed · 2022-11-29T16:31:15Z

New CRD LGTM

rna-afk · 2022-12-06T17:02:18Z

/retest

JoelSpeed · 2022-12-07T15:07:08Z

The Azure E2E on this has passed and looking at the test link, the CPMS looks happy! Thanks

patrickdillon · 2022-12-08T20:01:43Z

/approve
/lgtm

openshift-ci · 2022-12-08T20:02:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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~~ [patrickdillon]

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 · 2022-12-09T02:38:10Z

/retest-required

Remaining retests: 0 against base HEAD 40cf6fe and 2 for PR HEAD 6e096e2 in total

rna-afk · 2022-12-09T16:25:34Z

/retest

openshift-ci-robot · 2022-12-09T20:38:40Z

/retest-required

Remaining retests: 0 against base HEAD 6de3bb4 and 1 for PR HEAD 6e096e2 in total

patrickdillon · 2022-12-09T20:39:35Z

/skip

patrickdillon · 2022-12-09T20:40:14Z

/override ci/prow/e2e-gcp-ovn

we're having a ci "infra" issue with gcp right now and this pr is not on gcp, so skip it

openshift-ci · 2022-12-09T20:42:29Z

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-gcp-ovn

Details

In response to this:

/override ci/prow/e2e-gcp-ovn

we're having a ci "infra" issue with gcp right now and this pr is not on gcp, so skip it

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.

openshift-ci-robot · 2022-12-10T02:38:35Z

/retest-required

Remaining retests: 0 against base HEAD 3c693eb and 0 for PR HEAD 6e096e2 in total

openshift-ci-robot · 2022-12-10T11:38:13Z

/hold

Revision 6e096e2 was retested 3 times: holding

rna-afk · 2023-01-09T14:29:55Z

/hold cancel

openshift-ci-robot · 2023-01-09T14:46:05Z

/retest-required

Remaining retests: 0 against base HEAD a3e3714 and 2 for PR HEAD 84441fa in total

rna-afk · 2023-01-09T14:47:28Z

/skip

JoelSpeed · 2023-01-10T10:33:16Z

pkg/asset/machines/azure/machines.go

Are there any differences between the machineSet and the control plane machine specs? I know for example the boot diagnostics are configured via terraform and therefore should be for control plane machines too, but I'm not sure that ever happened

Edit: Looking at the cluster from the attached card, the internalLoadBalancer: jimacpms-s2rbw-internal field is missing, we need to make sure the spec we are using matches the control-plane machines else this won't work

pkg/asset/machines/azure/machines.go

JoelSpeed · 2023-01-11T12:50:23Z

/test e2e-azure-ovn

JoelSpeed · 2023-01-12T13:26:35Z

e2e-azure-ovn has passed so I think this is fixed now

JoelSpeed · 2023-01-12T13:29:02Z

/lgtm

openshift-ci-robot · 2023-01-12T14:03:17Z

/retest-required

Remaining retests: 0 against base HEAD 8c83507 and 2 for PR HEAD 08c8aec0e03a3afa8dd06f45217e93228eef67e1 in total

rna-afk · 2023-01-12T14:20:19Z

/hold

I need to change the error returned.

Creating an extra CRD asset for the ControlPlaneMachineSet which is required for the machine api operator for more control over the control-plane nodes that come up on Azure.

rna-afk · 2023-01-12T14:24:58Z

/hold cancel

Changed the error a bit to let the error flow to the earlier functions.

JoelSpeed · 2023-01-13T10:36:14Z

/lgtm

openshift-ci-robot · 2023-01-13T11:30:56Z

/retest-required

Remaining retests: 0 against base HEAD 960ebcf and 2 for PR HEAD 95c765a in total

openshift-ci · 2023-01-13T13:54:01Z

@rna-afk: The following tests 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-agent-compact	980b3c8290d12b85fb67badcda61a18dee8b74bc	link	true	`/test e2e-agent-compact`
ci/prow/e2e-agent-sno	980b3c8290d12b85fb67badcda61a18dee8b74bc	link	false	`/test e2e-agent-sno`
ci/prow/e2e-openstack	019e770cfc2de99cb8d5fc1c6f94f00f1c4b637d	link	false	`/test e2e-openstack`
ci/prow/e2e-libvirt	019e770cfc2de99cb8d5fc1c6f94f00f1c4b637d	link	false	`/test e2e-libvirt`
ci/prow/e2e-ibmcloud-ovn	019e770cfc2de99cb8d5fc1c6f94f00f1c4b637d	link	false	`/test e2e-ibmcloud-ovn`
ci/prow/e2e-openstack-ovn	019e770cfc2de99cb8d5fc1c6f94f00f1c4b637d	link	true	`/test e2e-openstack-ovn`
ci/prow/e2e-vsphere-ovn	019e770cfc2de99cb8d5fc1c6f94f00f1c4b637d	link	true	`/test e2e-vsphere-ovn`
ci/prow/e2e-aws-ovn-upgrade	`95c765a`	link	false	`/test e2e-aws-ovn-upgrade`
ci/prow/e2e-aws-ovn-disruptive	`95c765a`	link	false	`/test e2e-aws-ovn-disruptive`
ci/prow/e2e-aws-ovn-workers-rhel8	`95c765a`	link	false	`/test e2e-aws-ovn-workers-rhel8`
ci/prow/okd-e2e-aws-ovn-upgrade	`95c765a`	link	false	`/test okd-e2e-aws-ovn-upgrade`
ci/prow/okd-scos-e2e-aws-upgrade	`95c765a`	link	false	`/test okd-scos-e2e-aws-upgrade`
ci/prow/e2e-azurestack	`95c765a`	link	false	`/test e2e-azurestack`
ci/prow/okd-scos-e2e-aws-ovn	`95c765a`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/okd-e2e-aws-ovn	`95c765a`	link	false	`/test okd-e2e-aws-ovn`

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.

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 Nov 28, 2022

openshift-ci bot requested review from jhixson74 and patrickdillon November 28, 2022 18:09

rna-afk force-pushed the azure_cpms branch 2 times, most recently from 54d497b to 980b3c8 Compare November 29, 2022 16:28

rna-afk changed the title ~~[WIP] CORS-2405: Create ControlPlaneMachineSet CRDs~~ CORS-2405: Create ControlPlaneMachineSet CRDs Nov 29, 2022

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 29, 2022

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 29, 2022

rna-afk force-pushed the azure_cpms branch from 980b3c8 to 6e096e2 Compare December 1, 2022 14:23

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2022

openshift-ci bot assigned patrickdillon Dec 8, 2022

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

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

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

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

rna-afk force-pushed the azure_cpms branch from 84441fa to 019e770 Compare January 9, 2023 18:20

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2023

JoelSpeed reviewed Jan 10, 2023

View reviewed changes

rna-afk force-pushed the azure_cpms branch from 019e770 to 508d970 Compare January 10, 2023 14:37

JoelSpeed reviewed Jan 10, 2023

View reviewed changes

pkg/asset/machines/azure/machines.go Outdated Show resolved Hide resolved

rna-afk force-pushed the azure_cpms branch 2 times, most recently from 2cd344b to 876ce20 Compare January 10, 2023 17:44

rna-afk force-pushed the azure_cpms branch from 876ce20 to 08c8aec Compare January 11, 2023 16:34

openshift-ci bot assigned JoelSpeed Jan 12, 2023

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

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

azure: Create ControlPlaneMachineSet CRDs

95c765a

Creating an extra CRD asset for the ControlPlaneMachineSet which is required for the machine api operator for more control over the control-plane nodes that come up on Azure.

rna-afk force-pushed the azure_cpms branch from 08c8aec to 95c765a Compare January 12, 2023 14:24

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 12, 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 Jan 12, 2023

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

openshift-merge-robot merged commit c5bb817 into openshift:master Jan 14, 2023

CORS-2405: Create ControlPlaneMachineSet CRDs #6632

CORS-2405: Create ControlPlaneMachineSet CRDs #6632

Uh oh!

Conversation

rna-afk commented Nov 28, 2022

Uh oh!

rna-afk commented Nov 29, 2022

Uh oh!

JoelSpeed commented Nov 29, 2022

Uh oh!

rna-afk commented Nov 29, 2022

Uh oh!

JoelSpeed commented Nov 29, 2022

Uh oh!

rna-afk commented Dec 6, 2022

Uh oh!

JoelSpeed commented Dec 7, 2022

Uh oh!

patrickdillon commented Dec 8, 2022

Uh oh!

openshift-ci bot commented Dec 8, 2022

Uh oh!

openshift-ci-robot commented Dec 9, 2022

Uh oh!

rna-afk commented Dec 9, 2022

Uh oh!

openshift-ci-robot commented Dec 9, 2022

Uh oh!

patrickdillon commented Dec 9, 2022

Uh oh!

patrickdillon commented Dec 9, 2022

Uh oh!

openshift-ci bot commented Dec 9, 2022

Uh oh!

openshift-ci-robot commented Dec 10, 2022

Uh oh!

openshift-ci-robot commented Dec 10, 2022

Uh oh!

rna-afk commented Jan 9, 2023

Uh oh!

openshift-ci-robot commented Jan 9, 2023

Uh oh!

rna-afk commented Jan 9, 2023

Uh oh!

JoelSpeed Jan 10, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoelSpeed commented Jan 11, 2023

Uh oh!

JoelSpeed commented Jan 12, 2023

Uh oh!

JoelSpeed commented Jan 12, 2023

Uh oh!

openshift-ci-robot commented Jan 12, 2023

Uh oh!

rna-afk commented Jan 12, 2023

Uh oh!

rna-afk commented Jan 12, 2023

Uh oh!

JoelSpeed commented Jan 13, 2023

Uh oh!

openshift-ci-robot commented Jan 13, 2023

Uh oh!

openshift-ci bot commented Jan 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants