Skip to content

Remove csi-migration fss #3405

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+95 −257
Open

Remove csi-migration fss #3405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion pkg/common/unittestcommon/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func GetFakeContainerOrchestratorInterface(orchestratorType int) (commonco.COCom
fakeCO := &FakeK8SOrchestrator{
featureStatesLock: &sync.RWMutex{},
featureStates: map[string]string{
"csi-migration": "true",
"file-volume": "true",
"block-volume-snapshot": "true",
"tkgs-ha": "true",
Expand Down
5 changes: 1 addition & 4 deletions pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ func Newk8sOrchestrator(ctx context.Context, controllerClusterFlavor cnstypes.Cn

func getReleasedVanillaFSS() map[string]struct{} {
return map[string]struct{}{
common.CSIMigration: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove FSS from pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator_test.go, those are missed in this diff

common.OnlineVolumeExtend: {},
common.BlockVolumeSnapshot: {},
common.CSIWindowsSupport: {},
Expand Down Expand Up @@ -999,7 +998,6 @@ func pvAdded(obj interface{}) {
// Since cns query will return all the volumes including the migrated ones, the map would need to be a
// union of migrated VCP-CSI volumes and CSI volumes, as well.
if pv.Spec.VsphereVolume != nil &&
k8sOrchestratorInstance.IsFSSEnabled(context.Background(), common.CSIMigration) &&
isValidMigratedvSphereVolume(context.Background(), pv.ObjectMeta) {
if pv.Status.Phase == v1.VolumeBound {
k8sOrchestratorInstance.volumeIDToNameMap.add(pv.Spec.VsphereVolume.VolumePath, pv.Name)
Expand Down Expand Up @@ -1049,7 +1047,6 @@ func pvUpdated(oldObj, newObj interface{}) {
// Since cns query will return all the volumes including the migrated ones, the map would need to be a
// union of migrated VCP-CSI volumes and CSI volumes, as well.
if newPv.Spec.VsphereVolume != nil &&
k8sOrchestratorInstance.IsFSSEnabled(context.Background(), common.CSIMigration) &&
isValidMigratedvSphereVolume(context.Background(), newPv.ObjectMeta) {
if oldPv.Status.Phase != v1.VolumeBound && newPv.Status.Phase == v1.VolumeBound {
k8sOrchestratorInstance.volumeIDToNameMap.add(newPv.Spec.VsphereVolume.VolumePath, newPv.Name)
Expand Down Expand Up @@ -1078,7 +1075,7 @@ func pvDeleted(obj interface{}) {
log.Debugf("k8sorchestrator: Deleted key %s from pvcToVolumeID",
pv.Spec.ClaimRef.Namespace+"/"+pv.Spec.ClaimRef.Name)
}
if pv.Spec.VsphereVolume != nil && k8sOrchestratorInstance.IsFSSEnabled(context.Background(), common.CSIMigration) {
if pv.Spec.VsphereVolume != nil {
k8sOrchestratorInstance.volumeIDToNameMap.remove(pv.Spec.VsphereVolume.VolumePath)
log.Debugf("k8sorchestrator migrated volume: Deleted key %s from volumeIDToNameMap",
pv.Spec.VsphereVolume.VolumePath)
Expand Down
2 changes: 0 additions & 2 deletions pkg/csi/service/common/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,6 @@ const (
DefaultFeatureEnablementCheckInterval = 1 * time.Minute
// OnlineVolumeExtend guards the feature for online volume expansion.
OnlineVolumeExtend = "online-volume-extend"
// CSIMigration is feature flag for migrating in-tree vSphere volumes to CSI.
CSIMigration = "csi-migration"
// CSISVFeatureStateReplication is feature flag for SV feature state
// replication feature.
CSISVFeatureStateReplication = "csi-sv-feature-states-replication"
Expand Down
80 changes: 32 additions & 48 deletions pkg/csi/service/common/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,64 +240,48 @@ func IsValidVolumeCapabilities(ctx context.Context, volCaps []*csi.VolumeCapabil

// ParseStorageClassParams parses the params in the CSI CreateVolumeRequest API
// call back to StorageClassParams structure.
func ParseStorageClassParams(ctx context.Context, params map[string]string,
csiMigrationFeatureState bool) (*StorageClassParams, error) {
func ParseStorageClassParams(ctx context.Context, params map[string]string) (*StorageClassParams, error) {
log := logger.GetLogger(ctx)
scParams := &StorageClassParams{
DatastoreURL: "",
StoragePolicyName: "",
}
if !csiMigrationFeatureState {
for param, value := range params {
param = strings.ToLower(param)
if param == AttributeDatastoreURL {
scParams.DatastoreURL = value
} else if param == AttributeStoragePolicyName {
scParams.StoragePolicyName = value
} else if param == AttributeFsType {
log.Warnf("param 'fstype' is deprecated, please use 'csi.storage.k8s.io/fstype' instead")
} else {
return nil, fmt.Errorf("invalid param: %q and value: %q", param, value)
}
otherParams := make(map[string]string)
for param, value := range params {
param = strings.ToLower(param)
if param == AttributeDatastoreURL {
scParams.DatastoreURL = value
} else if param == AttributeStoragePolicyName {
scParams.StoragePolicyName = value
} else if param == AttributeFsType {
log.Warnf("param 'fstype' is deprecated, please use 'csi.storage.k8s.io/fstype' instead")
} else if param == CSIMigrationParams {
scParams.CSIMigration = value
} else {
otherParams[param] = value
}
} else {
otherParams := make(map[string]string)
for param, value := range params {
}
// check otherParams belongs to in-tree migrated Parameters.
if scParams.CSIMigration == "true" {
for param, value := range otherParams {
param = strings.ToLower(param)
if param == AttributeDatastoreURL {
scParams.DatastoreURL = value
} else if param == AttributeStoragePolicyName {
scParams.StoragePolicyName = value
} else if param == AttributeFsType {
log.Warnf("param 'fstype' is deprecated, please use 'csi.storage.k8s.io/fstype' instead")
} else if param == CSIMigrationParams {
scParams.CSIMigration = value
if param == DatastoreMigrationParam {
scParams.Datastore = value
} else if param == DiskFormatMigrationParam && value == "thin" {
continue
} else if param == HostFailuresToTolerateMigrationParam ||
param == ForceProvisioningMigrationParam || param == CacheReservationMigrationParam ||
param == DiskstripesMigrationParam || param == ObjectspacereservationMigrationParam ||
param == IopslimitMigrationParam {
return nil, fmt.Errorf("vSphere CSI driver does not support creating volume using "+
"in-tree vSphere volume plugin parameter key:%v, value:%v", param, value)
} else {
otherParams[param] = value
return nil, fmt.Errorf("invalid parameter. key:%v, value:%v", param, value)
}
}
// check otherParams belongs to in-tree migrated Parameters.
if scParams.CSIMigration == "true" {
for param, value := range otherParams {
param = strings.ToLower(param)
if param == DatastoreMigrationParam {
scParams.Datastore = value
} else if param == DiskFormatMigrationParam && value == "thin" {
continue
} else if param == HostFailuresToTolerateMigrationParam ||
param == ForceProvisioningMigrationParam || param == CacheReservationMigrationParam ||
param == DiskstripesMigrationParam || param == ObjectspacereservationMigrationParam ||
param == IopslimitMigrationParam {
return nil, fmt.Errorf("vSphere CSI driver does not support creating volume using "+
"in-tree vSphere volume plugin parameter key:%v, value:%v", param, value)
} else {
return nil, fmt.Errorf("invalid parameter. key:%v, value:%v", param, value)
}
}
} else {
if len(otherParams) != 0 {
return nil, fmt.Errorf("invalid parameters :%v", otherParams)
}
} else {
if len(otherParams) != 0 {
return nil, fmt.Errorf("invalid parameters :%v", otherParams)
}
}
return scParams, nil
Expand Down
61 changes: 4 additions & 57 deletions pkg/csi/service/common/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,42 +377,7 @@ func isStorageClassParamsEqual(expected *StorageClassParams, actual *StorageClas
return true
}

func TestParseStorageClassParamsWithDeprecatedFSType(t *testing.T) {
params := map[string]string{
"fstype": "ext4",
}
expectedScParams := &StorageClassParams{}
csiMigrationFeatureState := false
actualScParams, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
if err != nil {
t.Errorf("failed to parse params: %+v", params)
}
if !isStorageClassParamsEqual(expectedScParams, actualScParams) {
t.Errorf("Expected: %+v\n Actual: %+v", expectedScParams, actualScParams)
}
}

func TestParseStorageClassParamsWithValidParams(t *testing.T) {
params := map[string]string{
AttributeDatastoreURL: "ds1",
AttributeStoragePolicyName: "policy1",
}
expectedScParams := &StorageClassParams{
DatastoreURL: "ds1",
StoragePolicyName: "policy1",
}
csiMigrationFeatureState := false
actualScParams, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
if err != nil {
t.Errorf("failed to parse params: %+v", params)
}
if !isStorageClassParamsEqual(expectedScParams, actualScParams) {
t.Errorf("Expected: %+v\n Actual: %+v", expectedScParams, actualScParams)
}
}

func TestParseStorageClassParamsWithMigrationEnabledNagative(t *testing.T) {
csiMigrationFeatureState := true
params := map[string]string{
CSIMigrationParams: "true",
DatastoreMigrationParam: "vSANDatastore",
Expand All @@ -424,36 +389,34 @@ func TestParseStorageClassParamsWithMigrationEnabledNagative(t *testing.T) {
ObjectspacereservationMigrationParam: "50",
IopslimitMigrationParam: "16",
}
scParam, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
scParam, err := ParseStorageClassParams(ctx, params)
if err == nil {
t.Errorf("error expected but not received. scParam received from ParseStorageClassParams: %v", scParam)
}
t.Logf("expected err received. err: %v", err)
}

func TestParseStorageClassParamsWithDiskFormatMigrationEnableNegative(t *testing.T) {
csiMigrationFeatureState := true
params := map[string]string{
CSIMigrationParams: "true",
DiskFormatMigrationParam: "thick",
}
scParam, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
scParam, err := ParseStorageClassParams(ctx, params)
if err == nil {
t.Errorf("error expected but not received. scParam received from ParseStorageClassParams: %v", scParam)
}
t.Logf("expected err received. err: %v", err)
}

func TestParseStorageClassParamsWithDiskFormatMigrationEnablePositive(t *testing.T) {
csiMigrationFeatureState := true
params := map[string]string{
CSIMigrationParams: "true",
DiskFormatMigrationParam: "thin",
}
expectedScParams := &StorageClassParams{
CSIMigration: "true",
}
scParam, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
scParam, err := ParseStorageClassParams(ctx, params)
if err != nil {
t.Errorf("failed to parse params: %+v, err: %+v", params, err)
}
Expand All @@ -463,7 +426,6 @@ func TestParseStorageClassParamsWithDiskFormatMigrationEnablePositive(t *testing
}

func TestParseStorageClassParamsWithMigrationEnabledPositive(t *testing.T) {
csiMigrationFeatureState := true
params := map[string]string{
CSIMigrationParams: "true",
DatastoreMigrationParam: "vSANDatastore",
Expand All @@ -474,7 +436,7 @@ func TestParseStorageClassParamsWithMigrationEnabledPositive(t *testing.T) {
StoragePolicyName: "policy1",
CSIMigration: "true",
}
scParam, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
scParam, err := ParseStorageClassParams(ctx, params)
if err != nil {
t.Errorf("failed to parse params: %+v", params)
}
Expand All @@ -483,21 +445,6 @@ func TestParseStorageClassParamsWithMigrationEnabledPositive(t *testing.T) {
}
}

func TestParseStorageClassParamsWithMigrationDisabled(t *testing.T) {
csiMigrationFeatureState := false
params := map[string]string{
CSIMigrationParams: "true",
DatastoreMigrationParam: "vSANDatastore",
AttributeStoragePolicyName: "policy1",
HostFailuresToTolerateMigrationParam: "1",
}
scParam, err := ParseStorageClassParams(ctx, params, csiMigrationFeatureState)
if err == nil {
t.Errorf("error expected but not received. scParam received from ParseStorageClassParams: %v", scParam)
}
t.Logf("expected err received. err: %v", err)
}

func TestParseCSISnapshotID(t *testing.T) {
type args struct {
ctx context.Context
Expand Down
Loading