*: switch to v3.2.0 Ignition configs instead of v3.1.0 #2248

arithx · 2020-11-23T01:55:12Z

- What I did

Switch the underlying Ignition spec 3 configs to use the 3.2.0 types (and update the underlying translation for spec 3 configs to uptranslate into 3.2.0).

- How to verify it

Spec 3 configs should remain working. Additionally new config fields that are only present in the Ignition 3.2.0 spec (see here for a list of new features to the 3.2.0 spec) can be specified.

- Description for the changelog

vendor commit: Bump ign-converter to pick up v32tov2x translation packages & pull in spec 3.2.0 Ignition packages
other commit: update the ign3types import to use the config/v3_2/types package instead of config/v3_1/types

arithx · 2020-11-23T17:43:09Z

/retest

cgwalters

Mainly concerned about the MCS parts.

If every spec bump involves this level of churn that's a bit unfortunate but, solving that seems hard without reworking the API.

cgwalters · 2020-11-23T20:19:50Z

pkg/server/api.go

@@ -283,8 +283,8 @@ func detectSpecVersionFromAcceptHeader(acceptHeader string) (*semver.Version, er

 	for _, header := range headers {
 		if header.MIMESubtype == "vnd.coreos.ignition+json" && header.SemVer != nil {
-			if !header.SemVer.LessThan(*v3_1) && header.SemVer.LessThan(*semver.New("4.0.0")) {
-				return v3_1, nil
+			if !header.SemVer.LessThan(*v3_2) && header.SemVer.LessThan(*semver.New("4.0.0")) {


In both the API and the tests we need to also continue to handle 3.1 right?

Ah yep, that was an oversight. I've got coreos/ign-converter#19 up to add v32tov31 down-translation. After that merges I'll bump the ign-converter vendor again & update to check for v3_1 semver, down-translate in server/api & add tests for 3.1.0 in server/api_test.

Ok, I think I've addressed this.

I've added back support for v3_1 headers, added 3.2.0 -> 3.1.0 translation, and re-added unit tests in pkg/server/api_test.

Ignition was already bumped to version 2.7.0 but nothing was actually referencing the v3_2 configs to pull them in.

cgwalters · 2020-11-24T15:17:55Z

/approve

cgwalters · 2020-11-24T16:38:34Z

Hmm not sure what went wrong with that e2e-aws run; installer ran fine but then this failed which seems like something in CI glue:

2020/11/23 23:11:26 Failed to upload $KUBECONFIG: timed out waiting for the condition

arithx · 2020-11-24T17:15:38Z

/retest

arithx · 2020-11-24T19:21:27Z

/retest

kikisdeliveryservice · 2020-11-24T19:42:23Z

throwing a hold while i review

/hold

kikisdeliveryservice · 2020-11-24T19:43:14Z

Also okd-e2e is currently red and it looks like rhel7 is flaky, so you don't have to retest @arithx ill confirm we can skip the rhel7 one and deal with that later.

darkmuggle · 2020-12-01T16:44:10Z

@kikisdeliveryservice in light of your hold, have you had a chance to review?

The update to spec is needed for finalizing complex disks (confirmed by @bgilbert via Slack)

I looked over the code, and nothing stands out.
/approve

darkmuggle · 2020-12-01T16:44:41Z

/retest

kikisdeliveryservice · 2020-12-01T23:42:59Z

I believe @yuqi-zhang is looking it over too, wanted to give MCO team the opp to review this as it's non-trivial.

ashcrow · 2020-12-02T21:52:12Z

@yuqi-zhang PTAL

yuqi-zhang · 2020-12-02T22:32:58Z

Did some testing for this, upgraded fine, and all generated configs are on 3.2, applied 2.2 config looks fine, scaling node looks fine

/lgtm

openshift-ci-robot · 2020-12-02T22:33:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arithx, cgwalters, darkmuggle, yuqi-zhang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [cgwalters,yuqi-zhang]

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

yuqi-zhang · 2020-12-02T22:36:56Z

Will let @kikisdeliveryservice cancel the hold when comfortable

pkg/controller/common/helpers.go

kikisdeliveryservice · 2020-12-02T22:44:31Z

will lift hold once jerry's q above is addressed.

kikisdeliveryservice · 2020-12-02T22:53:48Z

Q as a consumer of ignition : are you planning on chaining parsers? bc as you can see moving forward with future versions this is going to cause a mess...

kikisdeliveryservice · 2020-12-02T23:05:11Z

/hold cancel

bgilbert · 2020-12-02T23:09:38Z

@kikisdeliveryservice We chained parsers in spec 2.x but removed that functionality for spec 3. I agree that we should think about adding it back.

bgilbert · 2020-12-03T04:23:04Z

/retest

bgilbert · 2020-12-03T05:58:56Z

/retest

openshift-bot · 2020-12-03T14:54:28Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-03T15:59:28Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-03T20:06:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

kikisdeliveryservice · 2020-12-03T20:15:39Z

this is fully approved and made the deadline, e2e is still hitting some well documented ci resource issues:

   * could not run steps: step e2e-agnostic-upgrade failed: failed to acquire lease: resources not found
time="2020-12-03T17:10:30Z" level=info msg="Reporting job state 'failed' with reason 'executing_graph:step_failed:utilizing_lease'"

which are still being worked on by admins...

openshift-bot · 2020-12-03T22:16:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-04T04:07:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-04T04:20:06Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-04T07:48:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-04T08:01:27Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot requested review from mtrmac and umohnani8 November 23, 2020 01:55

arithx force-pushed the spec320 branch 2 times, most recently from 62e4225 to 2ab63df Compare November 23, 2020 02:07

cgwalters reviewed Nov 23, 2020

View reviewed changes

arithx added 2 commits November 23, 2020 16:07

vendor: bump ign-converter & pull Ignition v3.2.0 config

e9b43eb

Ignition was already bumped to version 2.7.0 but nothing was actually referencing the v3_2 configs to pull them in.

*: switch to v3.2.0 configs instead of v3.1.0

d4188ac

arithx force-pushed the spec320 branch from 2ab63df to d4188ac Compare November 23, 2020 22:32

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2020

kikisdeliveryservice requested review from runcom, yuqi-zhang and kikisdeliveryservice and removed request for mtrmac and umohnani8 November 24, 2020 19:44

arithx mentioned this pull request Nov 25, 2020

modules/encrypt-disk: update encryption to use Ignition-created LUKS openshift/openshift-docs#27661

Merged

openshift-ci-robot assigned yuqi-zhang Dec 2, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2020

yuqi-zhang reviewed Dec 2, 2020

View reviewed changes

pkg/controller/common/helpers.go Show resolved Hide resolved

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2020

openshift-merge-robot merged commit 906673c into openshift:master Dec 4, 2020

bertinatto mentioned this pull request Jan 18, 2021

Bug 1896226: Remove recycler pod templates #2318

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: switch to v3.2.0 Ignition configs instead of v3.1.0 #2248

*: switch to v3.2.0 Ignition configs instead of v3.1.0 #2248

arithx commented Nov 23, 2020 •

edited

arithx commented Nov 23, 2020

cgwalters left a comment

cgwalters Nov 23, 2020

arithx Nov 23, 2020

arithx Nov 23, 2020

cgwalters commented Nov 24, 2020

cgwalters commented Nov 24, 2020

arithx commented Nov 24, 2020

arithx commented Nov 24, 2020

kikisdeliveryservice commented Nov 24, 2020

kikisdeliveryservice commented Nov 24, 2020

darkmuggle commented Dec 1, 2020

darkmuggle commented Dec 1, 2020

kikisdeliveryservice commented Dec 1, 2020

ashcrow commented Dec 2, 2020

yuqi-zhang commented Dec 2, 2020

openshift-ci-robot commented Dec 2, 2020

yuqi-zhang commented Dec 2, 2020

kikisdeliveryservice commented Dec 2, 2020

kikisdeliveryservice commented Dec 2, 2020

kikisdeliveryservice commented Dec 2, 2020

bgilbert commented Dec 2, 2020

bgilbert commented Dec 3, 2020

bgilbert commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

kikisdeliveryservice commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

openshift-bot commented Dec 4, 2020

openshift-bot commented Dec 4, 2020

openshift-bot commented Dec 4, 2020

openshift-bot commented Dec 4, 2020

*: switch to v3.2.0 Ignition configs instead of v3.1.0 #2248

*: switch to v3.2.0 Ignition configs instead of v3.1.0 #2248

Conversation

arithx commented Nov 23, 2020 • edited

arithx commented Nov 23, 2020

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Nov 23, 2020

Choose a reason for hiding this comment

arithx Nov 23, 2020

Choose a reason for hiding this comment

arithx Nov 23, 2020

Choose a reason for hiding this comment

cgwalters commented Nov 24, 2020

cgwalters commented Nov 24, 2020

arithx commented Nov 24, 2020

arithx commented Nov 24, 2020

kikisdeliveryservice commented Nov 24, 2020

kikisdeliveryservice commented Nov 24, 2020

darkmuggle commented Dec 1, 2020

darkmuggle commented Dec 1, 2020

kikisdeliveryservice commented Dec 1, 2020

ashcrow commented Dec 2, 2020

yuqi-zhang commented Dec 2, 2020

openshift-ci-robot commented Dec 2, 2020

yuqi-zhang commented Dec 2, 2020

kikisdeliveryservice commented Dec 2, 2020

kikisdeliveryservice commented Dec 2, 2020

kikisdeliveryservice commented Dec 2, 2020

bgilbert commented Dec 2, 2020

bgilbert commented Dec 3, 2020

bgilbert commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

kikisdeliveryservice commented Dec 3, 2020

openshift-bot commented Dec 3, 2020

openshift-bot commented Dec 4, 2020

openshift-bot commented Dec 4, 2020

openshift-bot commented Dec 4, 2020

openshift-bot commented Dec 4, 2020

arithx commented Nov 23, 2020 •

edited