Skip to content

Correct partition index loop in butane/*/translate.go - #2304

Open
skewballfox wants to merge 1 commit into
coreos:mainfrom
skewballfox:translate-fix
Open

Correct partition index loop in butane/*/translate.go#2304
skewballfox wants to merge 1 commit into
coreos:mainfrom
skewballfox:translate-fix

Conversation

@skewballfox

Copy link
Copy Markdown

patching work from PR 722 for butane.

currently trying to override the default sizes for the boot partitions.

In my butain file, I have:

        - number: 1
          label: bios
          size_mib: 1
          type_guid: 21686148-6449-6E6F-744E-656564454649
        - number: 2
          label: efi 
          size_mib: 1024
          type_guid: C12A7328-F81F-11D2-BA4B-00A0C93EC93B
          wipe_partition_entry: true
        - number: 3
          label: boot
          size_mib: 2048
          type_guid: 0FC63DAF-8483-4772-8E79-3D69D8477DE4
          wipe_partition_entry: true
        - label: root
          number: 4
          wipe_partition_entry: true
          type_guid: 4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709

which should be valid. root is the last partition, and all the others have a set size. currently this throws:

     |                  ^^^^ root partition cannot expand; it is set to fill available space but is followed by an auto-positioned partition
 122 |           number: 4
 123 |           start_mib: 0
     |
Config produced warnings and --strict was specified

this shoud only check the partitions after root

@coderabbitai

coderabbitai Bot commented Aug 24, 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: Enterprise

Run ID: 1720cc0f-bfe1-48fa-89f3-a9521d2bf460

📥 Commits

Reviewing files that changed from the base of the PR and between 9e2b242 and 594667e.

📒 Files selected for processing (14)
  • butane/config/fcos/v1_3/translate.go
  • butane/config/fcos/v1_3/translate_test.go
  • butane/config/fcos/v1_4/translate.go
  • butane/config/fcos/v1_4/translate_test.go
  • butane/config/fcos/v1_5/translate.go
  • butane/config/fcos/v1_5/translate_test.go
  • butane/config/fcos/v1_6/translate.go
  • butane/config/fcos/v1_6/translate_test.go
  • butane/config/fcos/v1_7/translate.go
  • butane/config/fcos/v1_7/translate_test.go
  • butane/config/fcos/v1_8_exp/translate.go
  • butane/config/fcos/v1_8_exp/translate_test.go
  • butane/docs/release-notes.md
  • docs/release-notes.md

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

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (2)
docs/**

⚙️ CodeRabbit configuration file

docs/**: Documentation served via GitHub Pages/Jekyll. Every platform must be documented in supported-platforms.md. The ./test script validates doc consistency.

Files:

  • docs/release-notes.md
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Include the required Apache 2.0 license header at the top of every Go source file.
Use the project's import ordering in Go files: standard library imports, blank line, project packages, blank line, then external dependencies.
Follow the project's Go naming conventions: exported identifiers use PascalCase, unexported identifiers use camelCase, and filenames use snake_case.

Files:

  • butane/config/fcos/v1_8_exp/translate_test.go
  • butane/config/fcos/v1_6/translate_test.go
  • butane/config/fcos/v1_3/translate_test.go
  • butane/config/fcos/v1_7/translate_test.go
  • butane/config/fcos/v1_8_exp/translate.go
  • butane/config/fcos/v1_6/translate.go
  • butane/config/fcos/v1_5/translate.go
  • butane/config/fcos/v1_3/translate.go
  • butane/config/fcos/v1_4/translate_test.go
  • butane/config/fcos/v1_5/translate_test.go
  • butane/config/fcos/v1_4/translate.go
  • butane/config/fcos/v1_7/translate.go
🔇 Additional comments (14)
butane/config/fcos/v1_3/translate.go (1)

78-78: LGTM!

butane/config/fcos/v1_3/translate_test.go (1)

1655-1685: LGTM!

butane/config/fcos/v1_4/translate.go (1)

78-78: LGTM!

butane/config/fcos/v1_4/translate_test.go (1)

1655-1685: LGTM!

butane/config/fcos/v1_5/translate.go (1)

79-79: LGTM!

butane/config/fcos/v1_5/translate_test.go (1)

1737-1767: LGTM!

docs/release-notes.md (1)

22-22: LGTM!

butane/config/fcos/v1_6/translate.go (1)

79-79: LGTM!

butane/config/fcos/v1_6/translate_test.go (1)

1737-1767: LGTM!

butane/config/fcos/v1_7/translate.go (1)

79-79: LGTM!

butane/config/fcos/v1_7/translate_test.go (1)

1738-1768: LGTM!

butane/config/fcos/v1_8_exp/translate.go (1)

79-79: LGTM!

butane/config/fcos/v1_8_exp/translate_test.go (1)

1884-1914: LGTM!

butane/docs/release-notes.md (1)

24-24: LGTM!


📝 Walkthrough

Walkthrough

Root partition constraint checks across FCOS versions 1.3 through 1.8 experimental now inspect only partitions after the root partition. Regression tests cover unsized root partitions placed last, and release notes document the fix.

Changes

Root partition constraint handling

Layer / File(s) Summary
Constraint validation and regression tests
butane/config/fcos/v1_3/..., butane/config/fcos/v1_4/..., butane/config/fcos/v1_5/..., butane/config/fcos/v1_6/..., butane/config/fcos/v1_7/..., butane/config/fcos/v1_8_exp/...
Root partition validation skips preceding partitions and retains checks for following partitions. Tests cover final unsized root partitions with explicitly sized preceding partitions.
Release note updates
butane/docs/release-notes.md, docs/release-notes.md
Release notes document the root partition constraint fix, and the Butane release-notes section gains spacing.

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

Merge Risk: ⚪ Minimal · up to 59466

This change corrects partition-index validation so a final expandable root partition is handled correctly across supported configuration versions. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: prestist

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes the partition loop fix but does not follow the required subsystem: lowercase description format or imperative lowercase styling. Use a title such as "config/*: correct partition index loop" with a subsystem prefix and lowercase description.
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description explains the root partition validation issue, provides a reproduction, and matches the stated objectives.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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.
Commit Message Convention ✅ Passed The PR has one non-merge commit, and its subject uses a subsystem prefix, starts the description with imperative lowercase “fix,” and has no trailing period.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

Copy link
Copy Markdown

Binary size report (bin/amd64/ignition)

Size
Base (main) 33MiB
PR (#2304) 33MiB
Delta +64B (0.00%)

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.

1 participant