Skip to content

OCPBUGS-85509: Add omitempty to vSphere and Nutanix MachinePool slice fields#10551

Open
chdeshpa-hue wants to merge 1 commit into
openshift:mainfrom
chdeshpa-hue:OCPBUGS-85509-omitempty-datadisks
Open

OCPBUGS-85509: Add omitempty to vSphere and Nutanix MachinePool slice fields#10551
chdeshpa-hue wants to merge 1 commit into
openshift:mainfrom
chdeshpa-hue:OCPBUGS-85509-omitempty-datadisks

Conversation

@chdeshpa-hue
Copy link
Copy Markdown

@chdeshpa-hue chdeshpa-hue commented May 13, 2026

Summary

  • Add omitempty to optional slice fields (DataDisks, GPUs) in vSphere and Nutanix MachinePool types that were missing it
  • Aligns with Azure's DataDisks which already carries omitempty

Problem

When automation tools (Hive, assisted-service) vendor newer installer types and marshal an InstallConfig targeting an older cluster version (e.g., 4.18), the empty DataDisks slice is serialized as dataDisks: null. The older installer binary does not recognize this field, and UnmarshalStrict emits:

failed to parse first occurrence of unknown field: failed to unmarshal install-config.yaml:
error unmarshaling JSON: while decoding JSON: json: unknown field "dataDisks"

The install succeeds (the installer falls back to non-strict unmarshal when OPENSHIFT_INSTALL_INVOKER is set), but the warning creates customer-facing noise.

Fix

Add omitempty to 3 fields across 2 files:

File Field Before After
pkg/types/vsphere/machinepool.go DataDisks json:"dataDisks" json:"dataDisks,omitempty"
pkg/types/nutanix/machinepool.go DataDisks json:"dataDisks" json:"dataDisks,omitempty"
pkg/types/nutanix/machinepool.go GPUs json:"gpus" json:"gpus,omitempty"

This is safe because:

  • All fields are already annotated +optional
  • All consumers use len() > 0 guards before accessing elements
  • An omitted JSON field unmarshals to a nil slice (len() == 0), identical to the current empty-slice behavior
  • The MachinePool.Set() methods guard with len(required.DataDisks) > 0

Test plan

  • Verify existing installer unit tests pass (go test ./pkg/types/vsphere/... ./pkg/types/nutanix/...)
  • Verify marshaling a MachinePool with no DataDisks no longer emits the dataDisks key
  • Verify marshaling a MachinePool with explicit DataDisks still includes them

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Nutanix: DataDisks field is now properly marked as optional in machine pool configurations, allowing users to omit this field when not required.
    • vSphere: DataDisks field is now properly marked as optional in machine pool configurations, enabling more concise and flexible configuration files.

… fields

Add omitempty to optional slice fields (DataDisks, GPUs) in vSphere
and Nutanix MachinePool types. Without omitempty, marshaling a
zero-valued struct emits dataDisks: null / gpus: null, which older
installer payloads treat as unknown fields and emit warnings during
cluster provisioning via automation tools (Hive, assisted-service).

Azure's DataDisks already carries omitempty; this brings vSphere and
Nutanix into alignment.

Assisted-by: Codex 5.3
Co-authored-by: Cursor <cursoragent@cursor.com>
@openshift-ci-robot openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels May 13, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@chdeshpa-hue: This pull request references Jira Issue OCPBUGS-85509, which is invalid:

  • expected the bug to target the "5.0.0" version, but no target version was set

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

  • Add omitempty to optional slice fields (DataDisks, GPUs) in vSphere and Nutanix MachinePool types that were missing it
  • Aligns with Azure's DataDisks which already carries omitempty

Problem

When automation tools (Hive, assisted-service) vendor newer installer types and marshal an InstallConfig targeting an older cluster version (e.g., 4.18), the empty DataDisks slice is serialized as dataDisks: null. The older installer binary does not recognize this field, and UnmarshalStrict emits:

failed to parse first occurrence of unknown field: failed to unmarshal install-config.yaml:
error unmarshaling JSON: while decoding JSON: json: unknown field "dataDisks"

The install succeeds (the installer falls back to non-strict unmarshal when OPENSHIFT_INSTALL_INVOKER is set), but the warning creates customer-facing noise.

Fix

Add omitempty to 3 fields across 2 files:

File Field Before After
pkg/types/vsphere/machinepool.go DataDisks json:"dataDisks" json:"dataDisks,omitempty"
pkg/types/nutanix/machinepool.go DataDisks json:"dataDisks" json:"dataDisks,omitempty"
pkg/types/nutanix/machinepool.go GPUs json:"gpus" json:"gpus,omitempty"

