You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As of coreos/ignition#1193, Ignition provides functions allowing any config 3.0 ≤ x ≤ 3.n to be parsed to a 3.n config struct without writing a series of explicit parsing attempts in the caller. The MCO would also like to do the opposite: first render a config to the maximum supported version, then downconvert it to the oldest 3.x spec that supports the features actually used in the config. This is needed because the MCO writes pre-rendered configs that must be maximally compatible with old MCO versions, and there is no mechanism like the Accept header for negotiating supported spec versions.
This is possible today by explicitly attempting downconversions until one returns an error. However, it'd be nice to implement that code centrally in ign-converter, rather than in the caller. There's a catch though: the generic downconversion function won't know what concrete struct types it's accepting or returning. There are a few options:
Accept and return interface{}. We'd then need to do some type assertions to figure out the input type, and the caller would then do type assertions to figure out the output type (if it cares) or just serialize the interface{} directly to JSON.
Accept and return reflect.Value. I think this is basically equivalent to 1 except that it requires the caller to import reflect.
Accept and return []byte. That requires JSON encoding/decoding, which ign-converter doesn't currently do (except for the command-line wrapper), and possibly we'd want to let Ignition code handle the actual parsing.
I'm not sure that 2 or 3 improve either the API or the implementation, so I'd lean toward option 1 for simplicity. An interface{} -> interface{} API is awkward, but that seems intrinsic to the problem we're solving.
@yuqi-zhang pointed out that there's no reason for the MCO to downgrade rendered configs to an older version than those configs already have, and released versions of the MCO already write 3.2.0 configs. The MCO codebase has not yet been updated to 3.3.0 (though see openshift/machine-config-operator#2675), and chaining will not be needed until 3.4.0. At the moment, therefore, it seems there's no pressing need for this functionality.
As of coreos/ignition#1193, Ignition provides functions allowing any config 3.0 ≤ x ≤ 3.n to be parsed to a 3.n config struct without writing a series of explicit parsing attempts in the caller. The MCO would also like to do the opposite: first render a config to the maximum supported version, then downconvert it to the oldest 3.x spec that supports the features actually used in the config. This is needed because the MCO writes pre-rendered configs that must be maximally compatible with old MCO versions, and there is no mechanism like the
Accept
header for negotiating supported spec versions.This is possible today by explicitly attempting downconversions until one returns an error. However, it'd be nice to implement that code centrally in ign-converter, rather than in the caller. There's a catch though: the generic downconversion function won't know what concrete struct types it's accepting or returning. There are a few options:
interface{}
. We'd then need to do some type assertions to figure out the input type, and the caller would then do type assertions to figure out the output type (if it cares) or just serialize theinterface{}
directly to JSON.reflect.Value
. I think this is basically equivalent to 1 except that it requires the caller to importreflect
.[]byte
. That requires JSON encoding/decoding, which ign-converter doesn't currently do (except for the command-line wrapper), and possibly we'd want to let Ignition code handle the actual parsing.I'm not sure that 2 or 3 improve either the API or the implementation, so I'd lean toward option 1 for simplicity. An
interface{}
->interface{}
API is awkward, but that seems intrinsic to the problem we're solving.cc @yuqi-zhang
The text was updated successfully, but these errors were encountered: