Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mantle/platform/conf: transparently handle Butane spec stabilizations #3422

Merged
merged 3 commits into from
Apr 13, 2023
Merged

mantle/platform/conf: transparently handle Butane spec stabilizations #3422

merged 3 commits into from
Apr 13, 2023

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Apr 11, 2023

There are three stabilization cases we need to handle:

  1. Tests using an experimental Ignition spec that's being stabilized.
  2. Tests using an experimental Butane spec that's being stabilized, where the Butane spec already produces a stable Ignition spec.
  3. Tests using an experimental Butane spec that's being stabilized, where the Butane spec produces an Ignition spec which is also being stabilized.

We already have a procedure for case 1. Ignition configs in tests are stabilized by the PR that lands the new Ignition in fedora-coreos-config, and mantle has code to detect the next stable Ignition spec (otherwise unparsable) and wrap it in an older spec version that mantle does know how to handle. This is good enough for Ignition in the image, and avoids requiring immediate revendoring of the new Ignition into mantle.

Implement a similar approach for cases 2 and 3. Butane configs in tests can be stabilized by the PR that updates Ignition in the image. When we see a Butane config with an unrecognized variant/version pair, try stapling -experimental onto the spec version and see if we can parse the config. That's sufficient for case 2, but case 3 has the additional complication that Butane's output will no longer be accepted by Ignition in the image. Handle that by stripping -experimental from the Ignition config version. (Parsing the result will succeed because of the case 1 workaround.)

As a result, revendoring of both Ignition and Butane into mantle can be deferred until convenient, allowing both projects to be updated simultaneously once the new Butane release is out. This is important because there's generally a delay between the two releases, and the previous Butane version will depend on API that's missing from the new Ignition version.

Fixes coreos/butane#451.

jlebon
jlebon previously approved these changes Apr 11, 2023
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Awesome! This is going to save us a lot of time in the future.

mantle/platform/conf/conf.go Outdated Show resolved Hide resolved
@cgwalters
Copy link
Member

/retest

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW just a skim/superficial LGTM

There are three stabilization cases we need to handle:

1. Tests using an experimental Ignition spec that's being stabilized.
2. Tests using an experimental Butane spec that's being stabilized, where
   the Butane spec already produces a stable Ignition spec.
3. Tests using an experimental Butane spec that's being stabilized, where
   the Butane spec produces an Ignition spec which is also being
   stabilized.

We already have a procedure for case 1.  Ignition configs in tests are
stabilized by the PR that lands the new Ignition in
fedora-coreos-config, and mantle has code to detect the next stable
Ignition spec (otherwise unparsable) and wrap it in an older spec
version that mantle does know how to handle.  This is good enough for
Ignition in the image, and avoids requiring immediate revendoring of
the new Ignition into mantle.

Implement a similar approach for cases 2 and 3.  Butane configs in
tests can be stabilized by the PR that updates Ignition in the image.
When we see a Butane config with an unrecognized variant/version pair,
try stapling "-experimental" onto the spec version and see if we can
parse the config.  That's sufficient for case 2, but case 3 has the
additional complication that Butane's output will no longer be accepted
by Ignition in the image.  Handle that by stripping "-experimental"
from the Ignition config version.  (Parsing the result will succeed
because of the case 1 workaround.)

As a result, revendoring of both Ignition and Butane into mantle can be
deferred until convenient, allowing both projects to be updated
simultaneously once the new Butane release is out.  This is important
because there's generally a delay between the two releases, and the
previous Butane version will depend on API that's missing from the new
Ignition version.

Fixes coreos/butane#451.
Use a message similar to the new ones for Butane configs.
@bgilbert bgilbert marked this pull request as ready for review April 12, 2023 23:05
@bgilbert
Copy link
Contributor Author

Okay, should be ready now!

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jlebon
Copy link
Member

jlebon commented Apr 13, 2023

Prow timed out, but I think for this CoreOS CI is sufficient.

/override ci/prow/rhcos

@openshift-ci
Copy link

openshift-ci bot commented Apr 13, 2023

@jlebon: Overrode contexts on behalf of jlebon: ci/prow/rhcos

In response to this:

Prow timed out, but I think for this CoreOS CI is sufficient.

/override ci/prow/rhcos

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/test-infra repository.

@bgilbert bgilbert merged commit 7f023d5 into coreos:main Apr 13, 2023
2 checks passed
@bgilbert bgilbert deleted the stabilize branch April 13, 2023 15:57
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.

Make experimental spec stabilization smoother
3 participants