OCPBUGS-11146: update cloud provider flags on vSphere for storage options by elmiko · Pull Request #3655 · openshift/machine-config-operator

elmiko · 2023-03-31T20:18:29Z

- What I did

modify the render pipeline to include the storage operator object
update the cloud provider flag to check the storage options when using vSphere
only use external cloud provider flag on vSphere when CSI storage is enabled

- How to verify it

there are 3 main conditions to verify with this change:

starting with a 4.12 vSphere cluster using in-tree storage, upgrade to 4.13 should preserve the --cloud-provider=vsphere flag on the kubelet while using --cloud-provider=external on the kube-apiserver and kube-controller-manager.
starting with a 4.12 vSphere cluster using out-of-tree storage (CSI), upgrade to 4.13 should upgrade all cloud provider flags to --cloud-provider=external.
installing new clusters on 4.13 should use all cloud provider flags as --cloud-provider=external.

- Description for the changelog

Update external cloud provider check for vSphere to observe the CSI migration status and properly configure the cluster for in-tree storage and out-of-tree cloud providers.

openshift-ci-robot · 2023-03-31T20:18:32Z

@elmiko: This pull request references OCPCLOUD-2027 which is a valid jira issue.

Details

In response to this:

- What I did

modify the render pipeline to include the storage operator object

update the cloud provider flag to check the storage options when using vSphere

only use external cloud provider flag on vSphere when CSI storage is enabled

- How to verify it

there are 3 main conditions to verify with this change:

starting with a 4.12 vSphere cluster using in-tree storage, upgrade to 4.13 should preserve the --cloud-provider=vsphere flag on the kubelet while using --cloud-provider=external on the kube-apiserver and kube-controller-manager.

starting with a 4.12 vSphere cluster using out-of-tree storage (CSI), upgrade to 4.13 should upgrade all cloud provider flags to --cloud-provider=external.

installing new clusters on 4.13 should use all cloud provider flags as --cloud-provider=external.

- Description for the changelog

Update external cloud provider check for vSphere to observe the CSI migration status and properly configure the cluster for in-tree storage and out-of-tree cloud providers.

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 · 2023-03-31T20:18:36Z

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

elmiko · 2023-03-31T20:19:05Z

i am still working on this but wanted to start sharing for others to see what we are proposing.

pkg/controller/template/render.go

pkg/controller/template/template_controller.go

elmiko · 2023-04-04T17:34:38Z

i think this PR is ready for review, i am going to begin manually testing it.

bringing in the updates storage operator field.

openshift-ci-robot · 2023-04-04T21:11:40Z

@elmiko: This pull request references OCPCLOUD-2027 which is a valid jira issue.

Details

In response to this:

- What I did

modify the render pipeline to include the storage operator object

update the cloud provider flag to check the storage options when using vSphere

only use external cloud provider flag on vSphere when CSI storage is enabled

- How to verify it

there are 3 main conditions to verify with this change:

starting with a 4.12 vSphere cluster using in-tree storage, upgrade to 4.13 should preserve the --cloud-provider=vsphere flag on the kubelet while using --cloud-provider=external on the kube-apiserver and kube-controller-manager.

starting with a 4.12 vSphere cluster using out-of-tree storage (CSI), upgrade to 4.13 should upgrade all cloud provider flags to --cloud-provider=external.

installing new clusters on 4.13 should use all cloud provider flags as --cloud-provider=external.

- Description for the changelog

Update external cloud provider check for vSphere to observe the CSI migration status and properly configure the cluster for in-tree storage and out-of-tree cloud providers.

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 · 2023-04-04T21:11:55Z

@elmiko: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

OCPCLOUD-2027: update cloud provider flags on vSphere for storage options

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 · 2023-04-04T21:12:58Z

switched to the release-4.13 branch and rebased this PR to match.

also added a commit to handle the OPENSHIFT_DO_VSPHERE_MIGRATION environment variable.

i am still wrestling with the unit tests.

pkg/controller/template/template_controller.go

pkg/controller/template/render.go

elmiko · 2023-04-04T22:50:30Z

updated with latest comments

add functions for storage handlers
check for storage nil object in render func

elmiko · 2023-04-05T16:57:05Z

updated to fix up the unit tests and add tests for the new logic.

elmiko · 2023-04-05T18:56:57Z

i am manually testing this on a 4.13 cluster with the other storage related changes and it looks like it is doing what we want on a default install. i can see that the kubelet has --cloud-provider=vsphere and the kcm has --cloud-provider=external --external-cloud-volume-plugin=vsphere.

gnufied · 2023-04-05T21:08:38Z

pkg/controller/template/template_controller_test.go

We should get a test here that storage object is populated and passed correctly.

sounds like a good upgrade, i'll take a look

elmiko · 2023-04-05T21:50:57Z

i was wrong about the expected default condition, and it looks like we might have hit a condition where the Storage object is nil going into the logic. i have added the Storage lister to the function that caches the informer, i am going to test manually.

elmiko · 2023-04-06T13:56:25Z

updated to add rbac for the storage object to the machine-config-controller

knobunc · 2023-04-18T16:05:24Z

/label backport-risk-assessed
/label cherry-pick-approved

openshift-ci · 2023-04-18T16:06:10Z

@knobunc: Can not set label backport-risk-assessed: Must be member in one of these teams: [openshift-patch-managers]

Details

In response to this:

/label backport-risk-assessed
/label cherry-pick-approved

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 · 2023-04-18T16:06:14Z

@knobunc: Can not set label cherry-pick-approved: Must be member in one of these teams: []

Details

In response to this:

/label backport-risk-assessed
/label cherry-pick-approved

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 · 2023-04-18T17:31:33Z

