Detect rhcos for Ignition spec 2, add start of 3to2 converter #537

cgwalters · 2019-05-30T17:32:34Z

It would be really extremely convenient for me if I could at least
cosa run -d /path/to/rhcos.qcow2 when using cosa git master.

This prototypes out the start of a ign-3to2 helper which
will also be extremely useful for people working on OpenShift/RHCOS
today. If this works out we can elevate it to a toplevel command.

cgwalters · 2019-05-30T17:32:57Z

I often just want to run rhcos, this would help.

ajeddeloh · 2019-05-30T17:54:12Z

Lets just have two ignition configs and switch between them, rather than using bash substitution inside them.

cgwalters · 2019-05-30T18:12:55Z

Lets just have two ignition configs and switch between them, rather than using bash substitution inside them.

Mmm...I think it'd be more generally useful to write cosa ignition-3-to-2 inside COSA, or perhaps as part of ignition-validate.

ajeddeloh · 2019-05-30T18:31:58Z

or perhaps as part of ignition-validate.

I'd advocate against this. ignition-validate should just validate.

Mmm...I think it'd be more generally useful to write cosa ignition-3-to-2 inside COSA

Outside of specifically this case, what other uses would it have?

cgwalters · 2019-05-30T20:43:24Z

Outside of specifically this case, what other uses would it have?

We're about to ship OpenShift 4 which will use Ignition spec 2. A whole lot of people are writing spec 2; I deal with it every day.

cgwalters · 2019-05-30T20:44:25Z

OK, updated and now working! I only quickly hacked the ign-3to2, didn't even try to have it support anything beyond what the run command uses right now. But we can always improve it.

arithx · 2019-05-30T21:01:34Z

src/cmd-kola

-    echo "WARNING: Failed to detect distro, use -b to specify"
+    echo "WARNING: Failed to detect args, use -b to specify"
+fi
+if [ $(disk_ignition_version "${latest_qcow}") = "2.2.0" ]; then


Notably with this change --ignition-version v2 will only be specified in the basename matches rhcos-410.8* instead of just rhcos-*. Is that intentional?

ajeddeloh · 2019-05-30T21:06:56Z

Wouldn't you want a 2to3 not a 3to2 in that case?

I'm incredibly wary of tools that don't know their limits in terms of what they can and cannot do correctly. This is something I think would better writing a proper tool that knows what it can do, what it can't do, and when to report that it doesn't know if it did something right.

cgwalters · 2019-05-30T21:12:21Z

Wouldn't you want a 2to3 not a 3to2 in that case?

If we productized this as a real library, I think it'd be nicer to maintain our Ignition as spec 3 and just downconvert it where we need to, the same way cmd-run is doing.

(Of course, making this a library gets into the language issue...)

cgwalters · 2019-05-31T13:20:11Z

Updated, lifting WIP. I demoted the ign-3to2 to an internal binary instead of a command.

We talked about this on IRC #fedora-coreos. One option that was floated is to duplicate the run Ignition. I think it's telling that the converter is fewer lines of code than that. I also want to start exploring the conversion space more because as noted above, we're going to be dealing with both spec 2 and 3 for a while.

I have trouble overstating just how useful it'd be for me to have run work out of the box for RHCOS in master, since I use run a lot more than I do builds. It's a key preparatory step for getting builds working too.

src/ign-3to2

jlebon · 2019-06-03T15:59:17Z

src/cmd-run

@@ -161,12 +164,19 @@ EOF
    }
 }
 EOF
+    if [ "${ignition_version}" = "2.2.0" ]; then
+        ign_validate="true"


Could still validate the config before downgrading it to v2 though, no?

Oh yeah, but we'd have to put a build up of ignition spec 2 somewhere. And in this case we know it's valid; what we really want to do is validate in the converter I think.

ajeddeloh · 2019-06-03T18:32:56Z

I think it's telling that the converter is fewer lines of code than that.

That's because the converter doesn't cover the whole spec or handle a lot of the weird "gotcha" behavior of Ignition spec 2.x (actually it's impossible to catch everything but we ought to catch what we can).

I'm still opposed to this. It's a hack right now. I'd like to avoid hacks as much as possible. If we fully implemented a 3->2 converter where it can detect things that it can't do and covered the whole spec I'd be ok with it, but that's a pretty large undertaking.

If it's really beneficial for the short term, it should at least scream about how it's limited every time it's invoked. The file should have a giant warning banner that yells that it is incomplete and isn't a general purpose converter. We could even make it print a warning to stderr when run. I really don't want this to be a long term solution.

cgwalters · 2019-06-03T18:52:08Z

That's because the converter doesn't cover the whole spec or handle a lot of the weird "gotcha" behavior of Ignition spec 2.x (actually it's impossible to catch everything but we ought to catch what we can).

We're only scoping this right now to an internal implementation detail of run.

ashcrow · 2019-06-03T18:56:43Z

FWIW: #538 (comment)

>   Testing #537 in it's current state rebased on master now (IE: without inject-core-user).

The above built RHCOS (as long as I removed inject-core-user and let Ignition make the user in my config). I believe once #537 lands we should be at (or VERY near) to a point we can start trying out master for building.

ajeddeloh · 2019-06-03T18:57:03Z

We're only scoping this right now to an internal implementation detail of run.

Sure, but that's not documented in it anywhere. The file is called ign-3to-2 which doesn't imply any of the limitations I mentioned. It's a big footgun with no warning label.

cgwalters · 2019-06-03T20:21:38Z

Sure, but that's not documented in it anywhere.

Added a comment at the top of the file.

The file is called ign-3to-2 which doesn't imply any of the limitations I mentioned.

It's now called incomplete-hack-ign-3to2.

It would be really extremely convenient for me if I could at least `cosa run -d /path/to/rhcos.qcow2` when using cosa git master. This prototypes out the start of a `ign-3to2` helper which will also be extremely useful for people working on OpenShift/RHCOS today. If this works out we can elevate it to a toplevel command. I reworked the `abspath` bits as it turns out my use of `git annex` didn't work with this as the symlink ended up being canonicalized to the object name.

jlebon · 2019-06-04T18:38:29Z

Going to merge this to unblock further work now that it clearly specifies that it's not a general purpose translator.

cgwalters force-pushed the run-v2 branch from 76067e6 to ccd664d Compare May 30, 2019 20:42

cgwalters changed the title ~~WIP: detect rhcos for ignition spec 2~~ WIP: Add ign-3to2, detect rhcos for ignition spec 2 May 30, 2019

arithx reviewed May 30, 2019

View reviewed changes

cgwalters force-pushed the run-v2 branch 2 times, most recently from 87cd3dc to fddaa41 Compare May 31, 2019 13:16

cgwalters changed the title ~~WIP: Add ign-3to2, detect rhcos for ignition spec 2~~ Detect rhcos for Ignition spec 2, add start of 3to2 converter May 31, 2019

cgwalters force-pushed the run-v2 branch from fddaa41 to 6bc779f Compare May 31, 2019 20:24

ashcrow reviewed Jun 3, 2019

View reviewed changes

src/ign-3to2 Outdated Show resolved Hide resolved

ashcrow approved these changes Jun 3, 2019

View reviewed changes

jlebon reviewed Jun 3, 2019

View reviewed changes

ashcrow mentioned this pull request Jun 3, 2019

DONOTMERGE: EXAMPLE: COSA without rhcos breaking patches #538

Closed

cgwalters force-pushed the run-v2 branch from 6bc779f to 98e60c9 Compare June 3, 2019 20:19

ashcrow approved these changes Jun 3, 2019

View reviewed changes

cgwalters force-pushed the run-v2 branch from 98e60c9 to 9015d44 Compare June 3, 2019 20:25

jlebon merged commit 89a31f1 into coreos:master Jun 4, 2019

cgwalters mentioned this pull request Jul 12, 2019

MCS: Supporting Ignition v3 openshift/machine-config-operator#492

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect rhcos for Ignition spec 2, add start of 3to2 converter #537

Detect rhcos for Ignition spec 2, add start of 3to2 converter #537

cgwalters commented May 30, 2019 •

edited

Loading

cgwalters commented May 30, 2019

ajeddeloh commented May 30, 2019

cgwalters commented May 30, 2019

ajeddeloh commented May 30, 2019

cgwalters commented May 30, 2019

cgwalters commented May 30, 2019

arithx May 30, 2019

cgwalters May 30, 2019

ajeddeloh commented May 30, 2019

cgwalters commented May 30, 2019

cgwalters commented May 31, 2019

jlebon Jun 3, 2019

cgwalters Jun 3, 2019

ajeddeloh commented Jun 3, 2019

cgwalters commented Jun 3, 2019

ashcrow commented Jun 3, 2019

ajeddeloh commented Jun 3, 2019

cgwalters commented Jun 3, 2019

jlebon commented Jun 4, 2019

Detect rhcos for Ignition spec 2, add start of 3to2 converter #537

Detect rhcos for Ignition spec 2, add start of 3to2 converter #537

Conversation

cgwalters commented May 30, 2019 • edited Loading

cgwalters commented May 30, 2019

ajeddeloh commented May 30, 2019

cgwalters commented May 30, 2019

ajeddeloh commented May 30, 2019

cgwalters commented May 30, 2019

cgwalters commented May 30, 2019

arithx May 30, 2019

Choose a reason for hiding this comment

cgwalters May 30, 2019

Choose a reason for hiding this comment

ajeddeloh commented May 30, 2019

cgwalters commented May 30, 2019

cgwalters commented May 31, 2019

jlebon Jun 3, 2019

Choose a reason for hiding this comment

cgwalters Jun 3, 2019

Choose a reason for hiding this comment

ajeddeloh commented Jun 3, 2019

cgwalters commented Jun 3, 2019

ashcrow commented Jun 3, 2019

ajeddeloh commented Jun 3, 2019

cgwalters commented Jun 3, 2019

jlebon commented Jun 4, 2019

cgwalters commented May 30, 2019 •

edited

Loading