Skip to content

add ability to detect vsphere csi migration#1490

Closed
elmiko wants to merge 1 commit intoopenshift:masterfrom
elmiko:update-cloud-provider-check
Closed

add ability to detect vsphere csi migration#1490
elmiko wants to merge 1 commit intoopenshift:masterfrom
elmiko:update-cloud-provider-check

Conversation

@elmiko
Copy link

@elmiko elmiko commented Mar 22, 2023

Due to changes in upstream kubernetes, the vSphere in-tree cloud provider will need to be used in situations where the in-tree storage is still in use. Because OpenShift 4.13 is enabling CSI migration by default for vSphere, there will need to be a regression method for the CCM as well. This change transforms the IsCloudProviderExternal function to detect this condition.

references:
https://groups.google.com/g/kubernetes-sig-storage/c/wvJpNDBS9vU
kubernetes/kubernetes#116342
https://issues.redhat.com/browse/STOR-1265

Due to changes in upstream kubernetes, the vSphere in-tree cloud
provider will need to be used in situations where the in-tree storage is
still in use. Because OpenShift 4.13 is enabling CSI migration by
default for vSphere, there will need to be a regression method for the
CCM as well. This change transforms the IsCloudProviderExternal function
to detect this condition.

references:
https://groups.google.com/g/kubernetes-sig-storage/c/wvJpNDBS9vU
kubernetes/kubernetes#116342
https://issues.redhat.com/browse/STOR-1265
@openshift-ci 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 22, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 22, 2023

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
Copy link
Author

elmiko commented Mar 22, 2023

this will require openshift/api#1423 before it can work

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 22, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: elmiko
Once this PR has been reviewed and has the lgtm label, please assign soltysh for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

return false, err
}

return !disabledFeatureGates.Has(configv1.InTreeVSphereVolumes) && enabledFeatureGates.Has(configv1.InTreeVSphereVolumes), nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InTreeVSphereVolumes is the FeatureSet that disables the CSIMigrationvSphere FeatureGate here, so this line should check for CSIMigrationvSphere instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack, thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, just to be clear here, the CSIMigrationvSphere needs to be in the disabled list?

@elmiko
Copy link
Author

elmiko commented Mar 29, 2023

it seems like we will go another direction that is more specific in the mco. i am closing this.

@elmiko elmiko closed this Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants