OCPBUGS-114375: azure: skip AppendVarPartition when DiskSetup provides a user-defined /var mount

[mfbonfigli]

Summary

On Azure, the installer unconditionally creates a /var partition on the boot disk. When a user configures a UserDefined DiskSetup entry with mountPath: /var, this collides with the auto-generated partition, causing ignition to fail with conflicting partition.

This PR adds a guard in Master.Generate() that checks whether any control plane DiskSetup entry of type UserDefined already claims /var as its mount path. If so, AppendVarPartition is skipped.

Changes

• pkg/asset/ignition/machine/master.go: Check ControlPlane.DiskSetup for a UserDefined entry with MountPath == "/var" before calling AppendVarPartition
• pkg/asset/ignition/machine/master_test.go: Add 3 test cases covering: Azure without DiskSetup (appends /var), Azure with /var UserDefined DiskSetup (skips), Azure with non-/var DiskSetup like etcd (still appends). Also migrated deprecated k8s.io/utils/pointer → k8s.io/utils/ptr

Summary by CodeRabbit

• Bug Fixes
  • Improved Azure control-plane disk configuration to respect user-defined /var mounts.
  • Automatically adds a /var partition when no user-managed /var disk is configured.
  • Preserves automatic /var setup when only other disks, such as an etcd disk, are defined.

[openshift-merge-bot]

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-robot]

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

• expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "4.19.0" 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:

Summary

On Azure, the installer unconditionally creates a /var partition on the boot disk. When a user configures a UserDefined DiskSetup entry with mountPath: /var, this collides with the auto-generated partition, causing ignition to fail with conflicting partition.

This PR adds a guard in Master.Generate() that checks whether any control plane DiskSetup entry of type UserDefined already claims /var as its mount path. If so, AppendVarPartition is skipped.

Changes

• pkg/asset/ignition/machine/master.go: Check ControlPlane.DiskSetup for a UserDefined entry with MountPath == "/var" before calling AppendVarPartition
• pkg/asset/ignition/machine/master_test.go: Add 3 test cases covering: Azure without DiskSetup (appends /var), Azure with /var UserDefined DiskSetup (skips), Azure with non-/var DiskSetup like etcd (still appends). Also migrated deprecated k8s.io/utils/pointer → k8s.io/utils/ptr

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.

[coderabbitai]

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: Pro Plus

Run ID: 64280b25-782d-4b88-a10d-f7d04786b96a

📥 Commits

Reviewing files that changed from the base of the PR and between 673c5fc and 7875723.

📒 Files selected for processing (2)

• pkg/asset/ignition/machine/master.go
• pkg/asset/ignition/machine/master_test.go

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

---

📝 Walkthrough

Walkthrough

Azure master ignition generation now avoids adding a duplicate /var partition when control-plane disk setup defines a user-managed /var mount. Tests cover automatic creation, suppression, and unrelated etcd disk configuration.

Changes

Azure /var partition handling

Layer / File(s)	Summary
Conditional Azure /var partition generation pkg/asset/ignition/machine/master.go	Azure generation checks user-defined control-plane disk entries and adds the /var partition only when no user-defined /var mount exists.
Azure /var partition test coverage pkg/asset/ignition/machine/master_test.go	Tests verify automatic /var creation, suppression for a user-defined /var disk, and retention when only an etcd disk is defined. The tests also use k8s.io/utils/ptr and a helper to inspect generated storage.

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

Merge Risk: ⚪ Minimal · up to 78757

This localized change prevents an automatically generated /var partition from conflicting with a user-defined /var mount while preserving existing behavior for other configurations; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 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 identifies the Azure ignition change and the condition that suppresses automatic /var partition creation. It is specific and related to the main change.
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	PASS: The changed tests use Go testing with static t.Run names, not Ginkgo title functions. The three titles are fixed descriptive strings and contain no pod names, timestamps, UUIDs, node names, …
Test Structure And Quality	✅ Passed	PASS: The changed tests use Go's testing package with t.Run and testify/assert; they do not use Ginkgo constructs. Each subtest checks one Azure /var partition behavior. The tests perform only…
Microshift Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go unit test, TestMasterGenerateAzureVarPartition, using testing.T and t.Run. It does not add Ginkgo e2e tests, reference MicroShift-unavailable OpenShift …
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go unit test, TestMasterGenerateAzureVarPartition, with t.Run subtests. It does not add Ginkgo It, Describe, Context, or When e2e tests. The test con…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The pull request changes only Azure master Ignition partition generation and its tests. The diff adds no deployment manifests, operators, controllers, replica logic, affinity, topology spread co…
Ote Binary Stdout Contract	✅ Passed	PASS: The pull request changes only pkg/asset/ignition/machine/master.go and its unit test. The changed code is Master.Generate(), not main(), init(), TestMain(), a Ginkgo suite hook, or `Ru…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go testing unit test, not a Ginkgo e2e test. The test only builds ignition configuration fixtures and checks generated partitions. It does not create cluster r…
No-Weak-Crypto	✅ Passed	PASS. The pull request adds only Azure /var DiskSetup detection and related tests, plus the k8s.io/utils/ptr import migration. The changed lines introduce no MD5, SHA1, DES, 3DES, RC4, Blowfish, E…
Container-Privileges	✅ Passed	PASS: The pull request changes only Go source and tests for Azure Ignition /var partition handling. The diff introduces no container or Kubernetes manifest fields for privileged, hostPID, `hostN…
No-Sensitive-Data-In-Logs	✅ Passed	PASS: The pull request adds only a static logrus.Debugf message: Adding /var partition to skip CoreOS growfs step. It passes no disk IDs, mount paths from configuration, credentials, tokens, hostn…

Full details: Stable And Deterministic Test Names

Explanation

PASS: The changed tests use Go testing with static t.Run names, not Ginkgo title functions. The three titles are fixed descriptive strings and contain no pod names, timestamps, UUIDs, node names, namespaces, IP addresses, or generated identifiers. TestMasterGenerateAzureVarPartition is also static.

Full details: Test Structure And Quality

Explanation

PASS: The changed tests use Go's testing package with t.Run and testify/assert; they do not use Ginkgo constructs. Each subtest checks one Azure /var partition behavior. The tests perform only in-memory asset generation, create no cluster resources, and use no wait operations requiring timeouts. Therefore, this Ginkgo-specific check is not applicable, and no stated failure condition is introduced.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request adds a standard Go unit test, TestMasterGenerateAzureVarPartition, using testing.T and t.Run. It does not add Ginkgo e2e tests, reference MicroShift-unavailable OpenShift APIs or namespaces, or make unsupported cluster assumptions.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request adds a standard Go unit test, TestMasterGenerateAzureVarPartition, with t.Run subtests. It does not add Ginkgo It, Describe, Context, or When e2e tests. The test constructs Azure install-config objects and checks generated Ignition partitions; it makes no multi-node or HA assumptions.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only Azure master Ignition partition generation and its tests. The diff adds no deployment manifests, operators, controllers, replica logic, affinity, topology spread constraints, node selectors, tolerations, or PDBs. It introduces no topology scheduling constraint covered by this check.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only pkg/asset/ignition/machine/master.go and its unit test. The changed code is Master.Generate(), not main(), init(), TestMain(), a Ginkgo suite hook, or RunSpecs() setup. The diff adds no stdout write, klog call, Ginkgo configuration, or log.SetOutput change. The repository contains no OTE openshift-tests binary or suite entry point affected by this pull request.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS: The pull request adds a standard Go testing unit test, not a Ginkgo e2e test. The test only builds ignition configuration fixtures and checks generated partitions. It does not create cluster resources, parse runtime IPs, contact external hosts, pull images, or use external URLs. The 10.0.1.0/24 values are local input fixtures and do not create an IPv4 networking dependency for this check.

Full details: No-Weak-Crypto

Explanation

PASS. The pull request adds only Azure /var DiskSetup detection and related tests, plus the k8s.io/utils/ptr import migration. The changed lines introduce no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token comparisons.

Full details: Container-Privileges

Explanation

PASS: The pull request changes only Go source and tests for Azure Ignition /var partition handling. The diff introduces no container or Kubernetes manifest fields for privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation. The changed behavior only conditionally calls AppendVarPartition, which adds storage and mount configuration, not container privileges.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The pull request adds only a static logrus.Debugf message: Adding /var partition to skip CoreOS growfs step. It passes no disk IDs, mount paths from configuration, credentials, tokens, hostnames, or personal data to logging. The new test data is synthetic (eastus, test.com, var-disk, and etcd-disk). No explicit sensitive-data logging condition is introduced.

• 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.12.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.}

[jcpowermac]

/lgtm

[openshift-merge-bot]

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

[mfbonfigli]

/jira refresh

[openshift-ci-robot]

@mfbonfigli: This pull request references Jira Issue OCPBUGS-114375, which is valid. The bug has been moved to the POST state.

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 New, which is one of the valid states (NEW, ASSIGNED, POST)

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.

[tthvo]

/test verify-vendor

[tthvo]

/approve

[openshift-ci]

[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

[mfbonfigli]

/test e2e-azure-ovn
/test e2e-azure-ovn-multidisk-techpreview

[mfbonfigli]

/test okd-scos-images

[openshift-ci]

@mfbonfigli: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn	7875723	link	true	/test e2e-aws-ovn

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.