feat: support filter list

[Yu-Jack]

Problem:

We only support filter and auto-provision from pod env, which isn't easy to use.

Solution:

I added config map structure and the following features:

1. Adde device filter, which we didn't have before.

2. Loading config map to filter the blockdevice and select candidates for auto provision.

   1. Add creating a default config map resource under pkg/data. (reason)

   2. Set exclude filter and auto provision every time when scanner starts. It's useful when config map is changed. But, I don't think we need to notify scanner to scan again when the config map is changed. Just one way direction is enough.

3. Fallback mechanism. If config map doesn't exist, it directly uses the env variables. This is just for the upgrade if the upgrade fails, which causes some unexpected behaviors.

4. Add webhook to validate config map

   $ k edit configmap harvester-node-disk-manager  -n harvester-system
   error: configmaps "harvester-node-disk-manager" could not be patched: admission webhook "validator.harvester-system.harvester-node-disk-manager-webhook" denied the request: Internal error occurred: invalid filters.yaml: filter config at index 0 has empty hostname, which is not allowed
   You can run `kubectl replace -f /var/folders/tm/bthf15dx15s6rhf8p_9jy5k40000gn/T/kubectl-edit-3750804514.yaml` to try this update again.
   

p.s. I don't change too much logic inside the scanner, I'd like to keep it simple, so I just reuse it and change the interface of accepting the filters from the outside.

Out of the scope

1. Auto provision for longhorn v2 and LVM. This PR doesn't include that and only focuses on filter list. However, in order to support that in the future, the config map still accepts different provisions and params.

Related Issue:
harvester/harvester#5059

Test plan:

This is an example testing. There are too many test cases. I'll cover as many as possible in our unit tests.

apiVersion: v1
kind: ConfigMap
metadata:
  name: harvester-node-disk-manager
  namespace: harvester-system
data:
  autoprovision.yaml: |
    - hostname: "*"
      devices:
        - "/dev/sdc"
        - "/dev/sdd"
  filters.yaml: |
    - hostname: "*"
      excludeLabels: ["COS_*, HARV_*"]
      excludeVendors: ["longhorn", "thisisaexample"]
      excludeDevices: ["/dev/sdd"]
    - hostname: "harvester1"
      excludeVendors: ["harvester1"]
    - hostname: "harvester2"
      excludeVendors: ["harvester2"]

harvester1 node

• excludeLabels: ["COS_*, HARV_*"]
• excludeVendors: ["longhorn", "thisisaexample", "harvester1"]
• excludeDevices: ["/dev/sdd"]
• autoprovision: ["/dev/sdc", "/dev/sdd"]

harvester2 node

• excludeLabels: ["COS_*, HARV_*"]
• excludeVendors: ["longhorn", "thisisaexample", "harvester2"]
• excludeDevices: ["/dev/sdd"]
• autoprovision: ["/dev/sdc", "/dev/sdd"]

In the end, /dev/sdd is skipped because the of exclusion although it's in auto-provision list.

[Copilot]

Pull request overview

Adds ConfigMap-backed filter/autoprovision configuration (with fallback to env vars) and exposes filter “Details” for debug logging, enabling easier per-node disk selection without relying solely on pod env configuration.

Changes:

• Introduces ConfigMapLoader to load/merge filters.yaml and autoprovision.yaml configs (with hostname matching).
• Updates scanner to reload config on each scan and emit detailed debug output for active filters.
• Adds Helm chart ConfigMap template/values and unit tests for loader behavior.

Reviewed changes

