Repository navigation
OCP 4.22 MCO does not support Ignition spec 3.6.0 produced by Butane openshift 4.22.0 - #717
Conversation
There was a problem hiding this comment.
Code Review
This pull request rolls back the Ignition spec version from 3.6.0 to 3.5.0 for OpenShift 4.22.0. This change reverts the associated imports, function names, test assertions, and documentation to align with Ignition 3.5.0 and Butane FCOS 1.6 / Base 0.6. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
prestist
left a comment
There was a problem hiding this comment.
Thanks for working on this @yasminvalim! The rollback approach follows the established pattern (we've done this for 4.19, 4.17, 4.14, etc.) and the code changes are mechanically correct.
A couple things to address before merging:
- The commit messages need to follow the repo's : convention. I'd suggest squashing into a single commit with something like:
openshift/v4_22: roll back to Ignition spec 3.5.0
You can look at the 4.19.0 rollback (ca25c01) or the stabilization commits on main for the naming style.
- There's a doc issue with storage.trees -- see inline comment on butane.yaml.
Inline comment on internal/doc/butane.yaml, line 42 (the min: 4.23.0 change):
This change is correct (bumping the setuid "not supported" transform to start at 4.23), but I think there's a related issue with the trees description transform further down.
At line 286, the transform that strips the file_mode/dir_mode prose from the trees description has max: 4.20.0:
- variant: openshift max: 4.20.0
Since v4_22 (and v4_21) now both use base/v0_6 which doesn't have file_mode, dir_mode, user, or group on Tree, this should be max: 4.22.0 so the generated docs don't advertise fields that don't exist in the schema.
You can verify by looking at docs/config-openshift-v4_22.md line 140 -- it still says "Ownership, file modes (using file_mode) and directories modes (using dir_mode) can be specified for the tree" even though those fields were removed from the field listing. This looks like it was actually a pre-existing bug from v4_21 as well, but we should fix it here since this PR is what makes v4_22 affected.
Revert v4_22 to base/v0_6 (Ignition 3.5.0). Fix generated docs for storage.trees on openshift 4.21–4.22 and adjust setuid transform bounds for the 4.23 experimental spec.
|
@prestist thanks for your review! I squashed the changes in one commit and fixed the message following the repo convention. I also updated the max version and the docs. Feel free to take a look on that :) |
prestist
left a comment
There was a problem hiding this comment.
LGTM, thank you @yasminvalim! Commit message looks good and the trees doc fix for v4_21 is a nice bonus
| * **_cex_** (object): describes the IBM Crypto Express (CEX) card configuration for the luks device. | ||
| * **_enabled_** (boolean): whether or not to enable cex compatibility for luks. If omitted, defaults to false. | ||
| * **_trees_** (list of objects): a list of local directory trees to be embedded in the config. Symlinks must not be present. Ownership, file modes (using `file_mode`) and directories modes (using `dir_mode`) can be specified for the tree. If not specified, ownership is not preserved and file modes are set to 0755 if the local file is executable or 0644 otherwise. File attributes can be overridden by creating a corresponding entry in the `files` section; such entries must omit `contents`. | ||
| * **_trees_** (list of objects): a list of local directory trees to be embedded in the config. Symlinks must not be present. Ownership is not preserved. File modes are set to 0755 if the local file is executable or 0644 otherwise. File attributes can be overridden by creating a corresponding entry in the `files` section; such entries must omit `contents`. |
There was a problem hiding this comment.
Why do we need this change here? This should have been in a distinct commit with an explaination.
|
|
||
| ### Misc. changes | ||
|
|
||
| - Roll back to Ignition spec 3.5.0 _(openshift 4.22.0)_ |
There was a problem hiding this comment.
This is not a misc change, this would be a breaking change.
| max: 1.1.0 | ||
| - variant: openshift | ||
| max: 4.20.0 | ||
| max: 4.22.0 |
There was a problem hiding this comment.
Hum, that's a two version jump, so is this related to the docs change that we missed? This should have been in its own commit.
|
We should not squash unrelated changes into a single commit. We need the split into individual commits to understand what each change mean. |
|
This should reference:
|
|
This needs an entry in the upgrading docs to note the breaking change. |
|
@travier can I revert this and create a new PR with those changes? |
|
Yes please, make a PR that reverts this and re-splits it with fixes. |
|
Revert PR: #718 |
Revert "Merge pull request #717 from coreos/update-4.22"
As a result when butane stabilized for OCP 4.22 we got into a state where OCP 4.22 renders into ignition 3.6 which the MCO will not know. To work around this for 4.22 we need to do the following.