diff --git a/charts/latest/templates/daemonset.yaml b/charts/latest/templates/daemonset.yaml index 715a3037..e91b0c30 100644 --- a/charts/latest/templates/daemonset.yaml +++ b/charts/latest/templates/daemonset.yaml @@ -93,6 +93,10 @@ spec: - --enable-lvm-orphan-cleanup={{ .Values.cleanup.lvmOrphanCleanup.enabled }} - --lvm-orphan-cleanup-interval={{ .Values.cleanup.lvmOrphanCleanup.interval }} - --run-alongside-webhook={{ .Values.webhook.hyperconverged.enabled | default false }} + + - --disk-path-prefixes={{ join "," (.Values.diskSelection.extraPathPrefixes | default list) }} + - --disk-models={{ join "," (.Values.diskSelection.extraModels | default list) }} + - --disk-types={{ join "," (.Values.diskSelection.extraTypes | default list) }} command: - /local-csi-driver env: diff --git a/charts/latest/values.yaml b/charts/latest/values.yaml index 80da7179..81071c0d 100644 --- a/charts/latest/values.yaml +++ b/charts/latest/values.yaml @@ -78,6 +78,22 @@ raid: # StorageClass in the `VolumeGroup` parameter. volumeGroup: containerstorage + +# Disk selection configuration. +# Controls which block devices the driver picks up as LVM physical volumes. +# A device must match ALL of the criteria below (path prefix AND model AND +# type). Within each list, a device matches if it satisfies ANY entry. +# Leave a list empty to disable filtering on that attribute. The defaults +# target NVMe disks found on Azure VMs. +diskSelection: + # Additional device path prefixes to select, appended to the built-in defaults. + extraPathPrefixes: [] + # Additional disk models to select, appended to the built-in defaults. + # Add new SKUs here as they are introduced. + extraModels: [] + # Additional device types to select, appended to the built-in defaults. + extraTypes: [] + # Garbage collection and cleanup configuration. cleanup: # Cleanup volume groups and physical volumes on pod termination if logical volume are not in use. diff --git a/cmd/driver/main.go b/cmd/driver/main.go index dcd9f999..6abe5d13 100644 --- a/cmd/driver/main.go +++ b/cmd/driver/main.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) @@ -88,6 +89,9 @@ func main() { var enableLVMOrphanCleanup bool var lvmOrphanCleanupInterval time.Duration var runAlongsideWebhook bool + var diskPathPrefixes string + var diskModels string + var diskTypes string flag.StringVar(&nodeName, "node-name", "", "The name of the node this agent is running on.") flag.StringVar(&podName, "pod-name", "", @@ -129,7 +133,12 @@ func main() { "Interval for the LVM orphan cleanup controller to scan and clean up orphaned volumes.") flag.BoolVar(&runAlongsideWebhook, "run-alongside-webhook", false, "If set, indicates that the driver is running alongside a separate webhook deployment. This affects PV node affinity behavior.") - // Initialize logger flagsconfig. + flag.StringVar(&diskPathPrefixes, "disk-path-prefixes", "", + "Comma-separated list of additional device path prefixes to select (e.g. /dev/sda), appended to the built-in defaults.") + flag.StringVar(&diskModels, "disk-models", "", + "Comma-separated list of additional device models to select, appended to the built-in defaults.") + flag.StringVar(&diskTypes, "disk-types", "", + "Comma-separated list of additional device types to select (e.g. loop), appended to the built-in defaults.") // Initialize logger flagsconfig. logConfig := textlogger.NewConfig(textlogger.VerbosityFlagName("v")) logConfig.AddFlags(flag.CommandLine) @@ -239,8 +248,18 @@ func main() { // TODO(sc): move filter to controller so we can read filters from // storageclass params. Hardcoded for now. + + // Build the disk selection filter by appending any CLI-provided entries to + // the built-in defaults. This preserves the default NVMe selection while + // letting operators add extra path prefixes, models, or types via Helm. + diskFilter := probe.NewEphemeralDiskFilter( + strings.Split(diskPathPrefixes, ","), + strings.Split(diskModels, ","), + strings.Split(diskTypes, ","), + ) + blockDevUtils := block.New() - deviceProbe := probe.New(blockDevUtils, probe.EphemeralDiskFilter) + deviceProbe := probe.New(blockDevUtils, diskFilter) // Create the LVM manager. // LVM manager is an abstraction that understands how to create and @@ -276,7 +295,7 @@ func main() { // Run startup diagnostic to check disk availability and emit a Warning // event on the pod if no disks are available for volume group creation. - startupDiag := lvm.NewStartupDiagnostic(deviceProbe, blockDevUtils, probe.EphemeralDiskFilter, recorder, selfPod) + startupDiag := lvm.NewStartupDiagnostic(deviceProbe, blockDevUtils, diskFilter, recorder, selfPod) if err := mgr.Add(startupDiag); err != nil { logAndExit(err, "unable to add startup diagnostic to manager") } diff --git a/internal/csi/core/lvm/startup_test.go b/internal/csi/core/lvm/startup_test.go index 6cab465e..4ee8145b 100644 --- a/internal/csi/core/lvm/startup_test.go +++ b/internal/csi/core/lvm/startup_test.go @@ -61,7 +61,7 @@ func TestStartupDiagnostic_DisksAvailable(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -108,7 +108,7 @@ func TestStartupDiagnostic_DisksAvailable_NoInUse(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -147,7 +147,7 @@ func TestStartupDiagnostic_NoDisks_AllFormattedNVMe(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -189,7 +189,7 @@ func TestStartupDiagnostic_NoDisks_NoNVMe(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -222,7 +222,7 @@ func TestStartupDiagnostic_ScanError(t *testing.T) { mockBlock := block.NewMock(ctrl) recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -257,7 +257,7 @@ func TestStartupDiagnostic_MultipleNVMe_SomeFormatted(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -300,7 +300,7 @@ func TestStartupDiagnostic_GetDevicesError(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -335,7 +335,7 @@ func TestStartupDiagnostic_IsFormattedError(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) @@ -370,7 +370,7 @@ func TestStartupDiagnostic_IsLVM2Error(t *testing.T) { recorder := kevents.NewFakeRecorder(10) - diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.EphemeralDiskFilter, recorder, newTestPod()) + diag := lvm.NewStartupDiagnostic(mockProbe, mockBlock, probe.NewEphemeralDiskFilter(nil, nil, nil), recorder, newTestPod()) if err := diag.Start(context.Background()); err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/internal/pkg/probe/filter.go b/internal/pkg/probe/filter.go index 0a618cc8..3b8de458 100644 --- a/internal/pkg/probe/filter.go +++ b/internal/pkg/probe/filter.go @@ -9,13 +9,81 @@ import ( "local-csi-driver/internal/pkg/block" ) -// EphemeralDiskFilter is a filter for ephemeral disks. -var EphemeralDiskFilter = &Filter{ - Filters: []FilterPredicate{ - &PathFilter{Path: "/dev/nvme"}, - NewModelFilter("Microsoft NVMe Direct Disk", "Microsoft NVMe Direct Disk v2"), - &TypeFilter{Type: "disk"}, - }, +// Default disk selection values. These preserve the historical hardcoded +// behavior when no overrides are provided via CLI flags. +var ( + DefaultDiskPathPrefixes = []string{"/dev/nvme"} + DefaultDiskModels = []string{ + "Microsoft NVMe Direct Disk", + "Microsoft NVMe Direct Disk v2", + "Amazon EC2 NVMe Instance Storage", + } + DefaultDiskTypes = []string{"disk"} +) + +// NewEphemeralDiskFilter builds a Filter from the built-in defaults plus any +// extra path prefixes, models, and types (for example, sourced from CLI flags). +// A device must match all three categories (path AND model AND type); within a +// category it matches if it satisfies any entry. Extra entries are appended to +// the defaults; empty/whitespace entries and case-insensitive duplicates are +// ignored. +func NewEphemeralDiskFilter(extraPathPrefixes, extraModels, extraTypes []string) *Filter { + pathPrefixes := appendUnique(DefaultDiskPathPrefixes, extraPathPrefixes) + models := appendUnique(DefaultDiskModels, extraModels) + types := appendUnique(DefaultDiskTypes, extraTypes) + + anyPath := make([]FilterPredicate, 0, len(pathPrefixes)) + for _, p := range pathPrefixes { + anyPath = append(anyPath, &PathFilter{Path: p}) + } + + anyType := make([]FilterPredicate, 0, len(types)) + for _, t := range types { + anyType = append(anyType, &TypeFilter{Type: t}) + } + + return &Filter{Filters: []FilterPredicate{ + &anyFilter{filters: anyPath}, + NewModelFilter(models...), + &anyFilter{filters: anyType}, + }} +} + +// appendUnique returns base followed by the entries of extra that are not +// already present. Entries are trimmed, empty entries are dropped, and +// de-duplication is case-insensitive. +func appendUnique(base, extra []string) []string { + seen := make(map[string]struct{}) + out := make([]string, 0, len(base)+len(extra)) + for _, list := range [][]string{base, extra} { + for _, s := range list { + s = strings.TrimSpace(s) + if s == "" { + continue + } + key := strings.ToLower(s) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + out = append(out, s) + } + } + return out +} + +// anyFilter matches if any of its contained predicates match (OR). +type anyFilter struct { + filters []FilterPredicate +} + +func (f *anyFilter) Match(device block.Device) bool { + for _, filter := range f.filters { + if filter.Match(device) { + return true + } + } + return false } // FilterPredicate defines a predicate for filtering devices. diff --git a/internal/pkg/probe/filter_test.go b/internal/pkg/probe/filter_test.go index d585546a..85627270 100644 --- a/internal/pkg/probe/filter_test.go +++ b/internal/pkg/probe/filter_test.go @@ -151,41 +151,168 @@ func TestFilter(t *testing.T) { } } -func TestEphemeralDiskFilter(t *testing.T) { +func TestAnyFilter(t *testing.T) { tests := []struct { name string + filters []FilterPredicate device block.Device expected bool }{ { - // Standard_NC40ads_H100_v5 nodes have a different model name - // for the ephemeral disk. This test case is to ensure that - // the filter matches the model name for these nodes. - name: "match disk for v2 direct disk nodes", - device: block.Device{ - Path: "/dev/nvme0n1", - Type: "disk", - Model: "Microsoft NVMe Direct Disk v2 ", + name: "matches when first predicate matches", + filters: []FilterPredicate{ + &PathFilter{Path: "/dev/nvme"}, + &PathFilter{Path: "/dev/sda"}, }, + device: block.Device{Path: "/dev/nvme0n1"}, expected: true, }, { - name: "match disk for direct disk nodes", - device: block.Device{ - Path: "/dev/nvme0n1", - Type: "disk", - Model: "Microsoft NVMe Direct Disk ", + name: "matches when second predicate matches", + filters: []FilterPredicate{ + &PathFilter{Path: "/dev/nvme"}, + &PathFilter{Path: "/dev/sda"}, }, + device: block.Device{Path: "/dev/sda"}, expected: true, }, + { + name: "no match when none match", + filters: []FilterPredicate{ + &PathFilter{Path: "/dev/nvme"}, + &PathFilter{Path: "/dev/sda"}, + }, + device: block.Device{Path: "/dev/vdb"}, + expected: false, + }, + { + name: "no match when empty", + filters: []FilterPredicate{}, + device: block.Device{Path: "/dev/nvme0n1"}, + expected: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := EphemeralDiskFilter.Match(tt.device) + filter := &anyFilter{filters: tt.filters} + result := filter.Match(tt.device) if result != tt.expected { t.Errorf("Match(%v) = %v, want %v", tt.device, result, tt.expected) } }) } } + +func TestAppendUnique(t *testing.T) { + tests := []struct { + name string + base []string + extra []string + expected []string + }{ + { + name: "appends new entries", + base: []string{"/dev/nvme"}, + extra: []string{"/dev/sda"}, + expected: []string{"/dev/nvme", "/dev/sda"}, + }, + { + name: "drops case-insensitive duplicates", + base: []string{"Microsoft NVMe Direct Disk"}, + extra: []string{"microsoft nvme direct disk", "Contoso Disk"}, + expected: []string{"Microsoft NVMe Direct Disk", "Contoso Disk"}, + }, + { + name: "trims and drops empty extras", + base: []string{"disk"}, + extra: []string{" ", "loop "}, + expected: []string{"disk", "loop"}, + }, + { + name: "nil extra returns base", + base: []string{"disk"}, + extra: nil, + expected: []string{"disk"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := appendUnique(tt.base, tt.extra) + if len(result) != len(tt.expected) { + t.Fatalf("appendUnique(%v, %v) = %v, want %v", tt.base, tt.extra, result, tt.expected) + } + for i := range result { + if result[i] != tt.expected[i] { + t.Errorf("appendUnique[%d] = %q, want %q", i, result[i], tt.expected[i]) + } + } + }) + } +} + +func TestNewEphemeralDiskFilter(t *testing.T) { + tests := []struct { + name string + extraPaths []string + extraModels []string + extraTypes []string + device block.Device + expected bool + }{ + { + name: "built-in default model matches with no extras", + device: block.Device{Path: "/dev/nvme0n1", Model: "Microsoft NVMe Direct Disk", Type: "disk"}, + expected: true, + }, + { + name: "default amazon model matches with no extras", + device: block.Device{Path: "/dev/nvme0n1", Model: "Amazon EC2 NVMe Instance Storage", Type: "disk"}, + expected: true, + }, + { + name: "extra model is matched", + extraModels: []string{"Contoso NVMe Disk"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Contoso NVMe Disk", Type: "disk"}, + expected: true, + }, + { + name: "default model still matches after adding an extra", + extraModels: []string{"Contoso NVMe Disk"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Microsoft NVMe Direct Disk v2", Type: "disk"}, + expected: true, + }, + { + name: "extra path prefix matches with a default model", + extraPaths: []string{"/dev/sda"}, + device: block.Device{Path: "/dev/sda", Model: "Microsoft NVMe Direct Disk", Type: "disk"}, + expected: true, + }, + { + name: "extra type is matched with a default model", + extraTypes: []string{"loop"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Microsoft NVMe Direct Disk", Type: "loop"}, + expected: true, + }, + { + name: "unknown model is rejected", + device: block.Device{Path: "/dev/nvme0n1", Model: "Unknown Disk", Type: "disk"}, + expected: false, + }, + { + name: "non-default path is rejected without an extra prefix", + device: block.Device{Path: "/dev/sda", Model: "Microsoft NVMe Direct Disk", Type: "disk"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filter := NewEphemeralDiskFilter(tt.extraPaths, tt.extraModels, tt.extraTypes) + if got := filter.Match(tt.device); got != tt.expected { + t.Errorf("Match(%v) = %v, want %v", tt.device, got, tt.expected) + } + }) + } +} diff --git a/test/e2e/lvm_expansion_test.go b/test/e2e/lvm_expansion_test.go index be43fad5..5d28c4ad 100644 --- a/test/e2e/lvm_expansion_test.go +++ b/test/e2e/lvm_expansion_test.go @@ -186,7 +186,7 @@ func getDriverPodOnNode(ctx context.Context, node string) (string, error) { if err != nil { return "", err } - for _, line := range strings.Split(strings.TrimSpace(out), "\n") { + for line := range strings.SplitSeq(strings.TrimSpace(out), "\n") { fields := strings.Split(line, "\t") if len(fields) != 2 { continue diff --git a/test/e2e/lvm_failover_test.go b/test/e2e/lvm_failover_test.go index 30c80fa2..42e36f7d 100644 --- a/test/e2e/lvm_failover_test.go +++ b/test/e2e/lvm_failover_test.go @@ -232,7 +232,7 @@ func pickDifferentDriverNode(ctx context.Context, exclude string) string { "-l", "app.kubernetes.io/component=csi-local-node", "-o", "jsonpath={range .items[*]}{.spec.nodeName}{\"\\n\"}{end}")) Expect(err).NotTo(HaveOccurred(), "Failed to list csi-local-node pods") - for _, line := range strings.Split(strings.TrimSpace(out), "\n") { + for line := range strings.SplitSeq(strings.TrimSpace(out), "\n") { node := strings.TrimSpace(line) if node != "" && node != exclude { return node