@elmiko: 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-openstack	5c7d26ffe349c45aadf7eae2825f25a4bab187f3	link	false	`/test e2e-openstack`
ci/prow/okd-scos-e2e-gcp-ovn-upgrade	`7f43eaf`	link	false	`/test okd-scos-e2e-gcp-ovn-upgrade`
ci/prow/okd-scos-e2e-vsphere-ovn	`7f43eaf`	link	false	`/test okd-scos-e2e-vsphere-ovn`
ci/prow/e2e-gcp-rt-op	`7f43eaf`	link	false	`/test e2e-gcp-rt-op`
ci/prow/okd-scos-e2e-aws-ovn	`7f43eaf`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/e2e-alibabacloud-ovn	`7f43eaf`	link	false	`/test e2e-alibabacloud-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.

yuqi-zhang

It sounds like this was tested extensively, so I think I am fine giving the approval. The general workflow lgtm although I haven't personally tested on vsphere. So long as we don't break any use cases (which CI should cover) I think this is fine

openshift-ci · 2023-04-18T17:55:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dobsonj, elmiko, yuqi-zhang

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~~ [yuqi-zhang]

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

dobsonj · 2023-04-18T19:33:34Z

/unhold

openshift-ci-robot · 2023-04-18T19:38:37Z

@elmiko: Jira Issue OCPBUGS-11146: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

openshift/vmware-vsphere-csi-driver-operator#145 is open

These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-11146 has not been moved to the MODIFIED state.

Details

In response to this:

- What I did

modify the render pipeline to include the storage operator object

update the cloud provider flag to check the storage options when using vSphere

only use external cloud provider flag on vSphere when CSI storage is enabled

- How to verify it

there are 3 main conditions to verify with this change:

starting with a 4.12 vSphere cluster using in-tree storage, upgrade to 4.13 should preserve the --cloud-provider=vsphere flag on the kubelet while using --cloud-provider=external on the kube-apiserver and kube-controller-manager.

starting with a 4.12 vSphere cluster using out-of-tree storage (CSI), upgrade to 4.13 should upgrade all cloud provider flags to --cloud-provider=external.

installing new clusters on 4.13 should use all cloud provider flags as --cloud-provider=external.

- Description for the changelog

Update external cloud provider check for vSphere to observe the CSI migration status and properly configure the cluster for in-tree storage and out-of-tree cloud providers.

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-merge-robot · 2023-04-28T05:20:38Z

Fix included in accepted release 4.13.0-0.nightly-2023-04-27-232707

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 31, 2023

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 Mar 31, 2023

elmiko commented Mar 31, 2023

View reviewed changes

pkg/controller/template/render.go Outdated Show resolved Hide resolved

elmiko commented Mar 31, 2023

View reviewed changes

pkg/controller/template/template_controller.go Outdated Show resolved Hide resolved

elmiko force-pushed the vsphere-csi branch from 1dc2e01 to f08e554 Compare April 4, 2023 17:33

elmiko marked this pull request as ready for review April 4, 2023 17:34

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 4, 2023

openshift-ci bot requested review from jkyros and yuqi-zhang April 4, 2023 17:38

update vendor for openshift/api

bd7ecc7

bringing in the updates storage operator field.

elmiko force-pushed the vsphere-csi branch from f08e554 to 5c7d26f Compare April 4, 2023 21:11

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

elmiko changed the base branch from master to release-4.13 April 4, 2023 21:11

dobsonj reviewed Apr 4, 2023

View reviewed changes

pkg/controller/template/template_controller.go Outdated Show resolved Hide resolved

pkg/controller/template/render.go Outdated Show resolved Hide resolved

elmiko force-pushed the vsphere-csi branch from 5c7d26f to 5f8d9bc Compare April 4, 2023 22:49

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

elmiko force-pushed the vsphere-csi branch from 5f8d9bc to cc9baa4 Compare April 4, 2023 22:50

gnufied reviewed Apr 5, 2023

View reviewed changes

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

knobunc added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. labels Apr 18, 2023

openshift-ci bot assigned anuragthehatter, eurijon, mburman5, mhanss, rbbratta, reihl, rioliu-rh and rlobillo Apr 18, 2023

yuqi-zhang approved these changes Apr 18, 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 Apr 18, 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 Apr 18, 2023

openshift-merge-robot merged commit 65337f0 into openshift:release-4.13 Apr 18, 2023

elmiko deleted the vsphere-csi branch April 19, 2023 08:27

elmiko restored the vsphere-csi branch April 19, 2023 08:27

This was referenced Apr 19, 2023

OCPBUGS-11146: Mark cluster un-upgradeable when CSI migration is disabled openshift/vmware-vsphere-csi-driver-operator#145

Merged

OCPBUGS-11146: Check migration status openshift/vsphere-problem-detector#108

Merged

aravindhp mentioned this pull request Apr 24, 2023

[release-4.13] WINC-1014: Enable vSphere in-tree storage openshift/windows-machine-config-operator#1547

Merged

dobsonj mentioned this pull request May 23, 2023

OCPBUGS-13914: allow removal of vsphereStorageDriver field in 4.14 openshift/api#1464

Merged

elmiko deleted the vsphere-csi branch June 26, 2023 19:31

Conversation

elmiko commented Mar 31, 2023

Uh oh!

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

Uh oh!

Uh oh!

openshift-ci bot commented Mar 31, 2023

Uh oh!

elmiko commented Mar 31, 2023

Uh oh!

Uh oh!

Uh oh!

elmiko commented Apr 4, 2023

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

elmiko commented Apr 4, 2023

Uh oh!

Uh oh!

Uh oh!

elmiko commented Apr 4, 2023

Uh oh!

elmiko commented Apr 5, 2023

Uh oh!

elmiko commented Apr 5, 2023

Uh oh!

gnufied Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

elmiko Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

elmiko commented Apr 5, 2023

Uh oh!

elmiko commented Apr 6, 2023

Uh oh!

knobunc commented Apr 18, 2023

Uh oh!

openshift-ci bot commented Apr 18, 2023

Uh oh!

openshift-ci bot commented Apr 18, 2023

Uh oh!

openshift-ci bot commented Apr 18, 2023

Uh oh!

yuqi-zhang left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Apr 18, 2023

Uh oh!

dobsonj commented Apr 18, 2023

Uh oh!

openshift-ci-robot commented Apr 18, 2023

Uh oh!

openshift-merge-robot commented Apr 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

openshift-ci-robot commented Mar 31, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 4, 2023 •

edited by openshift-ci bot

Loading

openshift-ci bot commented Apr 4, 2023 •

edited

Loading