This is safe because:

  • All fields are already annotated +optional
  • All consumers use len() > 0 guards before accessing elements
  • An omitted JSON field unmarshals to a nil slice (len() == 0), identical to the current empty-slice behavior
  • The MachinePool.Set() methods guard with len(required.DataDisks) > 0

Test plan

  • Verify existing installer unit tests pass (go test ./pkg/types/vsphere/... ./pkg/types/nutanix/...)
  • Verify marshaling a MachinePool with no DataDisks no longer emits the dataDisks key
  • Verify marshaling a MachinePool with explicit DataDisks still includes them

Made with Cursor

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 openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label May 13, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c0f4fdab-9463-45a3-bbfe-742e03b4b607

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc515d and ca9b95c.

📒 Files selected for processing (2)
  • pkg/types/nutanix/machinepool.go
  • pkg/types/vsphere/machinepool.go

Walkthrough

The PR makes DataDisks optional in Nutanix and vSphere machine pool types by adding omitempty JSON tags and explicitly marking the field optional via kubebuilder annotations in Nutanix.

Changes

Machine Pool DataDisks Optional

Layer / File(s) Summary
Schema updates for optional DataDisks
pkg/types/nutanix/machinepool.go, pkg/types/vsphere/machinepool.go
DataDisks field in both Nutanix and vSphere MachinePool types is marked optional by adding omitempty to JSON tags. Nutanix additionally adds the +optional kubebuilder annotation to explicitly denote the optional nature at the API level.

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 12
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding omitempty JSON tags to optional slice fields (DataDisks and GPUs) in vSphere and Nutanix MachinePool types.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 PR does not contain any Ginkgo tests. Modified test files use standard Go testing (testing.T), not Ginkgo framework. The check for stable Ginkgo test names is not applicable.
Test Structure And Quality ✅ Passed PR modifies only struct definitions (JSON tags) without adding or modifying Ginkgo tests. Existing tests use standard Go testing package, not Ginkgo. Check is not applicable.
Microshift Test Compatibility ✅ Passed This PR does not add any Ginkgo e2e tests. It only modifies type definition files to add omitempty JSON tags. The MicroShift test compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No Ginkgo e2e tests added. PR modifies only configuration type files, not test code.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only struct field JSON tags in MachinePool types. No deployment manifests, operator code, controllers, or scheduling constraints are introduced.
Ote Binary Stdout Contract ✅ Passed PR changes only affect struct field JSON tags in pkg/types/*machinepool.go files. No process-level code, logging config, or stdout writes are modified. The check is not applicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR modifies only type definition files, not test files. The custom check applies only when new Ginkgo e2e tests are added.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 13, 2026

Hi @chdeshpa-hue. Thanks for your PR.

I'm waiting for a openshift 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.

@openshift-ci openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 13, 2026
@openshift-ci openshift-ci Bot requested review from jcpowermac and rvanderp3 May 13, 2026 04:53
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 13, 2026

[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 rvanderp3 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:

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

@jcpowermac
Copy link
Copy Markdown
Contributor

/ok-to-test

@openshift-ci openshift-ci 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 May 13, 2026
@eggfoobar
Copy link
Copy Markdown
Contributor

/test shellcheck

Validating CI job shellcheck, ignore

@Prucek
Copy link
Copy Markdown
Member

Prucek commented May 14, 2026

/test ci/prow/shellcheck

@Prucek
Copy link
Copy Markdown
Member

Prucek commented May 14, 2026

/test shellcheck

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 14, 2026

@chdeshpa-hue: The following tests 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-vsphere-ovn-disk-setup-techpreview ca9b95c link false /test e2e-vsphere-ovn-disk-setup-techpreview
ci/prow/e2e-nutanix-ovn ca9b95c link false /test e2e-nutanix-ovn
ci/prow/e2e-vsphere-ovn ca9b95c link true /test e2e-vsphere-ovn
ci/prow/e2e-vsphere-multi-vcenter-ovn ca9b95c link false /test e2e-vsphere-multi-vcenter-ovn
ci/prow/e2e-vsphere-ovn-hybrid-env ca9b95c link false /test e2e-vsphere-ovn-hybrid-env
ci/prow/e2e-vsphere-ovn-techpreview ca9b95c link false /test e2e-vsphere-ovn-techpreview
ci/prow/e2e-vsphere-ovn-devpreview ca9b95c link false /test e2e-vsphere-ovn-devpreview

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants