OCPBUGS-61892: fix swap disk nil-panic and block swap on control plane by mfbonfigli · Pull Request #10859 · openshift/installer

mfbonfigli · 2026-09-08T15:58:58Z

Summary

Fix nil-pointer dereference in swap disk setup where diskSetup.Etcd.PlatformDiskID was incorrectly referenced instead of diskSetup.Swap.PlatformDiskID in both master.go and worker.go
Add validation to reject swap disks on control plane nodes — the MCO hardcodes failSwapOn: true for masters (kubelet.yaml) and actively blocks KubeletConfig CRs that attempt to override it, so swap cannot function on control plane

Summary by CodeRabbit

Bug Fixes
- Corrected swap disk identification during multi-disk setup for worker and control plane machines, ensuring the configured swap disk is used.
- Swap disk configuration on control plane nodes is now rejected with a clear validation error.
- Improved validation handling for unsupported disk setup types, providing consistent error reporting during machine configuration.

openshift-merge-bot · 2026-09-08T15:59:01Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci · 2026-09-08T15:59:05Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-09-08T15:59:06Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: 21133497-97bd-4a88-b6be-998b46d569e6

📥 Commits

Reviewing files that changed from the base of the PR and between 3fc6df4 and a0e52b4.

📒 Files selected for processing (5)

pkg/asset/machines/master.go
pkg/asset/machines/util.go
pkg/asset/machines/worker.go
pkg/types/validation/machinepools.go
pkg/types/validation/machinepools_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

pkg/types/validation/machinepools_test.go
pkg/types/validation/machinepools.go

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

Swap disk generation now uses the configured swap platform disk ID for master and worker machines. Validation rejects swap disk configuration in master machine pools and tests the rejection.

Changes

Swap disk handling

Layer / File(s)	Summary
Centralize disk ID resolution `pkg/asset/machines/util.go`, `pkg/asset/machines/master.go`, `pkg/asset/machines/worker.go`	`DiskName` resolves platform disk IDs for supported disk types. Master and worker ignition generation use the helper and propagate unsupported-type errors.
Reject master swap disks `pkg/types/validation/machinepools.go`, `pkg/types/validation/machinepools_test.go`	Validation rejects swap disks in `master` machine pools. A test verifies the expected validation error.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to a0e52

Swap disk generation now resolves the configured swap disk ID, and control-plane swap configurations are rejected. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both primary changes: fixing the swap disk nil-pointer issue and blocking swap disks on control plane nodes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The pull request adds one table-test title: `invalid swap disk on master machine pool`. The title is a static descriptive string. The test uses Go `t.Run(tc.name)` and does not interpolate pod names, …
Test Structure And Quality	✅ Passed	PASS. The PR adds one table-driven `t.Run` case in `pkg/types/validation/machinepools_test.go`; it checks only master-pool swap rejection. The test uses in-memory structs and creates no cluster resour…
Microshift Test Compatibility	✅ Passed	PASS. The pull request adds no Ginkgo e2e tests. The only added test is a standard `TestValidateMachinePool` case in `pkg/types/validation/machinepools_test.go`. The added code does not reference Micr…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e test was added. The only added test case is a standard Go subtest inside `TestValidateMachinePool` and validates swap rejection for a master machine pool. It contains no multi-node o…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS. The pull request changes only disk-ID resolution, ignition disk setup, and machine-pool validation. The exact diff modifies five Go files and introduces no Deployment, controller, pod, replica, …
Ote Binary Stdout Contract	✅ Passed	PASS — The pull request changes only disk-name resolution and machine-pool validation. The diff adds no `fmt.Print*`, `log`/`klog` output, `os.Stdout` writes, suite setup, `TestMain`, `RunSpecs`, `Bef…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go table-test case in `pkg/types/validation/machinepools_test.go` using `testing.T`. It does not add a Ginkgo e2e test. The test only validates disk configuratio…
No-Weak-Crypto	✅ Passed	The pull request adds disk-ID lookup and swap validation only. The exact HEAD^..HEAD diff introduces no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret-comparison code. The existing…
Container-Privileges	✅ Passed	PASS. The pull request changes only five Go source/test files and adds no YAML, JSON, Dockerfile, or Containerfile manifest. The added lines contain none of `privileged: true`, `hostPID`, `hostNetwork…
No-Sensitive-Data-In-Logs	✅ Passed	PASS. The pull request adds no logging calls and does not add passwords, tokens, API keys, PII, session IDs, hostnames, or customer data to log messages. The changed code only adds disk-ID selection, …

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.13.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

_{Comment @coderabbitai help to get the list of available commands.}

openshift-ci-robot · 2026-09-08T15:59:38Z

@mfbonfigli: This pull request references Jira Issue OCPBUGS-61892, which is invalid:

expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "4.21" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Fix copy-paste error where swap disk setup incorrectly referenced etcd's PlatformDiskID, causing a nil-pointer dereference when a swap disk is configured.

Add validation to reject swap disks on control plane nodes. The MCO hardcodes failSwapOn=true for masters and blocks KubeletConfig CRs that attempt to override it, so swap cannot function on control plane.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2026-09-08T16:11:15Z

@mfbonfigli: This pull request references Jira Issue OCPBUGS-61892, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.1.0) matches configured target version for branch (5.1.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jinyunma

Details

In response to this:

Summary

Fix nil-pointer dereference in swap disk setup where diskSetup.Etcd.PlatformDiskID was incorrectly referenced instead of diskSetup.Swap.PlatformDiskID in both master.go and worker.go

Add validation to reject swap disks on control plane nodes — the MCO hardcodes failSwapOn: true for masters (kubelet.yaml) and actively blocks KubeletConfig CRs that attempt to override it, so swap cannot function on control plane

Swap on workers is unaffected — the MCO already sets failSwapOn: false for workers (kubelet.yaml)

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-09-08T16:12:15Z

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: jinyunma.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@mfbonfigli: This pull request references Jira Issue OCPBUGS-61892, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)

bug target version (5.1.0) matches configured target version for branch (5.1.0)

bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jinyunma

In response to this:

Summary

Fix nil-pointer dereference in swap disk setup where diskSetup.Etcd.PlatformDiskID was incorrectly referenced instead of diskSetup.Swap.PlatformDiskID in both master.go and worker.go

Add validation to reject swap disks on control plane nodes — the MCO hardcodes failSwapOn: true for masters (kubelet.yaml) and actively blocks KubeletConfig CRs that attempt to override it, so swap cannot function on control plane

Swap on workers is unaffected — the MCO already sets failSwapOn: false for workers (kubelet.yaml)

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 openshift-eng/jira-lifecycle-plugin repository.

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.

mfbonfigli · 2026-09-08T16:12:48Z

/jira refresh

openshift-ci-robot · 2026-09-08T16:12:52Z

@mfbonfigli: This pull request references Jira Issue OCPBUGS-61892, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.1.0) matches configured target version for branch (5.1.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jinyunma

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-09-08T16:13:38Z

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: jinyunma.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@mfbonfigli: This pull request references Jira Issue OCPBUGS-61892, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)

bug target version (5.1.0) matches configured target version for branch (5.1.0)

bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jinyunma

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

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.

Fix copy-paste error where swap disk setup incorrectly referenced etcd's PlatformDiskID, causing a nil-pointer dereference when a swap disk is configured. Extract DiskName helper to util.go to prevent future drift between master.go and worker.go. Add validation to reject swap disks on control plane nodes. The MCO hardcodes failSwapOn=true for masters and blocks KubeletConfig CRs that attempt to override it, so swap cannot function on control plane. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai · 2026-09-09T11:39:34Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

vr4manta · 2026-09-09T11:41:42Z

/test ?

vr4manta · 2026-09-09T11:43:27Z

/test e2e-vsphere-ovn-disk-setup-techpreview

vr4manta · 2026-09-09T12:07:11Z

/lgtm

Started e2e-vsphere-ovn-disk-setup-techpreview to verify how it works against vsphere

openshift-merge-bot · 2026-09-09T12:07:26Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn

mfbonfigli · 2026-09-09T13:46:02Z

/test e2e-azure-ovn

tthvo

/approve

openshift-ci · 2026-09-09T20:43:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tthvo

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tthvo]

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

tthvo · 2026-09-09T20:44:21Z

/test e2e-aws-ovn

CI is under heavy load as I'm seeing some The maximum number of load balancers has been reached 😅

openshift-ci · 2026-09-10T00:30:28Z

@mfbonfigli: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

mfbonfigli · 2026-09-10T17:22:16Z

/verified by CI

openshift-ci-robot · 2026-09-10T17:22:28Z

@mfbonfigli: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2026-09-10T17:25:43Z

@mfbonfigli: Jira Issue Verification Checks: Jira Issue OCPBUGS-61892
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-61892 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

Summary

Fix nil-pointer dereference in swap disk setup where diskSetup.Etcd.PlatformDiskID was incorrectly referenced instead of diskSetup.Swap.PlatformDiskID in both master.go and worker.go

Add validation to reject swap disks on control plane nodes — the MCO hardcodes failSwapOn: true for masters (kubelet.yaml) and actively blocks KubeletConfig CRs that attempt to override it, so swap cannot function on control plane

Summary by CodeRabbit

Bug Fixes

Corrected swap disk identification during multi-disk setup for worker and control plane machines, ensuring the configured swap disk is used.

Swap disk configuration on control plane nodes is now rejected with a clear validation error.

Improved validation handling for unsupported disk setup types, providing consistent error reporting during machine configuration.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-merge-robot · 2026-09-11T13:43:02Z

Fix included in release 5.1.0-0.nightly-2026-09-11-071353

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2026

mfbonfigli changed the title ~~multi-disk: fix swap disk nil-panic and block swap on control plane~~ OCPBUGS-61892: fix swap disk nil-panic and block swap on control plane Sep 8, 2026

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 8, 2026

mfbonfigli marked this pull request as ready for review September 8, 2026 16:14

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2026

openshift-ci Bot requested review from jhixson74 and sadasu September 8, 2026 16:18

mfbonfigli force-pushed the OCPBUGS-61892_azure_swap_data_disk branch from ec1feb9 to a0e52b4 Compare September 9, 2026 11:39

openshift-ci Bot assigned vr4manta Sep 9, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2026

tthvo reviewed Sep 9, 2026

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 10, 2026

openshift-merge-bot Bot merged commit 43ef4b6 into openshift:main Sep 10, 2026
17 checks passed

Conversation

mfbonfigli commented Sep 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Sep 8, 2026

Uh oh!

openshift-ci Bot commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

openshift-ci-robot commented Sep 8, 2026

Uh oh!

openshift-ci-robot commented Sep 8, 2026

Summary

Uh oh!

openshift-ci Bot commented Sep 8, 2026

Summary

Uh oh!

mfbonfigli commented Sep 8, 2026

Uh oh!

openshift-ci-robot commented Sep 8, 2026

Uh oh!

openshift-ci Bot commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 9, 2026

Uh oh!

vr4manta commented Sep 9, 2026

Uh oh!

vr4manta commented Sep 9, 2026

Uh oh!

vr4manta commented Sep 9, 2026

Uh oh!

openshift-merge-bot Bot commented Sep 9, 2026

Uh oh!

mfbonfigli commented Sep 9, 2026

Uh oh!

tthvo left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Sep 9, 2026

Uh oh!

tthvo commented Sep 9, 2026

Uh oh!

openshift-ci Bot commented Sep 10, 2026

Uh oh!

mfbonfigli commented Sep 10, 2026

Uh oh!

openshift-ci-robot commented Sep 10, 2026

Uh oh!

Uh oh!

openshift-ci-robot commented Sep 10, 2026

Summary

Summary by CodeRabbit

Uh oh!

openshift-merge-robot commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mfbonfigli commented Sep 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading