From 88344edc6160f697169c63f403cce3673306263a Mon Sep 17 00:00:00 2001 From: Mahashri Saini Date: Fri, 17 Jul 2026 11:07:50 +0000 Subject: [PATCH 1/3] model-filtering --- charts/latest/templates/daemonset.yaml | 11 ++ charts/latest/values.yaml | 22 +++ cmd/driver/main.go | 24 +++- internal/pkg/probe/filter.go | 72 +++++++++- internal/pkg/probe/filter_test.go | 191 +++++++++++++++++++++++++ test/e2e/lvm_expansion_test.go | 2 +- test/e2e/lvm_failover_test.go | 2 +- 7 files changed, 312 insertions(+), 12 deletions(-) diff --git a/charts/latest/templates/daemonset.yaml b/charts/latest/templates/daemonset.yaml index 715a3037..1bb57d6b 100644 --- a/charts/latest/templates/daemonset.yaml +++ b/charts/latest/templates/daemonset.yaml @@ -93,6 +93,17 @@ 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 }} + + {{- if .Values.diskSelection.pathPrefixes }} + - --disk-path-prefixes={{ join "," .Values.diskSelection.pathPrefixes }} + {{- end }} + {{- if .Values.diskSelection.models }} + - --disk-models={{ join "," .Values.diskSelection.models }} + {{- end }} + {{- if .Values.diskSelection.types }} + - --disk-types={{ join "," .Values.diskSelection.types }} + {{- end }} + command: - /local-csi-driver env: diff --git a/charts/latest/values.yaml b/charts/latest/values.yaml index 80da7179..4e1af17d 100644 --- a/charts/latest/values.yaml +++ b/charts/latest/values.yaml @@ -78,6 +78,28 @@ 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: + # Path prefixes a device must start with, e.g. /dev/nvme. + pathPrefixes: + - /dev/nvme + # NVMe disk models to match. Add new models here as new SKUs are introduced. + models: + - Microsoft NVMe Direct Disk + - Microsoft NVMe Direct Disk v2 + - Amazon EC2 NVMe Instance Storage + - Amazon Elastic Block Store + # Device types to match, e.g. disk (as opposed to loop, part, etc.). + types: + - disk + + # 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..cc202eb3 100644 --- a/cmd/driver/main.go +++ b/cmd/driver/main.go @@ -9,8 +9,8 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -88,6 +88,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,6 +132,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.") + flag.StringVar(&diskPathPrefixes, "disk-path-prefixes", strings.Join(probe.DefaultDiskPathPrefixes, ","), + "Comma-separated list of device path prefixes to select, e.g. /dev/nvme. Empty disables path filtering.") + flag.StringVar(&diskModels, "disk-models", strings.Join(probe.DefaultDiskModels, ","), + "Comma-separated list of device models to select. Empty disables model filtering.") + flag.StringVar(&diskTypes, "disk-types", strings.Join(probe.DefaultDiskTypes, ","), + "Comma-separated list of device types to select, e.g. disk. Empty disables type filtering.") // Initialize logger flagsconfig. logConfig := textlogger.NewConfig(textlogger.VerbosityFlagName("v")) logConfig.AddFlags(flag.CommandLine) @@ -239,8 +248,17 @@ func main() { // TODO(sc): move filter to controller so we can read filters from // storageclass params. Hardcoded for now. + + // Build the disk selection filter from CLI args. Defaults preserve the + // historical NVMe selection behavior when the flags are not overridden. + 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 +294,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/pkg/probe/filter.go b/internal/pkg/probe/filter.go index 0a618cc8..2c2bf3e1 100644 --- a/internal/pkg/probe/filter.go +++ b/internal/pkg/probe/filter.go @@ -9,13 +9,71 @@ 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"} + DefaultDiskTypes = []string{"disk"} +) + +// EphemeralDiskFilter is a filter for ephemeral disks using default values. +var EphemeralDiskFilter = NewEphemeralDiskFilter(DefaultDiskPathPrefixes, DefaultDiskModels, DefaultDiskTypes) + +// NewEphemeralDiskFilter builds a Filter from the given path prefixes, models, +// and types. A device must match all non-empty categories (path AND model AND +// type); within a category it matches if it satisfies any entry. Empty or +// whitespace-only entries are ignored, and an empty category is skipped so it +// does not filter anything out. +func NewEphemeralDiskFilter(pathPrefixes, models, types []string) *Filter { + filters := make([]FilterPredicate, 0, 3) + + if paths := nonEmpty(pathPrefixes); len(paths) > 0 { + anyPath := make([]FilterPredicate, 0, len(paths)) + for _, p := range paths { + anyPath = append(anyPath, &PathFilter{Path: p}) + } + filters = append(filters, &anyFilter{filters: anyPath}) + } + + if m := nonEmpty(models); len(m) > 0 { + filters = append(filters, NewModelFilter(m...)) + } + + if ts := nonEmpty(types); len(ts) > 0 { + anyType := make([]FilterPredicate, 0, len(ts)) + for _, t := range ts { + anyType = append(anyType, &TypeFilter{Type: t}) + } + filters = append(filters, &anyFilter{filters: anyType}) + } + + return &Filter{Filters: filters} +} + +// nonEmpty returns the input with whitespace trimmed and empty entries removed. +func nonEmpty(in []string) []string { + out := make([]string, 0, len(in)) + for _, s := range in { + if s = strings.TrimSpace(s); s != "" { + 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..5071bcd5 100644 --- a/internal/pkg/probe/filter_test.go +++ b/internal/pkg/probe/filter_test.go @@ -189,3 +189,194 @@ func TestEphemeralDiskFilter(t *testing.T) { }) } } + +func TestAnyFilter(t *testing.T) { + tests := []struct { + name string + filters []FilterPredicate + device block.Device + expected bool + }{ + { + 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: "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) { + 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 TestNonEmpty(t *testing.T) { + tests := []struct { + name string + in []string + expected []string + }{ + { + name: "trims whitespace", + in: []string{" /dev/nvme ", "\t/dev/sda\n"}, + expected: []string{"/dev/nvme", "/dev/sda"}, + }, + { + name: "removes empty and whitespace-only entries", + in: []string{"/dev/nvme", "", " ", "/dev/sda"}, + expected: []string{"/dev/nvme", "/dev/sda"}, + }, + { + name: "all empty yields empty slice", + in: []string{"", " ", "\t"}, + expected: []string{}, + }, + { + name: "nil input yields empty slice", + in: nil, + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := nonEmpty(tt.in) + if len(result) != len(tt.expected) { + t.Fatalf("nonEmpty(%v) = %v, want %v", tt.in, result, tt.expected) + } + for i := range result { + if result[i] != tt.expected[i] { + t.Errorf("nonEmpty(%v)[%d] = %q, want %q", tt.in, i, result[i], tt.expected[i]) + } + } + }) + } +} + +func TestNewEphemeralDiskFilter(t *testing.T) { + tests := []struct { + name string + pathPrefixes []string + models []string + types []string + device block.Device + expected bool + }{ + { + name: "default selection matches nvme disk", + pathPrefixes: []string{"/dev/nvme"}, + models: []string{"Microsoft NVMe Direct Disk"}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Microsoft NVMe Direct Disk", Type: "disk"}, + expected: true, + }, + { + name: "path OR matches second prefix", + pathPrefixes: []string{"/dev/nvme", "/dev/sda"}, + models: []string{"Contoso Disk"}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/sda", Model: "Contoso Disk", Type: "disk"}, + expected: true, + }, + { + name: "path matches no prefix is rejected", + pathPrefixes: []string{"/dev/nvme", "/dev/sda"}, + models: []string{"Contoso Disk"}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/vdb", Model: "Contoso Disk", Type: "disk"}, + expected: false, + }, + { + name: "type OR matches second type", + pathPrefixes: []string{"/dev/nvme"}, + models: []string{"Contoso Disk"}, + types: []string{"disk", "loop"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Contoso Disk", Type: "loop"}, + expected: true, + }, + { + name: "model mismatch rejected despite matching path and type", + pathPrefixes: []string{"/dev/nvme"}, + models: []string{"Contoso Disk"}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Intel Disk", Type: "disk"}, + expected: false, + }, + { + name: "whitespace-only entries are ignored within a category", + pathPrefixes: []string{" ", "/dev/nvme"}, + models: []string{"Contoso Disk"}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Contoso Disk", Type: "disk"}, + expected: true, + }, + { + name: "empty model category is not filtered", + pathPrefixes: []string{"/dev/nvme"}, + models: []string{}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/nvme0n1", Model: "Any Model At All", Type: "disk"}, + expected: true, + }, + { + name: "empty model category still enforces path and type", + pathPrefixes: []string{"/dev/nvme"}, + models: []string{}, + types: []string{"disk"}, + device: block.Device{Path: "/dev/sda", Model: "Any Model At All", Type: "disk"}, + expected: false, + }, + { + name: "all categories empty matches any device", + pathPrefixes: []string{}, + models: []string{}, + types: []string{}, + device: block.Device{Path: "/dev/anything", Model: "whatever", Type: "part"}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filter := NewEphemeralDiskFilter(tt.pathPrefixes, tt.models, tt.types) + result := filter.Match(tt.device) + if result != tt.expected { + t.Errorf("Match(%v) = %v, want %v", tt.device, result, 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 From 4a0f18b951457f74ed956391bdebebcf1125ef32 Mon Sep 17 00:00:00 2001 From: Mahashri Saini Date: Fri, 17 Jul 2026 12:26:22 +0000 Subject: [PATCH 2/3] model-filtering --- charts/latest/templates/daemonset.yaml | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/charts/latest/templates/daemonset.yaml b/charts/latest/templates/daemonset.yaml index 1bb57d6b..0b76ce83 100644 --- a/charts/latest/templates/daemonset.yaml +++ b/charts/latest/templates/daemonset.yaml @@ -94,15 +94,9 @@ spec: - --lvm-orphan-cleanup-interval={{ .Values.cleanup.lvmOrphanCleanup.interval }} - --run-alongside-webhook={{ .Values.webhook.hyperconverged.enabled | default false }} - {{- if .Values.diskSelection.pathPrefixes }} - - --disk-path-prefixes={{ join "," .Values.diskSelection.pathPrefixes }} - {{- end }} - {{- if .Values.diskSelection.models }} - - --disk-models={{ join "," .Values.diskSelection.models }} - {{- end }} - {{- if .Values.diskSelection.types }} - - --disk-types={{ join "," .Values.diskSelection.types }} - {{- end }} + - --disk-path-prefixes={{ join "," (.Values.diskSelection.pathPrefixes | default (list "/dev/nvme")) }} + - --disk-models={{ join "," (.Values.diskSelection.models | default (list "Microsoft NVMe Direct Disk" "Microsoft NVMe Direct Disk v2")) }} + - --disk-types={{ join "," (.Values.diskSelection.types | default (list "disk")) }} command: - /local-csi-driver From 51d3bda774dce81470a44c784af5e94b2e3a7073 Mon Sep 17 00:00:00 2001 From: Mahashri Saini Date: Mon, 20 Jul 2026 11:20:14 +0000 Subject: [PATCH 3/3] model-filtering for aws-nvmes --- charts/latest/templates/daemonset.yaml | 7 +- charts/latest/values.yaml | 20 +- cmd/driver/main.go | 19 +- internal/csi/core/lvm/startup_test.go | 18 +- internal/pkg/probe/filter.go | 80 +++--- internal/pkg/probe/filter_test.go | 334 ++++++++++--------------- 6 files changed, 209 insertions(+), 269 deletions(-) diff --git a/charts/latest/templates/daemonset.yaml b/charts/latest/templates/daemonset.yaml index 0b76ce83..e91b0c30 100644 --- a/charts/latest/templates/daemonset.yaml +++ b/charts/latest/templates/daemonset.yaml @@ -94,10 +94,9 @@ spec: - --lvm-orphan-cleanup-interval={{ .Values.cleanup.lvmOrphanCleanup.interval }} - --run-alongside-webhook={{ .Values.webhook.hyperconverged.enabled | default false }} - - --disk-path-prefixes={{ join "," (.Values.diskSelection.pathPrefixes | default (list "/dev/nvme")) }} - - --disk-models={{ join "," (.Values.diskSelection.models | default (list "Microsoft NVMe Direct Disk" "Microsoft NVMe Direct Disk v2")) }} - - --disk-types={{ join "," (.Values.diskSelection.types | default (list "disk")) }} - + - --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 4e1af17d..81071c0d 100644 --- a/charts/latest/values.yaml +++ b/charts/latest/values.yaml @@ -86,19 +86,13 @@ raid: # Leave a list empty to disable filtering on that attribute. The defaults # target NVMe disks found on Azure VMs. diskSelection: - # Path prefixes a device must start with, e.g. /dev/nvme. - pathPrefixes: - - /dev/nvme - # NVMe disk models to match. Add new models here as new SKUs are introduced. - models: - - Microsoft NVMe Direct Disk - - Microsoft NVMe Direct Disk v2 - - Amazon EC2 NVMe Instance Storage - - Amazon Elastic Block Store - # Device types to match, e.g. disk (as opposed to loop, part, etc.). - types: - - disk - + # 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: diff --git a/cmd/driver/main.go b/cmd/driver/main.go index cc202eb3..6abe5d13 100644 --- a/cmd/driver/main.go +++ b/cmd/driver/main.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" "time" + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -132,13 +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.") - flag.StringVar(&diskPathPrefixes, "disk-path-prefixes", strings.Join(probe.DefaultDiskPathPrefixes, ","), - "Comma-separated list of device path prefixes to select, e.g. /dev/nvme. Empty disables path filtering.") - flag.StringVar(&diskModels, "disk-models", strings.Join(probe.DefaultDiskModels, ","), - "Comma-separated list of device models to select. Empty disables model filtering.") - flag.StringVar(&diskTypes, "disk-types", strings.Join(probe.DefaultDiskTypes, ","), - "Comma-separated list of device types to select, e.g. disk. Empty disables type filtering.") - // 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) @@ -249,8 +249,9 @@ func main() { // TODO(sc): move filter to controller so we can read filters from // storageclass params. Hardcoded for now. - // Build the disk selection filter from CLI args. Defaults preserve the - // historical NVMe selection behavior when the flags are not overridden. + // 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, ","), 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 2c2bf3e1..3b8de458 100644 --- a/internal/pkg/probe/filter.go +++ b/internal/pkg/probe/filter.go @@ -13,49 +13,59 @@ import ( // 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"} - DefaultDiskTypes = []string{"disk"} -) - -// EphemeralDiskFilter is a filter for ephemeral disks using default values. -var EphemeralDiskFilter = NewEphemeralDiskFilter(DefaultDiskPathPrefixes, DefaultDiskModels, DefaultDiskTypes) - -// NewEphemeralDiskFilter builds a Filter from the given path prefixes, models, -// and types. A device must match all non-empty categories (path AND model AND -// type); within a category it matches if it satisfies any entry. Empty or -// whitespace-only entries are ignored, and an empty category is skipped so it -// does not filter anything out. -func NewEphemeralDiskFilter(pathPrefixes, models, types []string) *Filter { - filters := make([]FilterPredicate, 0, 3) - - if paths := nonEmpty(pathPrefixes); len(paths) > 0 { - anyPath := make([]FilterPredicate, 0, len(paths)) - for _, p := range paths { - anyPath = append(anyPath, &PathFilter{Path: p}) - } - filters = append(filters, &anyFilter{filters: anyPath}) + DefaultDiskModels = []string{ + "Microsoft NVMe Direct Disk", + "Microsoft NVMe Direct Disk v2", + "Amazon EC2 NVMe Instance Storage", } + DefaultDiskTypes = []string{"disk"} +) - if m := nonEmpty(models); len(m) > 0 { - filters = append(filters, NewModelFilter(m...)) +// 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}) } - if ts := nonEmpty(types); len(ts) > 0 { - anyType := make([]FilterPredicate, 0, len(ts)) - for _, t := range ts { - anyType = append(anyType, &TypeFilter{Type: t}) - } - filters = append(filters, &anyFilter{filters: anyType}) + anyType := make([]FilterPredicate, 0, len(types)) + for _, t := range types { + anyType = append(anyType, &TypeFilter{Type: t}) } - return &Filter{Filters: filters} + return &Filter{Filters: []FilterPredicate{ + &anyFilter{filters: anyPath}, + NewModelFilter(models...), + &anyFilter{filters: anyType}, + }} } -// nonEmpty returns the input with whitespace trimmed and empty entries removed. -func nonEmpty(in []string) []string { - out := make([]string, 0, len(in)) - for _, s := range in { - if s = strings.TrimSpace(s); s != "" { +// 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) } } diff --git a/internal/pkg/probe/filter_test.go b/internal/pkg/probe/filter_test.go index 5071bcd5..85627270 100644 --- a/internal/pkg/probe/filter_test.go +++ b/internal/pkg/probe/filter_test.go @@ -151,38 +151,52 @@ 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) } @@ -190,193 +204,115 @@ func TestEphemeralDiskFilter(t *testing.T) { } } -func TestAnyFilter(t *testing.T) { - tests := []struct { - name string - filters []FilterPredicate - device block.Device - expected bool - }{ - { - 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: "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) { - 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 TestNonEmpty(t *testing.T) { - tests := []struct { - name string - in []string - expected []string - }{ - { - name: "trims whitespace", - in: []string{" /dev/nvme ", "\t/dev/sda\n"}, - expected: []string{"/dev/nvme", "/dev/sda"}, - }, - { - name: "removes empty and whitespace-only entries", - in: []string{"/dev/nvme", "", " ", "/dev/sda"}, - expected: []string{"/dev/nvme", "/dev/sda"}, - }, - { - name: "all empty yields empty slice", - in: []string{"", " ", "\t"}, - expected: []string{}, - }, - { - name: "nil input yields empty slice", - in: nil, - expected: []string{}, - }, - } +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 := nonEmpty(tt.in) - if len(result) != len(tt.expected) { - t.Fatalf("nonEmpty(%v) = %v, want %v", tt.in, result, tt.expected) - } - for i := range result { - if result[i] != tt.expected[i] { - t.Errorf("nonEmpty(%v)[%d] = %q, want %q", tt.in, i, result[i], tt.expected[i]) - } - } - }) - } + 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 - pathPrefixes []string - models []string - types []string - device block.Device - expected bool - }{ - { - name: "default selection matches nvme disk", - pathPrefixes: []string{"/dev/nvme"}, - models: []string{"Microsoft NVMe Direct Disk"}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/nvme0n1", Model: "Microsoft NVMe Direct Disk", Type: "disk"}, - expected: true, - }, - { - name: "path OR matches second prefix", - pathPrefixes: []string{"/dev/nvme", "/dev/sda"}, - models: []string{"Contoso Disk"}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/sda", Model: "Contoso Disk", Type: "disk"}, - expected: true, - }, - { - name: "path matches no prefix is rejected", - pathPrefixes: []string{"/dev/nvme", "/dev/sda"}, - models: []string{"Contoso Disk"}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/vdb", Model: "Contoso Disk", Type: "disk"}, - expected: false, - }, - { - name: "type OR matches second type", - pathPrefixes: []string{"/dev/nvme"}, - models: []string{"Contoso Disk"}, - types: []string{"disk", "loop"}, - device: block.Device{Path: "/dev/nvme0n1", Model: "Contoso Disk", Type: "loop"}, - expected: true, - }, - { - name: "model mismatch rejected despite matching path and type", - pathPrefixes: []string{"/dev/nvme"}, - models: []string{"Contoso Disk"}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/nvme0n1", Model: "Intel Disk", Type: "disk"}, - expected: false, - }, - { - name: "whitespace-only entries are ignored within a category", - pathPrefixes: []string{" ", "/dev/nvme"}, - models: []string{"Contoso Disk"}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/nvme0n1", Model: "Contoso Disk", Type: "disk"}, - expected: true, - }, - { - name: "empty model category is not filtered", - pathPrefixes: []string{"/dev/nvme"}, - models: []string{}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/nvme0n1", Model: "Any Model At All", Type: "disk"}, - expected: true, - }, - { - name: "empty model category still enforces path and type", - pathPrefixes: []string{"/dev/nvme"}, - models: []string{}, - types: []string{"disk"}, - device: block.Device{Path: "/dev/sda", Model: "Any Model At All", Type: "disk"}, - expected: false, - }, - { - name: "all categories empty matches any device", - pathPrefixes: []string{}, - models: []string{}, - types: []string{}, - device: block.Device{Path: "/dev/anything", Model: "whatever", Type: "part"}, - expected: true, - }, - } + 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.pathPrefixes, tt.models, tt.types) - result := filter.Match(tt.device) - if result != tt.expected { - t.Errorf("Match(%v) = %v, want %v", tt.device, result, tt.expected) - } - }) - } + 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) + } + }) + } } -