:sparkles: Add BareMetalSwitch CRD and controller for switch config generation by alegacy · Pull Request #3046 · metal3-io/baremetal-operator

alegacy · 2026-03-03T13:22:57Z

Introduces the BareMetalSwitch (BMS) CRD for defining Top-of-Rack switches managed by the Ironic Networking Service. The controller watches BMS resources and credential secrets, then generates two output secrets:

Switch configs secret (IRONIC_SWITCH_CONFIGS_SECRET): a single INI-format file with per-switch sections including address, credentials, driver/device type, and optional fields.
Switch credentials secret (IRONIC_SWITCH_CREDENTIALS_SECRET): SSH private key files (keyed by .key) for switches using publickey authentication. Password-authenticated switches store credentials inline in the INI config instead.

The controller is only registered when IRONIC_NETWORKING_ENABLED=true and requires IRONIC_SWITCH_CONFIGS_SECRET, IRONIC_SWITCH_CREDENTIALS_SECRET, and IRONIC_SWITCH_CREDENTIALS_PATH to be set at startup.

Credential secret changes (e.g. password rotation, key replacement) trigger config regeneration via a secret watch with a mapper that filters to only secrets referenced by BMS resources.

What this PR does / why we need it:

Partially implements: metal3-io/metal3-docs#586

Checklist:

Documentation has been updated, if necessary.
note: api docs updated here, but metal3-docs repo will be updated once feature is complete
Unit tests have been added, if necessary.
E2E tests have been added, if necessary.
Integration tests have been added, if necessary.

metal3-io-bot · 2026-03-03T13:23:07Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kashifest for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

metal3-io-bot · 2026-03-03T13:23:11Z

Hi @alegacy. Thanks for your PR.

I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

s3rj1k · 2026-03-03T14:13:08Z

I am starting to wonder if all of this networking should be moved to a dedicated controller (like we have for IPAM) instead of gluing it into BMO.

alegacy · 2026-03-16T18:37:16Z

/ok-to-test

dtantsur · 2026-03-20T17:00:12Z

@@ -1 +1,5 @@
 IRONIC_ENDPOINT=https://192.168.222.2:6385/v1/
+IRONIC_NETWORKING_ENABLED=true


Follow-up request: BMO is capable of reading an IrSO Ironic object to populate its configuration. Ideally, we should support the switch configuration in this way as well (once the IrSO change lands, of course).

alegacy · 2026-04-17T18:09:01Z

/retest

Copilot

Pull request overview

Adds a new BareMetalSwitch API and controller to generate Ironic Networking switch configuration + SSH key material from BareMetalSwitch resources and referenced credential Secrets, and wires it into manifests and E2E coverage.

Changes:

Introduces BareMetalSwitch CRD/API types (with status conditions) and registers it in rendered/base manifests and RBAC.
Adds a BareMetalSwitchReconciler plus config/secret generation helpers and unit tests.
Adds E2E test suite + overlay/config updates to enable the controller and run the new scenario.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`apis/metal3.io/v1alpha1/baremetalswitch_types.go`	New API type for `BareMetalSwitch`, credentials ref, and status conditions.
`apis/metal3.io/v1alpha1/zz_generated.deepcopy.go`	Generated deepcopy implementations for new API types.
`internal/controller/metal3.io/baremetalswitch_controller.go`	New controller reconcile loop and manager registration.
`internal/controller/metal3.io/baremetalswitch_config.go`	Switch INI config + credentials-secret generation and secret update helpers.
`internal/controller/metal3.io/baremetalswitch_config_test.go`	Unit tests for config generation and controller condition behavior.
`main.go`	Conditionally registers the BareMetalSwitch controller based on env vars.
`config/base/crds/kustomization.yaml`	Adds the new CRD into the base CRD kustomization.
`config/base/crds/bases/metal3.io_baremetalswitches.yaml`	New CRD manifest for BareMetalSwitch.
`config/base/rbac/role.yaml`	Grants manager permissions for baremetalswitches and status.
`config/render/capm3.yaml`	Rendered CRD + RBAC updates for BareMetalSwitch in CAPM3 render output.
`config/samples/baremetalswitch_sample.yaml`	Sample BareMetalSwitch + credentials Secret manifest.
`docs/api.md`	Adds API documentation section for BareMetalSwitch.
`config/overlays/fixture/kustomization.yaml`	Enables Ironic networking env vars in fixture overlay.
`config/overlays/e2e/kustomization.yaml`	Adds the baremetalswitch overlay to the E2E stack.
`config/overlays/e2e/baremetalswitch/kustomization.yaml`	New E2E overlay to create namespace + reuse roles/rolebindings.
`config/overlays/e2e/baremetalswitch/namespace.yaml`	Namespace manifest for baremetalswitch E2E tests.
`config/overlays/e2e/ironic.env`	Enables networking + switch secret env vars for E2E runs.
`config/overlays/e2e/namespaced-manager-patch.yaml`	Adds `baremetalswitch` to WATCH_NAMESPACE list for namespaced E2E runs.
`config/overlays/e2e/roles-rolebindings/roles-rolebindings.yaml`	Grants E2E roles access to baremetalswitches and status.
`test/e2e/baremetalswitch_test.go`	New E2E test validating config generation, credential types, and rotation.
`test/e2e/config/fixture.yaml`	Adds interval config for baremetalswitch secret update waits.
`test/e2e/config/ironic.yaml`	Adds interval config for baremetalswitch secret update waits.

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

tuminoid

Generally LGTM, asking some clarifying questions.

tuminoid · 2026-04-21T07:08:32Z

+	}
+
+	// Update the switch credentials secret
+	if err := updateSecretData(ctx, c, sm, namespace, credentialSecretName, result.keyFiles); err != nil {


keyFiles only contains keys that were successfully reconciled this cycle. So if there is some (transient) error in switch A, then those keys will be dropped until next reconcile and access is lost meanwhile. That intentional?

Yes, that was my intent. I figured it would be better to have some data written to the config secret rather than no data to the config secret (i.e., 1 bad switch config shouldn't block all of the good ones). I can change that if there is consensus, but I do think this is the best approach.

I have no strong preference. @Rozzii @dtantsur maybe more hands-on with this feature?

tuminoid · 2026-04-21T07:10:52Z

+		// User error: do not return an error to avoid exponential retries.
+		// Requeue after a delay so that if the credential secret is created
+		// later we will eventually pick it up (the Owns() watch cannot detect
+		// creation of a secret that has no owner reference yet and no label).
+		logger.Info("BareMetalSwitch has credential error, requeueing", "error", credErr)
+		return ctrl.Result{RequeueAfter: credentialErrorRequeueDelay}, nil
+	}


It has a comment, but 10s requeue means for user misconfugration we run reconcile 8640 times a day (360 times an hour). Do we REALLY need 10s requeues?

@tuminoid I modeled this after the BMH handling of BMC secrets. It also requeues every 10 secs (hostErrorRetryDelay) when the BMC secret cannot be found. The difference being that I treated all errors the same... I'll change this so that a missing secret is requeued while some sort of user error on processing the secret is not.

Given that the BMH requeue timer is also 10 seconds should I leave it as-is or do you want me to back it off? I was aiming for consistency, but do agree that 10 seconds is too fast. 60secs? 5mins?

@dtantsur any opinions ?

Introduces the BareMetalSwitch (BMS) CRD for defining Top-of-Rack switches managed by the Ironic Networking Service. The controller watches BMS resources and credential secrets, then generates two output secrets: 1. Switch configs secret (IRONIC_SWITCH_CONFIGS_SECRET): a single INI-format file with per-switch sections including address, credentials, driver/device type, and optional fields. 2. Switch credentials secret (IRONIC_SWITCH_CREDENTIALS_SECRET): SSH private key files (keyed by <mac-address>.key) for switches using publickey authentication. Password-authenticated switches store credentials inline in the INI config instead. The controller is only registered when IRONIC_NETWORKING_ENABLED=true and requires IRONIC_SWITCH_CONFIGS_SECRET, IRONIC_SWITCH_CREDENTIALS_SECRET, and IRONIC_SWITCH_CREDENTIALS_PATH to be set at startup. Credential secret changes (e.g. password rotation, key replacement) trigger config regeneration via a secret watch with a mapper that filters to only secrets referenced by BMS resources. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Allain Legacy <alegacy@redhat.com>

alegacy · 2026-04-22T13:30:06Z

@@ -1 +1,5 @@
 IRONIC_ENDPOINT=https://192.168.222.2:6385/v1/
+IRONIC_NETWORKING_ENABLED=true


@dtantsur @tuminoid basic question about the e2e tests... i started adding other tests for my other PR that will come in the future. In doing so I've been wondering if the way I did it in this PR is the right approach.

I've basically set IRONIC_NETWORKING_ENABLED=true always so the e2e tests always run this way regardless of whether the test is networking related or not. Should I split this out into its own overlay and not disturb the existing e2e tests with this variable addition?

On the other hand, it would be good for the networking code to be exercised in some of those other scenarios too.

Not sure.

Sorry, gave this some more thought today... I guess there wouldn't be much different in the code execution if there were no BareMetalSwitch CRs, and in the future no HostNetworkAttachment CRs or references to them in BMH.spec.networkInterfaces.

...so maybe I'm overthinking this a bit and always having IRONIC_NETWORKING_ENABLED=true isn't problematic.

metal3-io-bot requested review from dtantsur, s3rj1k and zhouhao3 March 3, 2026 13:23

metal3-io-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 3, 2026

metal3-io-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 3, 2026

metal3-io-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 16, 2026

alegacy force-pushed the feature/switch-management branch 2 times, most recently from f2993a8 to b04e473 Compare March 16, 2026 19:09

MahnoorAsghar reviewed Mar 20, 2026

View reviewed changes

Comment thread apis/metal3.io/v1alpha1/baremetalswitch_types.go Outdated