Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 48 additions & 9 deletions pkg/cloudprovider/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ func IsCloudProviderExternal(platformStatus *configv1.PlatformStatus, featureGat
return false, fmt.Errorf("platformStatus is required")
}
switch platformStatus.Type {
case configv1.VSpherePlatformType:
// vSphere has a special condition whereby CCM reversion must be possible in 4.13 OpenShift if the in-tree storage driver is enabled
disabled, err := isCSIMigrationvSphereDisabled(featureGate)
// we only want to use the in-tree cloud-provider when the migration is also disabled
return !disabled, err
case configv1.GCPPlatformType:
// Platforms that are external based on feature gate presence
return isExternalFeatureGateEnabled(featureGate)
Expand All @@ -32,8 +37,7 @@ func IsCloudProviderExternal(platformStatus *configv1.PlatformStatus, featureGat
configv1.KubevirtPlatformType,
configv1.NutanixPlatformType,
configv1.OpenStackPlatformType,
configv1.PowerVSPlatformType,
configv1.VSpherePlatformType:
configv1.PowerVSPlatformType:
return true, nil
default:
// Platforms that do not have external cloud providers implemented
Expand All @@ -48,18 +52,53 @@ func isExternalFeatureGateEnabled(featureGate *configv1.FeatureGate) (bool, erro
// If no featureGate is present, then the user hasn't opted in to the external cloud controllers
return false, nil
}

enabledFeatureGates, disabledFeatureGates, err := getEnabledDisabledFeatureGates(featureGate)
if err != nil {
return false, err
}

return !disabledFeatureGates.Has(ExternalCloudProviderFeature) && enabledFeatureGates.Has(ExternalCloudProviderFeature), nil
}

// isCSIMigrationvSphereDisabled determines whether the CSIMigrationvSphere feature gate is disabled in the current
// feature set. This function only returns true when the feature is actively disabled, otherwise we should
// deploy the CCM for vSphere.
// This function is needed to help address a reversion issue that is happening in 1.26 Kubernetes,
// which is also being brought to 4.13 OpenShift. This feature gate had been locked for 1.26, but has been
// unlocked in https://github.com/kubernetes/kubernetes/pull/116342. This feature gate had been removed from OpenShift
// as all vSphere clusters will be migrated to the CSI driver, due to the reversion we must support a situation where
// a user has upgraded to 4.13 but must revert their storage driver to in-tree. This reversion will also require the
// CCM to be migrated back to the in-tree KCM. Related storage JIRA, https://issues.redhat.com/browse/STOR-1265
// TODO remove this function once it is no longer needed, presumably 4.14 OpenShift.
func isCSIMigrationvSphereDisabled(featureGate *configv1.FeatureGate) (bool, error) {
if featureGate == nil {
// If no featureGate is present, then the user hasn't opted in to the in-tree vSphere driver
return false, nil
}

enabledFeatureGates, disabledFeatureGates, err := getEnabledDisabledFeatureGates(featureGate)
if err != nil {
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?

}

// get the enabled and disabled feature gates for the associated feature set
func getEnabledDisabledFeatureGates(featureGate *configv1.FeatureGate) (sets.String, sets.String, error) {
featureSet, ok := configv1.FeatureSets[featureGate.Spec.FeatureSet]
if !ok {
return false, fmt.Errorf(".spec.featureSet %q not found", featureGate.Spec.FeatureSet)
return nil, nil, fmt.Errorf(".spec.featureSet %q not found", featureGate.Spec.FeatureSet)
}

enabledFeatureGates := sets.NewString(featureSet.Enabled...)
disabledFeatureGates := sets.NewString(featureSet.Disabled...)
// CustomNoUpgrade will override the deafult enabled feature gates.
enabled := sets.NewString(featureSet.Enabled...)
disabled := sets.NewString(featureSet.Disabled...)
// CustomNoUpgrade will override the default enabled feature gates.
if featureGate.Spec.FeatureSet == configv1.CustomNoUpgrade && featureGate.Spec.CustomNoUpgrade != nil {
enabledFeatureGates = sets.NewString(featureGate.Spec.CustomNoUpgrade.Enabled...)
disabledFeatureGates = sets.NewString(featureGate.Spec.CustomNoUpgrade.Disabled...)
enabled = sets.NewString(featureGate.Spec.CustomNoUpgrade.Enabled...)
disabled = sets.NewString(featureGate.Spec.CustomNoUpgrade.Disabled...)
}

return !disabledFeatureGates.Has(ExternalCloudProviderFeature) && enabledFeatureGates.Has(ExternalCloudProviderFeature), nil
return enabled, disabled, nil
}