Skip to content

butane/util: emit large MachineConfig integers as decimals - #2312

Closed
butcher-pudge-0 wants to merge 2 commits into
coreos:mainfrom
butcher-pudge-0:cursor/mc-yaml-plain-integers
Closed

butcher-pudge-0 wants to merge 2 commits into
coreos:mainfrom
butcher-pudge-0:cursor/mc-yaml-plain-integers

Conversation

@butcher-pudge-0

Copy link
Copy Markdown

Summary

  • Fix MachineConfig YAML encoding of large integers such as sizeMiB/startMiB so values >= 1e6 are written as decimals (8389000) instead of scientific notation (8.389e+06).
  • Preserve JSON numbers as int64 before YAML encoding in TranslateBytesYAML. json.Unmarshal into interface{} otherwise uses float64, and yaml.v3 then formats those values with scientific notation. MCO/Ignition cannot unmarshal that into *int.
  • Add unit and OpenShift 4.18 regression tests covering the GitHub butane/MCO: MiB fileds are being in scientific notation (1e6, instead of 1000000) #2309 and OCPBUGS-114735 reproductions.

Fixes #2309
Fixes: https://redhat.atlassian.net/browse/OCPBUGS-114735

This is an alternative to #2310: that change YAML-unmarshals the intermediate JSON (and is still missing tests and a release note). This PR keeps a JSON decoder with UseNumber() so integers stay integers, and includes tests plus a release-note entry.

Test plan

Made with Cursor

Super User and others added 2 commits August 29, 2026 19:31
Preserve JSON numbers as int64 before YAML encoding so sizeMiB and similar fields are not written in scientific notation.

Fixes coreos#2309

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 32a435e8-8c79-4eea-af04-658daad6149f

📥 Commits

Reviewing files that changed from the base of the PR and between ade39b4 and 03afb03.

📒 Files selected for processing (4)
  • butane/config/openshift/v4_18/translate_test.go
  • butane/config/util/util.go
  • butane/config/util/util_test.go
  • docs/release-notes.md

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

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
Documentation served via GitHub Pages/Jekyll. Every platform must be documented in supported-platforms.md. The ./test script validates doc consistency.

⚙️ CodeRabbit configuration file

Files:

  • docs/release-notes.md
- **Naming**: PascalCase exported, camelCase unexported, snake_case filenames

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • butane/config/openshift/v4_18/translate_test.go
  • butane/config/util/util_test.go
  • butane/config/util/util.go
- **Formatting**: `gofmt` enforced (CI and `./test`)

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • butane/config/openshift/v4_18/translate_test.go
  • butane/config/util/util_test.go
  • butane/config/util/util.go
🔇 Additional comments (4)
butane/config/util/util.go (1)

158-159: LGTM!

Also applies to: 177-218

butane/config/util/util_test.go (1)

18-20: LGTM!

Also applies to: 29-29, 125-143

butane/config/openshift/v4_18/translate_test.go (1)

516-551: LGTM!

docs/release-notes.md (1)

17-20: LGTM!


📝 Walkthrough

Walkthrough

Changes

MachineConfig YAML integer preservation

Layer / File(s) Summary
JSON number conversion and utility coverage
butane/config/util/util.go, butane/config/util/util_test.go
TranslateBytesYAML preserves JSON integer precision before YAML encoding. Tests cover large integers, decimal values, and scientific-notation exclusion.
OpenShift regression coverage and release note
butane/config/openshift/v4_18/translate_test.go, docs/release-notes.md
OpenShift translation tests verify large and zero sizeMiB values. The release note documents decimal encoding for large integer fields.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 03afb

This localized encoding change is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5 | ❌ 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 8 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the required subsystem-and-description format, uses imperative wording, starts the description with lowercase text, and accurately describes the main change.
Description check ✅ Passed The description clearly explains the integer-encoding fix, affected fields, tests, linked issues, and release-note change.
Linked Issues check ✅ Passed The implementation addresses issue #2309 by preserving large JSON integers as int64 so MachineConfig YAML emits decimal values instead of scientific notation. Regression tests cover the reported behav…
Out of Scope Changes check ✅ Passed The code changes, regression tests, and release note all support the linked issue and pull request objective. No unrelated changes are present.
Commit Message Convention ✅ Passed Both non-merge commits in the PR range conform. butane/util: emit large MachineConfig integers as decimals and butane: reference OCPBUGS-114735 in release notes and tests use valid subsystem prefi…
Full details: Linked Issues check

Explanation

The implementation addresses issue #2309 by preserving large JSON integers as int64 so MachineConfig YAML emits decimal values instead of scientific notation. Regression tests cover the reported behavior.

Full details: Docstring Coverage

Explanation

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 8 functions across 3 files. (1 skipped: 1 unsupported.)

Full details: Commit Message Convention

Explanation

Both non-merge commits in the PR range conform. butane/util: emit large MachineConfig integers as decimals and butane: reference OCPBUGS-114735 in release notes and tests use valid subsystem prefixes, start their descriptions with lowercase imperative verbs, and have no trailing periods. No merge commits require review.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@butcher-pudge-0

Copy link
Copy Markdown
Author

Closing — starting over from scratch.

@butcher-pudge-0
butcher-pudge-0 deleted the cursor/mc-yaml-plain-integers branch August 31, 2026 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

butane/MCO: MiB fileds are being in scientific notation (1e6, instead of 1000000)

1 participant