Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify handling of CSI migration feature gate #3139

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+49 −13
Open

Unify handling of CSI migration feature gate #3139

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d38489
Unify handling of CSI migration feature gate
gnufied Dec 17, 2024
2a5de93
Fix the unit tests
gnufied Dec 17, 2024
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
5 changes: 5 additions & 0 deletions pkg/common/unittestcommon/utils.go
View file
Original file line number Diff line number Diff line change
@@ -118,6 +118,11 @@ func (c *FakeK8SOrchestrator) EnableFSS(ctx context.Context, featureName string)
c.featureStates[featureName] = "true"
return nil
}

func (c *FakeK8SOrchestrator) IsCSIMigrationEnabled(ctx context.Context, cfg *config.Config) bool {
return c.IsFSSEnabled(ctx, common.CSIMigration)
}

func (c *FakeK8SOrchestrator) DisableFSS(ctx context.Context, featureName string) error {
c.featureStates[featureName] = "false"
return nil
4 changes: 4 additions & 0 deletions pkg/csi/service/common/commonco/coagnostic.go
View file
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@ import (
restclient "k8s.io/client-go/rest"

cnsvolume "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco/k8sorchestrator"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco/types"
@@ -47,6 +48,9 @@ type COCommonInterface interface {
IsPVCSIFSSEnabled(ctx context.Context, featureName string) bool
// EnableFSS helps enable feature state switch in the FSS config map
EnableFSS(ctx context.Context, featureName string) error
// Check if CSI migration is enabled. In true multi-VC clusters CSI
// migration will be disabled by default
IsCSIMigrationEnabled(ctx context.Context, cfg *config.Config) bool
Copy link
Contributor

Choose a reason for hiding this comment

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

CSI migration is already GA. We are not supposed to disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can't work in multi-VC clusters though, regardless of feature gate state. That is why syncer logs those stacktraces in logs. So, the right thing to do here IMO is to disable the state when multiple vCenters are present.

// DisableFSS helps disable feature state switch in the FSS config map
// This method is added for Unit tests coverage
DisableFSS(ctx context.Context, featureName string) error
17 changes: 17 additions & 0 deletions pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go
View file
Original file line number Diff line number Diff line change
@@ -44,6 +44,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
cnsoperatorv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
cnsvolume "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
cnsconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
@@ -1359,6 +1360,22 @@ func (c *K8sOrchestrator) IsFakeAttachAllowed(ctx context.Context, volumeID stri
return false, nil
}

func (c *K8sOrchestrator) IsCSIMigrationEnabled(ctx context.Context, cfg *config.Config) bool {
log := logger.GetLogger(ctx)
isMultiVCenterFssEnabled := c.IsFSSEnabled(ctx, common.MultiVCenterCSITopology)
isMigrationEnabled := c.IsFSSEnabled(ctx, common.CSIMigration)

if !isMultiVCenterFssEnabled {
return isMigrationEnabled
} else {
if len(cfg.VirtualCenter) > 1 {
log.Infof("Disabling CSI migration in multi-vc clusters")
return false
}
return isMigrationEnabled
}
}

// MarkFakeAttached updates the pvc corresponding to volume to have a fake
// attach annotation.
func (c *K8sOrchestrator) MarkFakeAttached(ctx context.Context, volumeID string) error {
23 changes: 17 additions & 6 deletions pkg/csi/service/vanilla/controller.go
View file
Original file line number Diff line number Diff line change
@@ -42,6 +42,7 @@ import (
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/node"
cnsvolume "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
cnsvsphere "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
cnsconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
csifault "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/fault"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/prometheus"
@@ -131,7 +132,8 @@ func (c *controller) Init(config *cnsconfig.Config, version string) error {
// Check if the feature states are enabled.
multivCenterCSITopologyEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
common.MultiVCenterCSITopology)
csiMigrationEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration)
csiMigrationEnabled = commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, config)

filterSuspendedDatastores = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
common.CnsMgrSuspendCreateVolume)
isTopologyAwareFileVolumeEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
@@ -328,6 +330,7 @@ func (c *controller) Init(config *cnsconfig.Config, version string) error {
log.Errorf("failed to watch on path: %q. err=%v", cfgDirPath, err)
return err
}

if commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration) {
if !multivCenterCSITopologyEnabled {
log.Info("CSI Migration Feature is Enabled. Loading Volume Migration Service")
@@ -510,6 +513,14 @@ func (c *controller) filterDatastores(ctx context.Context, sharedDatastores []*c
return filteredDatastores, nil
}

func (c *controller) getCNSConfig() *config.Config {
if multivCenterCSITopologyEnabled {
return c.managers.CnsConfig
} else {
return c.manager.CnsConfig
}
}

// createBlockVolume creates a block volume based on the CreateVolumeRequest.
func (c *controller) createBlockVolume(ctx context.Context, req *csi.CreateVolumeRequest) (
*csi.CreateVolumeResponse, string, error) {
@@ -523,7 +534,7 @@ func (c *controller) createBlockVolume(ctx context.Context, req *csi.CreateVolum

// Check if the feature states are enabled.
isBlockVolumeSnapshotEnabled := commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.BlockVolumeSnapshot)
csiMigrationFeatureState := commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration)
csiMigrationFeatureState := commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, c.getCNSConfig())

// Check if requested volume size and source snapshot size matches
volumeSource := req.GetVolumeContentSource()
@@ -1587,7 +1598,7 @@ func (c *controller) createFileVolume(ctx context.Context, req *csi.CreateVolume

// Fetching the feature state for csi-migration before parsing storage class
// params.
csiMigrationFeatureState := commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration)
csiMigrationFeatureState := commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, c.getCNSConfig())
scParams, err := common.ParseStorageClassParams(ctx, req.Parameters, csiMigrationFeatureState)
// TODO: Need to figure out the fault returned by ParseStorageClassParams.
// Currently, just return "csi.fault.Internal".
@@ -2075,7 +2086,7 @@ func (c *controller) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequ
volumeType = prometheus.PrometheusBlockVolumeType
cnsVolumeType = common.BlockVolumeType
// In-tree volume support.
if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration) {
if !commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, c.getCNSConfig()) {
// Migration feature switch is disabled.
return nil, csifault.CSIInternalFault, logger.LogNewErrorCodef(log, codes.Internal,
"volume-migration feature switch is disabled. Cannot use volume with vmdk path :%q", req.VolumeId)
@@ -2257,7 +2268,7 @@ func (c *controller) ControllerPublishVolume(ctx context.Context, req *csi.Contr
volumeType = prometheus.PrometheusBlockVolumeType
if strings.Contains(req.VolumeId, ".vmdk") {
// In-tree volume support.
if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration) {
if !commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, c.getCNSConfig()) {
// Migration feature switch is disabled.
return nil, csifault.CSIInternalFault, logger.LogNewErrorCodef(log, codes.Internal,
"volume-migration feature switch is disabled. Cannot use volume with vmdk path :%q", req.VolumeId)
@@ -2392,7 +2403,7 @@ func (c *controller) ControllerUnpublishVolume(ctx context.Context, req *csi.Con
} else {
// In-tree volume support.
volumeType = prometheus.PrometheusBlockVolumeType
if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration) {
if !commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, c.getCNSConfig()) {
// Migration feature switch is disabled.
return nil, csifault.CSIInternalFault, logger.LogNewErrorCodef(log, codes.Internal,
"volume-migration feature switch is disabled. Cannot use volume with vmdk path: %q", req.VolumeId)
3 changes: 1 addition & 2 deletions pkg/syncer/fullsync.go
View file
Original file line number Diff line number Diff line change
@@ -68,8 +68,7 @@ func CsiFullSync(ctx context.Context, metadataSyncer *metadataSyncInformer, vc s
var err error
// Fetch CSI migration feature state, before performing full sync operations.
if metadataSyncer.clusterFlavor == cnstypes.CnsClusterFlavorVanilla {
if metadataSyncer.coCommonInterface.IsFSSEnabled(ctx, common.CSIMigration) &&
len(metadataSyncer.configInfo.Cfg.VirtualCenter) == 1 {
if metadataSyncer.coCommonInterface.IsCSIMigrationEnabled(ctx, metadataSyncer.configInfo.Cfg) {
migrationFeatureStateForFullSync = true
}
}
2 changes: 1 addition & 1 deletion pkg/syncer/metadatasyncer.go
View file
Original file line number Diff line number Diff line change
@@ -223,7 +223,7 @@ func InitMetadataSyncer(ctx context.Context, clusterFlavor cnstypes.CnsClusterFl

if clusterFlavor == cnstypes.CnsClusterFlavorVanilla {
isMultiVCenterFssEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.MultiVCenterCSITopology)
IsMigrationEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CSIMigration)
IsMigrationEnabled = commonco.ContainerOrchestratorUtility.IsCSIMigrationEnabled(ctx, configInfo.Cfg)
}
isStorageQuotaM2FSSEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.StorageQuotaM2)
// Create the kubernetes client from config.
8 changes: 4 additions & 4 deletions pkg/syncer/util.go
View file
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ func getPVsInBoundAvailableOrReleased(ctx context.Context,
}
for _, pv := range allPVs {
if (pv.Spec.CSI != nil && pv.Spec.CSI.Driver == csitypes.Name) ||
(metadataSyncer.coCommonInterface.IsFSSEnabled(ctx, common.CSIMigration) && pv.Spec.VsphereVolume != nil &&
(metadataSyncer.coCommonInterface.IsCSIMigrationEnabled(ctx, metadataSyncer.configInfo.Cfg) && pv.Spec.VsphereVolume != nil &&
isValidvSphereVolume(ctx, pv)) {
log.Debugf("FullSync: pv %v is in state %v", pv.Name, pv.Status.Phase)
if pv.Status.Phase == v1.VolumeBound || pv.Status.Phase == v1.VolumeAvailable ||
@@ -156,7 +156,7 @@ func IsValidVolume(ctx context.Context, volume v1.Volume, pod *v1.Pod,
// Verify volume is a in-tree VCP volume.
if pv.Spec.VsphereVolume != nil {
// Check if migration feature switch is enabled.
if metadataSyncer.coCommonInterface.IsFSSEnabled(ctx, common.CSIMigration) {
if metadataSyncer.coCommonInterface.IsCSIMigrationEnabled(ctx, metadataSyncer.configInfo.Cfg) {
if !isValidvSphereVolume(ctx, pv) {
return false, nil, nil
}
@@ -599,7 +599,7 @@ func getPVsInBoundAvailableOrReleasedForVc(ctx context.Context, metadataSyncer *
for _, pv := range allPvs {
// Check if the PV is an in-tree volume.
if pv.Spec.CSI == nil {
if metadataSyncer.coCommonInterface.IsFSSEnabled(ctx, common.CSIMigration) &&
if metadataSyncer.coCommonInterface.IsCSIMigrationEnabled(ctx, metadataSyncer.configInfo.Cfg) &&
pv.Spec.VsphereVolume != nil {
return nil, logger.LogNewErrorf(log,
"In-tree volumes are not supported on a multi VC set up."+
@@ -785,7 +785,7 @@ func createMissingFileVolumeInfoCrs(ctx context.Context, metadataSyncer *metadat
fileVolumes := make([]*v1.PersistentVolume, 0)
for _, pv := range allPvs {
if pv.Spec.CSI == nil {
if metadataSyncer.coCommonInterface.IsFSSEnabled(ctx, common.CSIMigration) &&
if metadataSyncer.coCommonInterface.IsCSIMigrationEnabled(ctx, metadataSyncer.configInfo.Cfg) &&
pv.Spec.VsphereVolume != nil {
log.Errorf("In-tree volumes are not supported on a multi VC set up."+
"Found in-tree volume %s.", pv.Name)