Copilot reviewed 16 out of 303 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pkg/utils/fake/configmap.go	Adds a fake ConfigMap client adapter for tests.
pkg/filter/vendor_filter.go	Adds Details() output for vendor filters.
pkg/filter/path_filter.go	Adds Details() output for mount path filters.
pkg/filter/part_type_filter.go	Adds Details() output for partition type filters.
pkg/filter/label_filter.go	Adds Details() output for label filters.
pkg/filter/filter.go	Extends filter interfaces with Details() and adds device-path exclusion support.
pkg/filter/drive_type_filter.go	Adds Details() output for drive type filters.
pkg/filter/device_path_filter.go	Adds Details() output for device path filters.
pkg/filter/configmap_loader.go	Implements ConfigMap loading/parsing/merge logic for filters and autoprovision.
pkg/filter/configmap_loader_test.go	Adds unit tests for ConfigMapLoader behavior across multiple scenarios.
pkg/controller/blockdevice/scanner.go	Reloads config each scan and logs final filter configuration; threads ctx through scan.
pkg/controller/blockdevice/controller.go	Updates scanner start call to pass context.
cmd/node-disk-manager/main.go	Wires ConfigMapLoader into scanner creation and removes direct env-filter wiring.
deploy/charts/harvester-node-disk-manager/values.yaml	Adds chart values structure for ConfigMap-backed configuration.
deploy/charts/harvester-node-disk-manager/templates/configmap.yaml	Adds chart template to render filters/autoprovision data into a ConfigMap.
go.mod	Adds direct dependency on gopkg.in/yaml.v3 for YAML parsing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Vicente-Cheng]

overall lgtm, just some nits

[tserong]

I'm doing a bit of testing, and NDM doesn't seem to be noticing configmap changes. For example, if I run kubectl -n harvester-system edit configmap harvester-node-disk-manager then add an autoprovision filter, nothing happens (i.e. no disks are provisioned) unless I manually restart NDM.

[Yu-Jack]

I'm doing a bit of testing, and NDM doesn't seem to be noticing configmap changes. For example, if I run kubectl -n harvester-system edit configmap harvester-node-disk-manager then add an autoprovision filter, nothing happens (i.e. no disks are provisioned) unless I manually restart NDM.

Yeah, I don't add reconciling when config map is updated. But, when scanner is triggered again, it will read the new config map. Do you think we also need reconciling for changed config map?

[Yu-Jack]

Set exclude filter and auto provision every time when scanner starts. It's useful when config map is changed. But, I don't think we need to notify scanner to scan again when the config map is changed. Just one way direction is enough.

I mentioned it in this part of the original description.

[Yu-Jack]

webhook to validate config map

$ k edit configmap harvester-node-disk-manager  -n harvester-system
error: configmaps "harvester-node-disk-manager" could not be patched: admission webhook "validator.harvester-system.harvester-node-disk-manager-webhook" denied the request: Internal error occurred: invalid filters.yaml: filter config at index 0 has empty hostname, which is not allowed
You can run `kubectl replace -f /var/folders/tm/bthf15dx15s6rhf8p_9jy5k40000gn/T/kubectl-edit-3750804514.yaml` to try this update again.

[tserong]

I mentioned it in this part of the original description.

Yeah, sorry I re-read that after making my comment :-)

Yeah, I don't add reconciling when config map is updated. But, when scanner is triggered again, it will read the new config map. Do you think we also need reconciling for changed config map?

IIRC the scanner only triggers when devices are added or removed, so if a new device is added, re-reading the configmap makes sense then, because then we can know what to do with the new device.

But, consider the case where someone has deployed a new harvester system which has multiple disks per node, and now they decide they want to auto-provision all those disks. So they go edit the configmap to add the disks to provision and ... nothing happens. The disks don't get auto-provisioned.

[Yu-Jack]

IIRC the scanner only triggers when devices are added or removed, so if a new device is added, re-reading the configmap makes sense then, because then we can know what to do with the new device.

But, consider the case where someone has deployed a new harvester system which has multiple disks per node, and now they decide they want to auto-provision all those disks. So they go edit the configmap to add the disks to provision and ... nothing happens. The disks don't get auto-provisioned.

I agree with that case. But, when it comes down to deletion, thing would be a bit different. IIRC, we don't remove those unmatched block devices since day1? So, I think we can:

• Use two ways reconciling (in this PR)
• Add a better approach to delete block devices when it's not matched by rules (in the future issue, not in v1.8.0)

WDYT @tserong @Vicente-Cheng

[Vicente-Cheng]

IIRC the scanner only triggers when devices are added or removed, so if a new device is added, re-reading the configmap makes sense then, because then we can know what to do with the new device.
But, consider the case where someone has deployed a new harvester system which has multiple disks per node, and now they decide they want to auto-provision all those disks. So they go edit the configmap to add the disks to provision and ... nothing happens. The disks don't get auto-provisioned.

I agree with that case. But, when it comes down to deletion, thing would be a bit different. IIRC, we don't remove those unmatched block devices since day1? So, I think we can:

• Use two ways reconciling (in this PR)
• Add a better approach to delete block devices when it's not matched by rules (in the future issue, not in v1.8.0)

WDYT @tserong @Vicente-Cheng

Thanks @tserong for the good point with reconciling timing.

The above two steps look good to me.
We can handle the unmatched bd in a later version (and add it to the current limitation).

[Yu-Jack]

Done! cc @Vicente-Cheng @tserong It should work now!

[Vicente-Cheng]

lgtm, thanks!
Please squash yourself (for a few meaningful commits) or use the merge option.

All ok, up to you

[Yu-Jack]

Okay, I'll just use this.

[tserong]

LGTM, nice work!

I've found one little thing that probably wants removing (see below). There's also a few items for future work, but these need to be done elsewhere:

• harvester-installer adds labelFilter for COS_* and HARV_* which are passed as environment variables (see https://github.com/harvester/harvester-installer/blob/55655090ec7e0c21ba0ff370b9e3982ef65626c1/pkg/config/templates/rancherd-10-harvester.yaml#L196). I suspect this can be dropped now that we're auto-generating a configmap which includes these defaults.
• harvester exposes an auto-disk-provision-paths setting, which is applied to the managed chart to configure autoprovisioning (again) via environment variables (see https://github.com/harvester/harvester/blob/master/pkg/controller/master/setting/auto_disk_provision.go). We should probably just drop this setting completely, although I'm not sure offhand what that would mean for upgrades.
• It might be nice to add a UI component to help configuring the NDM filter configmap somehow, but TBH I don't really know where this would go or what it should look like, so failing that we should document how to work with the configmap :-)

[Yu-Jack]

• harvester-installer adds labelFilter for COS_* and HARV_* which are passed as environment variables (see https://github.com/harvester/harvester-installer/blob/55655090ec7e0c21ba0ff370b9e3982ef65626c1/pkg/config/templates/rancherd-10-harvester.yaml#L196). I suspect this can be dropped now that we're auto-generating a configmap which includes these defaults.

Yeah, we can drop it.

• harvester exposes an auto-disk-provision-paths setting, which is applied to the managed chart to configure autoprovisioning (again) via environment variables (see https://github.com/harvester/harvester/blob/master/pkg/controller/master/setting/auto_disk_provision.go). We should probably just drop this setting completely, although I'm not sure offhand what that would mean for upgrades.

Oh, I didn't know this before. I think we might need a new documentation and put it next by auto-disk-provision-paths, and recommend using new config map. Also, I believe some people are using it. I think we might need a migration for that. Like, when we start node disk manager, we'll need to fetch value from that setting and transfer it to config map. WDYT? If needed, I can do this in next PR for better review. cc @Vicente-Cheng

• It might be nice to add a UI component to help configuring the NDM filter configmap somehow, but TBH I don't really know where this would go or what it should look like, so failing that we should document how to work with the configmap :-)

Yeah, we'll need this documentation.

[tserong]

Oh, I didn't know this before. I think we might need a new documentation and put it next by auto-disk-provision-paths, and recommend using new config map. Also, I believe some people are using it. I think we might need a migration for that. Like, when we start node disk manager, we'll need to fetch value from that setting and transfer it to config map. WDYT? If needed, I can do this in next PR for better review.

I tested it, and that setting actually still works with this new implementation, because it's applied via environment variables. So existing users with that setting should have no trouble. That said, I didn't test applying that setting and also applying different autoprovision options via the configmap, so I'm not 100% sure how the two would interact. I think it's fine to look at any further changes here in a separate PR though.

[Yu-Jack]

I tested it, and that setting actually still works with this new implementation, because it's applied via environment variables. So existing users with that setting should have no trouble. That said, I didn't test applying that setting and also applying different autoprovision options via the configmap, so I'm not 100% sure how the two would interact. I think it's fine to look at any further changes here in a separate PR though.

Yeah, I found the code. I just suddenly forgot my logic ...

• Find yaml first.
• If yaml is empty, find env instead.

So, yes, it works with current default setting because autoprovision.yaml in config map is empty when we create it. I'll merge this PR. Then, write a new documentation and put it next to the description or title.