mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-10-21 22:03:09 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

e2e: match --features as the comma-separated list it is (#10661)

Browse Source 
* e2e: match --features as the comma-separated list it is

--features takes a comma-separated list, but every check for EnableCSI in
the suite compares the whole flag value against the single feature name.
With --features=EnableCSI,EnableAPIGroupVersions none of them match, so a
CSI run quietly behaves as a non-CSI one: BeforeSuite skips installing the
VolumeSnapshotClass, installKibishii picks the provider's plain overlay
rather than its CSI one, and BuildSnapshotCheckPointFromVolumeInfo returns
a checkpoint with EnableCSI false and no snapshot handles, so the CSI
snapshot count is never compared against the expected one.

One site already used strings.Contains instead, which handles a list but
matches any feature whose name merely starts with this one, and misses a
value that differs only in case.

Add FeatureEnabled, which splits on commas and compares each entry whole
and case-insensitively, and route all eleven call sites through it. The
table test covers the list, case and prefix shapes that the two previous
idioms each got wrong.

hack/test.sh listed only ./pkg/..., ./internal/... and ./cmd/..., so
nothing under test/ was ever unit tested and this table would not have run
in CI. Add ./test, which holds the suite's shared types and nothing that
needs a cluster. The e2e suites live in subpackages and stay unlisted.

Found while reviewing the kind-csi mapping in #10657, which copied the
EqualFold form from the Azure and AWS branches beside it.

Signed-off-by: krishhna24 <krishhnatupedev@gmail.com>

* Add changelog for #10661

Signed-off-by: krishhna24 <krishhnatupedev@gmail.com>

---------

Signed-off-by: krishhna24 <krishhnatupedev@gmail.com>
This commit is contained in:
KrishhnaT
2026-10-21 06:10:17 -07:00
committed by GitHub
parent ae329706ae
commit 642a5bdf87
10 changed files with 78 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10661-krishhna24
+1
View File
@@ -0,0 +1 @@
Match --features as the comma-separated list it is when checking for EnableCSI
hack/test.sh
+4
View File
@@ -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
test/e2e/basic/backup-volume-info/base.go
+1 -1
View File
@@ -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.")
test/e2e/e2e_suite_test.go
+2 -2
View File
@@ -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(
test/e2e/migration/migration.go
+3 -3
View File
@@ -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(
test/e2e/upgrade/upgrade.go
+1 -2
View File
@@ -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() {
test/types.go
+15
View File
@@ -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"
test/types_test.go
+47
View File
@@ -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)
}
})
}
}
test/util/kibishii/kibishii_utils.go
+3 -3
View File
@@ -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
}
test/util/velero/velero_utils.go
+1 -1
View File
@@ -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