diff --git a/changelogs/unreleased/10661-krishhna24 b/changelogs/unreleased/10661-krishhna24 new file mode 100644 index 000000000..c398c039a --- /dev/null +++ b/changelogs/unreleased/10661-krishhna24 @@ -0,0 +1 @@ +Match --features as the comma-separated list it is when checking for EnableCSI diff --git a/hack/test.sh b/hack/test.sh index 61756b12e..0ab625ba9 100755 --- a/hack/test.sh +++ b/hack/test.sh @@ -24,6 +24,10 @@ export CGO_ENABLED=0 TARGETS=($(go list ./pkg/... ./internal/...| grep -vE "/pkg/builder|pkg/apis|pkg/test|pkg/generated|pkg/plugin/generated|mocks|internal/restartabletest")) TARGETS+=( ./cmd/... + # The test/ root package only holds the e2e suite's shared types and their + # unit tests. The e2e suites themselves live in subpackages and are not + # listed, since running them needs a cluster. + ./test ) if [[ ${#@} -ne 0 ]]; then diff --git a/test/e2e/basic/backup-volume-info/base.go b/test/e2e/basic/backup-volume-info/base.go index 9a21c38a4..f3c059ac8 100644 --- a/test/e2e/basic/backup-volume-info/base.go +++ b/test/e2e/basic/backup-volume-info/base.go @@ -78,7 +78,7 @@ func (v *BackupVolumeInfo) Init() error { } func (v *BackupVolumeInfo) Start() error { - if strings.Contains(v.VeleroCfg.Features, FeatureCSI) { + if FeatureEnabled(v.VeleroCfg.Features, FeatureCSI) { if strings.Contains(v.CaseBaseName, "native-snapshot") { fmt.Printf("Skip native snapshot case %s when the CSI feature is enabled.\n", v.CaseBaseName) Skip("Skip native snapshot case due to CSI feature is enabled.") diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 8ff609a64..7806dce7f 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -823,7 +823,7 @@ var _ = BeforeSuite(func() { By("Install StorageClass for E2E.") Expect(veleroutil.InstallStorageClasses(test.VeleroCfg.CloudProvider)).To(Succeed()) - if strings.EqualFold(test.VeleroCfg.Features, test.FeatureCSI) && + if test.FeatureEnabled(test.VeleroCfg.Features, test.FeatureCSI) && test.VeleroCfg.UseVolumeSnapshots { By("Install VolumeSnapshotClass for E2E.") Expect( @@ -881,7 +881,7 @@ var _ = AfterSuite(func() { ), ).To(Succeed()) - if strings.EqualFold(test.VeleroCfg.Features, test.FeatureCSI) && + if test.FeatureEnabled(test.VeleroCfg.Features, test.FeatureCSI) && test.VeleroCfg.UseVolumeSnapshots { By("Delete VolumeSnapshotClass created by E2E") Expect( diff --git a/test/e2e/migration/migration.go b/test/e2e/migration/migration.go index c5ba68cf3..6495c4d6e 100644 --- a/test/e2e/migration/migration.go +++ b/test/e2e/migration/migration.go @@ -285,7 +285,7 @@ func (m *migrationE2E) Backup() error { // the snapshots of AWS may be still in pending status when do the restore. // wait for a while to avoid this https://github.com/velero-io/velero/issues/1799 if OriginVeleroCfg.CloudProvider == test.Azure && - strings.EqualFold(OriginVeleroCfg.Features, test.FeatureCSI) || + test.FeatureEnabled(OriginVeleroCfg.Features, test.FeatureCSI) || OriginVeleroCfg.CloudProvider == test.AWS { By("Sleep 5 minutes to avoid snapshot recreated by unknown reason ", func() { time.Sleep(5 * time.Minute) @@ -355,7 +355,7 @@ func (m *migrationE2E) Restore() error { test.VeleroCfg.StandbyClient.Kubebuilder, )).To(Succeed()) - if strings.EqualFold(m.VeleroCfg.Features, test.FeatureCSI) && + if test.FeatureEnabled(m.VeleroCfg.Features, test.FeatureCSI) && m.VeleroCfg.UseVolumeSnapshots { By("Install VolumeSnapshotClass for E2E.") Expect( @@ -488,7 +488,7 @@ func (m *migrationE2E) Clean() error { m.VeleroCfg.ClientToInstallVelero.Kubebuilder, )).To(Succeed()) - if strings.EqualFold(m.VeleroCfg.Features, test.FeatureCSI) && + if test.FeatureEnabled(m.VeleroCfg.Features, test.FeatureCSI) && m.VeleroCfg.UseVolumeSnapshots { By("Delete VolumeSnapshotClass created by E2E") if err := k8sutil.KubectlDeleteByFile( diff --git a/test/e2e/upgrade/upgrade.go b/test/e2e/upgrade/upgrade.go index 03262a8f9..fd39782fd 100644 --- a/test/e2e/upgrade/upgrade.go +++ b/test/e2e/upgrade/upgrade.go @@ -19,7 +19,6 @@ import ( "context" "flag" "fmt" - "strings" "time" "github.com/google/uuid" @@ -224,7 +223,7 @@ func BackupUpgradeRestoreTest(useVolumeSnapshots bool, veleroCLI2Version VeleroC fmt.Sprintf("failed to delete namespace %s", upgradeNamespace)) }) - if useVolumeSnapshots && veleroCfg.CloudProvider == Azure && strings.EqualFold(veleroCfg.Features, FeatureCSI) { + if useVolumeSnapshots && veleroCfg.CloudProvider == Azure && FeatureEnabled(veleroCfg.Features, FeatureCSI) { // Upgrade test is not running daily since no CSI plugin v1.0 released, because builds before // v1.0 have issues to fail upgrade case. By("Sleep 5 minutes to avoid snapshot recreated by unknown reason ", func() { diff --git a/test/types.go b/test/types.go index 2788eb86d..794f9ca49 100644 --- a/test/types.go +++ b/test/types.go @@ -17,6 +17,7 @@ limitations under the License. package test import ( + "strings" "time" "github.com/google/uuid" @@ -52,6 +53,20 @@ func SecondStorageClassInstalled() bool { return !(StorageClassFile2Set && StorageClassFile2 == "") } +// FeatureEnabled reports whether name is one of the features in a --features +// value. The flag takes a comma-separated list, so comparing the whole value +// against a single feature name stops matching as soon as a second feature is +// enabled, and comparing with strings.Contains matches any feature whose name +// merely starts with this one. +func FeatureEnabled(features, name string) bool { + for _, f := range strings.Split(features, ",") { + if strings.EqualFold(strings.TrimSpace(f), name) { + return true + } + } + return false +} + const FeatureCSI = "EnableCSI" const VanillaZFS = "vanilla-zfs" const Kind = "kind" diff --git a/test/types_test.go b/test/types_test.go new file mode 100644 index 000000000..137f74ec8 --- /dev/null +++ b/test/types_test.go @@ -0,0 +1,47 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import "testing" + +func TestFeatureEnabled(t *testing.T) { + tests := []struct { + name string + features string + feature string + want bool + }{ + {"empty value", "", FeatureCSI, false}, + {"exact match", "EnableCSI", FeatureCSI, true}, + {"different case", "enablecsi", FeatureCSI, true}, + {"first of a list", "EnableCSI,EnableAPIGroupVersions", FeatureCSI, true}, + {"last of a list", "EnableAPIGroupVersions,EnableCSI", FeatureCSI, true}, + {"middle of a list", "A,EnableCSI,B", FeatureCSI, true}, + {"surrounding spaces", " EnableCSI ", FeatureCSI, true}, + {"spaces after a comma", "EnableAPIGroupVersions, EnableCSI", FeatureCSI, true}, + {"absent", "EnableAPIGroupVersions", FeatureCSI, false}, + {"longer name sharing the prefix", "EnableCSIFoo", FeatureCSI, false}, + {"name as a substring of another", "NotEnableCSI", FeatureCSI, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := FeatureEnabled(tc.features, tc.feature); got != tc.want { + t.Errorf("FeatureEnabled(%q, %q) = %v, want %v", tc.features, tc.feature, got, tc.want) + } + }) + } +} diff --git a/test/util/kibishii/kibishii_utils.go b/test/util/kibishii/kibishii_utils.go index 651bdde11..bf62c06de 100644 --- a/test/util/kibishii/kibishii_utils.go +++ b/test/util/kibishii/kibishii_utils.go @@ -313,17 +313,17 @@ func installKibishii( workerOS string, ) error { if strings.EqualFold(cloudPlatform, Azure) && - strings.EqualFold(veleroFeatures, FeatureCSI) { + FeatureEnabled(veleroFeatures, FeatureCSI) { cloudPlatform = AzureCSI } if strings.EqualFold(cloudPlatform, AWS) && - strings.EqualFold(veleroFeatures, FeatureCSI) { + FeatureEnabled(veleroFeatures, FeatureCSI) { cloudPlatform = AwsCSI } // Keep this in step with KibishiiVolumesAreHostPath above, which is its // inverse: the cases skip exactly when this branch does not fire. if strings.EqualFold(cloudPlatform, Kind) && - strings.EqualFold(veleroFeatures, FeatureCSI) { + FeatureEnabled(veleroFeatures, FeatureCSI) { cloudPlatform = KindCSI } diff --git a/test/util/velero/velero_utils.go b/test/util/velero/velero_utils.go index 85b3643cb..35f5bc4ec 100644 --- a/test/util/velero/velero_utils.go +++ b/test/util/velero/velero_utils.go @@ -1318,7 +1318,7 @@ func BuildSnapshotCheckPointFromVolumeInfo( snapshotCheckPoint.PodName = KibishiiPVCNameList if (veleroCfg.CloudProvider == Azure || veleroCfg.CloudProvider == AWS || - veleroCfg.CloudProvider == Kind) && strings.EqualFold(veleroCfg.Features, FeatureCSI) { + veleroCfg.CloudProvider == Kind) && FeatureEnabled(veleroCfg.Features, FeatureCSI) { snapshotCheckPoint.EnableCSI = true var VscCount = 0