From 2c032047d2dde28d5802abecda4139014a706a00 Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Wed, 7 Jun 2023 16:44:37 -0500 Subject: [PATCH 1/4] clean 4.13 downconvert hacks out of helpers --- pkg/controller/common/helpers.go | 76 +------------------------------- 1 file changed, 1 insertion(+), 75 deletions(-) diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index 1ad0e2eb82..8967ea52c0 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -22,15 +22,12 @@ import ( "github.com/coreos/ign-converter/translate/v23tov30" "github.com/coreos/ign-converter/translate/v32tov22" "github.com/coreos/ign-converter/translate/v32tov31" - "github.com/coreos/ign-converter/translate/v33tov32" - "github.com/coreos/ign-converter/translate/v34tov33" ign2error "github.com/coreos/ignition/config/shared/errors" ign2 "github.com/coreos/ignition/config/v2_2" ign2types "github.com/coreos/ignition/config/v2_2/types" ign2_3 "github.com/coreos/ignition/config/v2_3" validate2 "github.com/coreos/ignition/config/validate" ign3error "github.com/coreos/ignition/v2/config/shared/errors" - ign3errors "github.com/coreos/ignition/v2/config/shared/errors" ign3_1 "github.com/coreos/ignition/v2/config/v3_1" translate3_1 "github.com/coreos/ignition/v2/config/v3_1/translate" ign3_1types "github.com/coreos/ignition/v2/config/v3_1/types" @@ -39,7 +36,6 @@ import ( ign3types "github.com/coreos/ignition/v2/config/v3_2/types" ign3_3 "github.com/coreos/ignition/v2/config/v3_3" ign3_4 "github.com/coreos/ignition/v2/config/v3_4" - ign3_4types "github.com/coreos/ignition/v2/config/v3_4/types" validate3 "github.com/coreos/ignition/v2/config/validate" "github.com/ghodss/yaml" "github.com/vincent-petithory/dataurl" @@ -146,23 +142,6 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.Contro } } - // Take the kargs from ignition and put them in MachineConfig if we're downgrading - // TODO(jkyros): This block is only here for downgrade compatibility - // with future MCOs that understand and use ignition 3.3+ KernelArguments - // remove this when we raise the ignition default to 3.4 - for _, cfg := range configs { - ignKargs, err := ExtractIgnitionKargsFor4_13(cfg.Spec.Config.Raw) - if err != nil { - return nil, fmt.Errorf("Error downgrading KernelArguments for %s: %w", cfg.Name, err) - } - for _, arg := range ignKargs { - // ignition KernelArgument is a string under the hood, so we can convert it - if !InSlice(string(arg), kargs) { - kargs = append(kargs, string(arg)) - } - } - } - extensions := []string{} for _, cfg := range configs { extensions = append(extensions, cfg.Spec.Extensions...) @@ -552,66 +531,13 @@ func ValidateMachineConfig(cfg mcfgv1.MachineConfigSpec) error { return nil } -// ExtractIgnitionKargsFor4_13 parses a raw ignition config to 3.4 and extracts the kernel args, -// erroring if ShouldNotExist is populated (since this MCO doesn't know how to implement it). This -// function is used by the render controller to extract the kargs from ignition and migrate them to -// MachineConfig so newer versions that specify kargs in ignition will still work during a downgrade. -// TODO(jkyros): remove this when we raise the ignition default to 3.4 -func ExtractIgnitionKargsFor4_13(rawIgn []byte) ([]ign3_4types.KernelArgument, error) { - ignCfgV3, rptV3, errV3 := ign3_4.ParseCompatibleVersion(rawIgn) - // No kargs in an empty config, and we only want to extract args from versions that have them - // We will fail earlier in render controller before we get here if it's actually a version we can't parse - if errors.Is(errV3, ign3errors.ErrEmpty) || errors.Is(errV3, ign3error.ErrUnknownVersion) { - return nil, nil - } - if errV3 == nil && !rptV3.IsFatal() { - // We can't reconcile ShouldNotExist - if len(ignCfgV3.KernelArguments.ShouldNotExist) > 0 { - return nil, fmt.Errorf("Ignition KernelArguments.ShouldNotExist is not supported in this release ( ShouldNotExist: %s was supplied )", ignCfgV3.KernelArguments.ShouldNotExist) - } - - // But we can stuff ShouldExist in MachineConfig, so send those back - if len(ignCfgV3.KernelArguments.ShouldExist) > 0 { - return ignCfgV3.KernelArguments.ShouldExist, nil - } - - // There were no args in this ignition - return nil, nil - } - - return nil, fmt.Errorf("parsing Ignition config spec v3.4 failed with error: %v\nReport: %v", errV3, rptV3) -} - // IgnParseWrapper parses rawIgn for both V2 and V3 ignition configs and returns // a V2 or V3 Config or an error. This wrapper is necessary since V2 and V3 use different parsers. func IgnParseWrapper(rawIgn []byte) (interface{}, error) { // ParseCompatibleVersion will parse any config <= N to version N ignCfgV3, rptV3, errV3 := ign3_4.ParseCompatibleVersion(rawIgn) if errV3 == nil && !rptV3.IsFatal() { - - // TODO(jkyros): This removes/ignores the kargs for the downconversion to 3.2 since 3.2 doesn't - // support the kargs fields (and the MCO doesn't either) but it still needs to be okay if we receive one. - // This is okay because in our MachineConfig render we merge them into MachineConfig kargs before - // these get stripped out. - ignCfgV3.KernelArguments = ign3_4types.KernelArguments{} - - // Regardless of the input version it has now been translated to a 3.4 config, downtranslate to 3.3 so we - // can get down to 3.2 - ignCfgV3_3, errV3_3 := v34tov33.Translate(ignCfgV3) - if errV3_3 != nil { - // This case should only be hit if fields that only exist in v3_4 are being used which are not - // currently supported by the MCO - return ign3types.Config{}, fmt.Errorf("translating Ignition config 3.4 to 3.3 failed with error: %v", errV3_3) - } - // Regardless of the input version it has now been translated to a 3.3 config, downtranslate to 3.2 to match - // what is used internally - ignCfgV3_2, errV3_2 := v33tov32.Translate(ignCfgV3_3) - if errV3_2 != nil { - // This case should only be hit if fields that only exist in v3_3 are being used which are not - // currently supported by the MCO - return ign3types.Config{}, fmt.Errorf("translating Ignition config 3.3 to 3.2 failed with error: %v", errV3_2) - } - return ignCfgV3_2, nil + return ignCfgV3, nil } // ParseCompatibleVersion differentiates between ErrUnknownVersion ("I know what it is and we don't support it") and From 7b6396fba104d84256d47476eed4d36008e22615 Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Wed, 7 Jun 2023 16:45:08 -0500 Subject: [PATCH 2/4] bump ignition api version to 3.4 in mcs --- pkg/server/api.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/server/api.go b/pkg/server/api.go index 646d753731..217acd2606 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -133,30 +133,30 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) return } - // we know we're at 3.2 in code.. serve directly, parsing is expensive... + // we know we're at 3.4 in code.. serve directly, parsing is expensive... // we're doing it during an HTTP request, and most notably before we write the HTTP headers var serveConf *runtime.RawExtension if reqConfigVer.Equal(*semver.New("3.4.0")) { - converted34, err := ctrlcommon.ConvertRawExtIgnitionToV3_4(conf) + serveConf = conf + + } else if reqConfigVer.Equal(*semver.New("3.3.0")) { + converted33, err := ctrlcommon.ConvertRawExtIgnitionToV3_3(conf) if err != nil { w.Header().Set("Content-Length", "0") w.WriteHeader(http.StatusInternalServerError) klog.Errorf("couldn't convert config for req: %v, error: %v", cr, err) return } - serveConf = &converted34 - - } else if reqConfigVer.Equal(*semver.New("3.3.0")) { - converted33, err := ctrlcommon.ConvertRawExtIgnitionToV3_3(conf) + serveConf = &converted33 + } else if reqConfigVer.Equal(*semver.New("3.2.0")) { + converted32, err := ctrlcommon.ConvertRawExtIgnitionToV3_2(conf) if err != nil { w.Header().Set("Content-Length", "0") w.WriteHeader(http.StatusInternalServerError) klog.Errorf("couldn't convert config for req: %v, error: %v", cr, err) return } - serveConf = &converted33 - } else if reqConfigVer.Equal(*semver.New("3.2.0")) { - serveConf = conf + serveConf = &converted32 } else if reqConfigVer.Equal(*semver.New("3.1.0")) { converted31, err := ctrlcommon.ConvertRawExtIgnitionToV3_1(conf) if err != nil { From edaecd9c78a456576c042fd5a28ff143bff2fd2f Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Fri, 14 Jul 2023 17:03:56 -0500 Subject: [PATCH 3/4] ignition to 3.4, fix references and translation This updates the default "internal" MCO version to 3.4.0 and it will by default try to convert/translate everything it gets to that version. The convention I'm following here is that int3types becomes the base ignition3 version that the MCO is using, and anything called out explicitly like ign3_4types is being used explicitly as that version not as the default version. So (hopefully) the next person that gets to do a migration should only have to go replace ign3types, add the downtranslator, and see what broke. --- lib/resourceapply/machineconfig_test.go | 2 +- pkg/controller/bootstrap/bootstrap_test.go | 2 +- pkg/controller/build/build_controller_test.go | 2 +- pkg/controller/build/fixtures_test.go | 2 +- pkg/controller/common/constants.go | 7 + pkg/controller/common/helpers.go | 172 ++++++++++++------ pkg/controller/common/helpers_test.go | 81 +++------ .../container_runtime_config_controller.go | 2 +- ...ontainer_runtime_config_controller_test.go | 2 +- .../container-runtime-config/helpers.go | 2 +- pkg/controller/kubelet-config/helpers.go | 2 +- .../kubelet_config_controller.go | 2 +- .../kubelet_config_controller_test.go | 2 +- .../kubelet_config_features_test.go | 2 +- .../kubelet_config_nodes_test.go | 2 +- .../render/render_controller_test.go | 2 +- pkg/controller/template/render_test.go | 2 +- pkg/daemon/config_drift_monitor.go | 2 +- pkg/daemon/config_drift_monitor_test.go | 2 +- pkg/daemon/daemon.go | 2 +- pkg/daemon/daemon_test.go | 2 +- pkg/daemon/drain.go | 2 +- pkg/daemon/drain_test.go | 2 +- pkg/daemon/file_writers.go | 2 +- pkg/daemon/on_disk_validation.go | 2 +- pkg/daemon/update.go | 9 +- pkg/daemon/update_test.go | 42 ++++- pkg/server/api.go | 2 +- pkg/server/server.go | 30 +-- pkg/server/server_test.go | 23 +-- test/e2e-shared-tests/mcd_config_drift.go | 2 +- test/e2e-single-node/sno_mcd_test.go | 2 +- test/e2e/mcd_test.go | 2 +- test/e2e/osimageurl_override_test.go | 2 +- test/helpers/helpers.go | 2 +- test/helpers/utils.go | 2 +- 36 files changed, 243 insertions(+), 179 deletions(-) diff --git a/lib/resourceapply/machineconfig_test.go b/lib/resourceapply/machineconfig_test.go index c0a1906cd9..97b1eb4352 100644 --- a/lib/resourceapply/machineconfig_test.go +++ b/lib/resourceapply/machineconfig_test.go @@ -4,7 +4,7 @@ import ( "fmt" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/davecgh/go-spew/spew" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" diff --git a/pkg/controller/bootstrap/bootstrap_test.go b/pkg/controller/bootstrap/bootstrap_test.go index 93590fc83f..6f11cb29a5 100644 --- a/pkg/controller/bootstrap/bootstrap_test.go +++ b/pkg/controller/bootstrap/bootstrap_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/diff" diff --git a/pkg/controller/build/build_controller_test.go b/pkg/controller/build/build_controller_test.go index d5bd54d981..f312f92d1f 100644 --- a/pkg/controller/build/build_controller_test.go +++ b/pkg/controller/build/build_controller_test.go @@ -6,7 +6,7 @@ import ( "os" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" buildv1 "github.com/openshift/api/build/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/controller/build/fixtures_test.go b/pkg/controller/build/fixtures_test.go index eec68a7747..2d29cb4401 100644 --- a/pkg/controller/build/fixtures_test.go +++ b/pkg/controller/build/fixtures_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/davecgh/go-spew/spew" "github.com/ghodss/yaml" buildv1 "github.com/openshift/api/build/v1" diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index 571fc825f2..54c2076c09 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -50,4 +50,11 @@ const ( ExperimentalNewestLayeredImageEquivalentConfigAnnotationKey = "machineconfiguration.openshift.io/newestImageEquivalentConfig" OSImageBuildPodLabel = "machineconfiguration.openshift.io/buildPod" + + // InternalMCOIgnitionVersion is the ignition version that the MCO converts everything to internally. The intent here is that + // we should be able to update this constant when we bump the internal ignition version instead of having to hunt down all of + // the version references and figure out "was this supposed to be explicitly 3.4.0 or just the default version which happens + // to be 3.4.0 currently". Ideally if you find an explicit "3.4.0", it's supposed to be "3.4.0" version. If it's this constant, + // it's supposed to be the internal default version. + InternalMCOIgnitionVersion = "3.4.0" ) diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index 8967ea52c0..1dd4ec1a39 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -22,20 +22,25 @@ import ( "github.com/coreos/ign-converter/translate/v23tov30" "github.com/coreos/ign-converter/translate/v32tov22" "github.com/coreos/ign-converter/translate/v32tov31" + "github.com/coreos/ign-converter/translate/v33tov32" + "github.com/coreos/ign-converter/translate/v34tov33" ign2error "github.com/coreos/ignition/config/shared/errors" ign2 "github.com/coreos/ignition/config/v2_2" ign2types "github.com/coreos/ignition/config/v2_2/types" ign2_3 "github.com/coreos/ignition/config/v2_3" validate2 "github.com/coreos/ignition/config/validate" ign3error "github.com/coreos/ignition/v2/config/shared/errors" - ign3_1 "github.com/coreos/ignition/v2/config/v3_1" translate3_1 "github.com/coreos/ignition/v2/config/v3_1/translate" ign3_1types "github.com/coreos/ignition/v2/config/v3_1/types" - ign3 "github.com/coreos/ignition/v2/config/v3_2" - translate3 "github.com/coreos/ignition/v2/config/v3_2/translate" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" - ign3_3 "github.com/coreos/ignition/v2/config/v3_3" + translate3_2 "github.com/coreos/ignition/v2/config/v3_2/translate" + ign3_2types "github.com/coreos/ignition/v2/config/v3_2/types" + translate3_3 "github.com/coreos/ignition/v2/config/v3_3/translate" + ign3_3types "github.com/coreos/ignition/v2/config/v3_3/types" + + ign3 "github.com/coreos/ignition/v2/config/v3_4" ign3_4 "github.com/coreos/ignition/v2/config/v3_4" + translate3 "github.com/coreos/ignition/v2/config/v3_4/translate" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" validate3 "github.com/coreos/ignition/v2/config/validate" "github.com/ghodss/yaml" "github.com/vincent-petithory/dataurl" @@ -242,30 +247,17 @@ func WriteTerminationError(err error) { // ConvertRawExtIgnitionToV3 ensures that the Ignition config in // the RawExtension is spec v3.2, or translates to it. -func ConvertRawExtIgnitionToV3(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { - // This function is only used by the MCServer so we don't need to consider v3.0 - _, rptV3, errV3 := ign3.Parse(inRawExtIgn.Raw) - if errV3 == nil && !rptV3.IsFatal() { - // The rawExt is already on V3.2, no need to translate - return *inRawExtIgn, nil - } +func ConvertRawExtIgnitionToV3_4(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { - var converted3 ign3types.Config - ignCfgV3_1, rptV3_1, errV3_1 := ign3_1.Parse(inRawExtIgn.Raw) - if errV3_1 == nil && !rptV3_1.IsFatal() { - converted3 = translate3.Translate(ignCfgV3_1) - } else { - ignCfg, rpt, err := ign2.Parse(inRawExtIgn.Raw) - if err != nil || rpt.IsFatal() { - return runtime.RawExtension{}, fmt.Errorf("parsing Ignition config spec v2.2 failed with error: %w\nReport: %v", err, rpt) - } - converted3, err = convertIgnition2to3(ignCfg) - if err != nil { - return runtime.RawExtension{}, fmt.Errorf("failed to convert config from spec v2.2 to v3.2: %w", err) - } + // Parse the raw extension to the MCO's current internal ignition version + ignCfgV3, err := IgnParseWrapper(inRawExtIgn.Raw) + if err != nil { + return runtime.RawExtension{}, err } - outIgnV3, err := json.Marshal(converted3) + // TODO(jkyros): we used to only re-marshal this if it was the wrong version, now we're + // re-marshaling every time + outIgnV3, err := json.Marshal(ignCfgV3) if err != nil { return runtime.RawExtension{}, fmt.Errorf("failed to marshal converted config: %w", err) } @@ -276,46 +268,69 @@ func ConvertRawExtIgnitionToV3(inRawExtIgn *runtime.RawExtension) (runtime.RawEx return outRawExt, nil } -// ConvertRawExtIgnitionToV3_4 ensures that the Ignition config in -// the RawExtension is spec v3.4, or translates to it. -func ConvertRawExtIgnitionToV3_4(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { - // TODO(jkyros): since 3.4 is "ahead" of our current default 3.2, we're going "up" not down, which is - // why we can use ParseCompatibleVersion. Once we bump to 3.4 this will be briefly obsolete, but once we - // bump the default past 3.4 we will have to come back and "downconvert". - ignCfgV34, rptV3, errV3 := ign3_4.ParseCompatibleVersion(inRawExtIgn.Raw) +// ConvertRawExtIgnitionToV3_3 ensures that the Ignition config in +// the RawExtension is spec v3.3, or translates to it. +func ConvertRawExtIgnitionToV3_3(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { + rawExt, err := ConvertRawExtIgnitionToV3_4(inRawExtIgn) + if err != nil { + return runtime.RawExtension{}, err + } + + ignCfgV3, rptV3, errV3 := ign3.Parse(rawExt.Raw) if errV3 != nil || rptV3.IsFatal() { return runtime.RawExtension{}, fmt.Errorf("parsing Ignition config failed with error: %w\nReport: %v", errV3, rptV3) } - outIgnV34, err := json.Marshal(ignCfgV34) + // TODO(jkyros): someday we should write a recursive chain-downconverter, but until then, + // we're going to do it the hard way + ignCfgV33, err := convertIgnition34to33(ignCfgV3) + if err != nil { + return runtime.RawExtension{}, err + } + + outIgnV33, err := json.Marshal(ignCfgV33) if err != nil { return runtime.RawExtension{}, fmt.Errorf("failed to marshal converted config: %w", err) } outRawExt := runtime.RawExtension{} - outRawExt.Raw = outIgnV34 + outRawExt.Raw = outIgnV33 return outRawExt, nil } // ConvertRawExtIgnitionToV3_3 ensures that the Ignition config in // the RawExtension is spec v3.3, or translates to it. -func ConvertRawExtIgnitionToV3_3(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { - // TODO(jkyros): since 3.3 is "ahead" of our current default 3.2, we're going "up" not down, which is - // why we can use ParseCompatibleVersion. Once we bump to 3.3 this will be briefly obsolete, but once we - // bump to 3.4 we will have to come back and "downconvert". - ignCfgV33, rptV3, errV3 := ign3_3.ParseCompatibleVersion(inRawExtIgn.Raw) +func ConvertRawExtIgnitionToV3_2(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { + rawExt, err := ConvertRawExtIgnitionToV3_4(inRawExtIgn) + if err != nil { + return runtime.RawExtension{}, err + } + + ignCfgV3, rptV3, errV3 := ign3.Parse(rawExt.Raw) if errV3 != nil || rptV3.IsFatal() { return runtime.RawExtension{}, fmt.Errorf("parsing Ignition config failed with error: %w\nReport: %v", errV3, rptV3) } - outIgnV33, err := json.Marshal(ignCfgV33) + // TODO(jkyros): someday we should write a recursive chain-downconverter, but until then, + // we're going to do it the hard way + ignCfgV33, err := convertIgnition34to33(ignCfgV3) + if err != nil { + return runtime.RawExtension{}, err + } + + ignCfgV32, err := convertIgnition33to32(ignCfgV33) + if err != nil { + return runtime.RawExtension{}, err + } + + outIgnV32, err := json.Marshal(ignCfgV32) if err != nil { return runtime.RawExtension{}, fmt.Errorf("failed to marshal converted config: %w", err) } outRawExt := runtime.RawExtension{} - outRawExt.Raw = outIgnV33 + outRawExt.Raw = outIgnV32 return outRawExt, nil } @@ -323,7 +338,7 @@ func ConvertRawExtIgnitionToV3_3(inRawExtIgn *runtime.RawExtension) (runtime.Raw // ConvertRawExtIgnitionToV3_1 ensures that the Ignition config in // the RawExtension is spec v3.1, or translates to it. func ConvertRawExtIgnitionToV3_1(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { - rawExt, err := ConvertRawExtIgnitionToV3(inRawExtIgn) + rawExt, err := ConvertRawExtIgnitionToV3_4(inRawExtIgn) if err != nil { return runtime.RawExtension{}, err } @@ -333,7 +348,19 @@ func ConvertRawExtIgnitionToV3_1(inRawExtIgn *runtime.RawExtension) (runtime.Raw return runtime.RawExtension{}, fmt.Errorf("parsing Ignition config failed with error: %w\nReport: %v", errV3, rptV3) } - ignCfgV31, err := convertIgnition32to31(ignCfgV3) + // TODO(jkyros): someday we should write a recursive chain-downconverter, but until then, + // we're going to do it the hard way + ignCfgV33, err := convertIgnition34to33(ignCfgV3) + if err != nil { + return runtime.RawExtension{}, err + } + + ignCfgV32, err := convertIgnition33to32(ignCfgV33) + if err != nil { + return runtime.RawExtension{}, err + } + + ignCfgV31, err := convertIgnition32to31(ignCfgV32) if err != nil { return runtime.RawExtension{}, err } @@ -351,13 +378,13 @@ func ConvertRawExtIgnitionToV3_1(inRawExtIgn *runtime.RawExtension) (runtime.Raw // ConvertRawExtIgnitionToV2 ensures that the Ignition config in // the RawExtension is spec v2.2, or translates to it. -func ConvertRawExtIgnitionToV2(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { +func ConvertRawExtIgnitionToV2_2(inRawExtIgn *runtime.RawExtension) (runtime.RawExtension, error) { ignCfg, rpt, err := ign3.Parse(inRawExtIgn.Raw) if err != nil || rpt.IsFatal() { return runtime.RawExtension{}, fmt.Errorf("parsing Ignition config spec v3.2 failed with error: %w\nReport: %v", err, rpt) } - converted2, err := convertIgnition3to2(ignCfg) + converted2, err := convertIgnition34to22(ignCfg) if err != nil { return runtime.RawExtension{}, fmt.Errorf("failed to convert config from spec v3.2 to v2.2: %w", err) } @@ -374,7 +401,7 @@ func ConvertRawExtIgnitionToV2(inRawExtIgn *runtime.RawExtension) (runtime.RawEx } // convertIgnition2to3 takes an ignition spec v2.2 config and returns a v3.2 config -func convertIgnition2to3(ign2config ign2types.Config) (ign3types.Config, error) { +func convertIgnition22to34(ign2config ign2types.Config) (ign3types.Config, error) { // only support writing to root file system fsMap := map[string]string{ "root": "/", @@ -386,16 +413,28 @@ func convertIgnition2to3(ign2config ign2types.Config) (ign3types.Config, error) if err != nil { return ign3types.Config{}, fmt.Errorf("unable to convert Ignition spec v2 config to v3: %w", err) } - // Workaround to get a v3.2 config as output - converted3 := translate3.Translate(translate3_1.Translate(ign3_0config)) + // Workaround to get a v3.4 config as output + converted3 := translate3.Translate(translate3_3.Translate(translate3_2.Translate(translate3_1.Translate(ign3_0config)))) klog.V(4).Infof("Successfully translated Ignition spec v2 config to Ignition spec v3 config: %v", converted3) return converted3, nil } // convertIgnition3to2 takes an ignition spec v3.2 config and returns a v2.2 config -func convertIgnition3to2(ign3config ign3types.Config) (ign2types.Config, error) { - converted2, err := v32tov22.Translate(ign3config) +func convertIgnition34to22(ign3config ign3types.Config) (ign2types.Config, error) { + + // TODO(jkyros): that recursive down-converter is looking like a better idea all the time + converted33, err := convertIgnition34to33(ign3config) + if err != nil { + return ign2types.Config{}, fmt.Errorf("unable to convert Ignition spec v3 config to v2: %w", err) + } + + converted32, err := convertIgnition33to32(converted33) + if err != nil { + return ign2types.Config{}, fmt.Errorf("unable to convert Ignition spec v3 config to v2: %w", err) + } + + converted2, err := v32tov22.Translate(converted32) if err != nil { return ign2types.Config{}, fmt.Errorf("unable to convert Ignition spec v3 config to v2: %w", err) } @@ -404,8 +443,30 @@ func convertIgnition3to2(ign3config ign3types.Config) (ign2types.Config, error) return converted2, nil } +// convertIgnition34to33 takes an ignition spec v3.4config and returns a v3.3 config +func convertIgnition34to33(ign3config ign3types.Config) (ign3_3types.Config, error) { + converted33, err := v34tov33.Translate(ign3config) + if err != nil { + return ign3_3types.Config{}, fmt.Errorf("unable to convert Ignition spec v3_2 config to v3_1: %w", err) + } + klog.V(4).Infof("Successfully translated Ignition spec v3_2 config to Ignition spec v3_1 config: %v", converted33) + + return converted33, nil +} + +// convertIgnition33to32 takes an ignition spec v3.3config and returns a v3.2 config +func convertIgnition33to32(ign3config ign3_3types.Config) (ign3_2types.Config, error) { + converted32, err := v33tov32.Translate(ign3config) + if err != nil { + return ign3_2types.Config{}, fmt.Errorf("unable to convert Ignition spec v3_2 config to v3_1: %w", err) + } + klog.V(4).Infof("Successfully translated Ignition spec v3_2 config to Ignition spec v3_1 config: %v", converted32) + + return converted32, nil +} + // convertIgnition32to31 takes an ignition spec v3.2 config and returns a v3.1 config -func convertIgnition32to31(ign3config ign3types.Config) (ign3_1types.Config, error) { +func convertIgnition32to31(ign3config ign3_2types.Config) (ign3_1types.Config, error) { converted31, err := v32tov31.Translate(ign3config) if err != nil { return ign3_1types.Config{}, fmt.Errorf("unable to convert Ignition spec v3_2 config to v3_1: %w", err) @@ -579,7 +640,7 @@ func ParseAndConvertConfig(rawIgn []byte) (ign3types.Config, error) { if err != nil { return ign3types.Config{}, err } - convertedIgnV3, err := convertIgnition2to3(ignconfv2) + convertedIgnV3, err := convertIgnition22to34(ignconfv2) if err != nil { return ign3types.Config{}, fmt.Errorf("failed to convert Ignition config spec v2 to v3: %w", err) } @@ -783,7 +844,8 @@ func TranspileCoreOSConfigToIgn(files, units []string) (*ign3types.Config, error if err != nil { return nil, fmt.Errorf("failed to transpile config to Ignition config %w\nTranslation set: %v", err, tSet) } - ign3_2config := translate3.Translate(translate3_1.Translate(ign3_0config)) + // TODO(jkyros): do we keep just...adding translations forever as we add more versions? :) + ign3_2config := translate3.Translate(translate3_3.Translate(translate3_2.Translate(translate3_1.Translate(ign3_0config)))) outConfig = ign3.Merge(outConfig, ign3_2config) } @@ -800,7 +862,7 @@ func TranspileCoreOSConfigToIgn(files, units []string) (*ign3types.Config, error if err != nil { return nil, fmt.Errorf("failed to transpile config to Ignition config %w\nTranslation set: %v", err, tSet) } - ign3_2config := translate3.Translate(translate3_1.Translate(ign3_0config)) + ign3_2config := translate3.Translate(translate3_3.Translate(translate3_2.Translate(translate3_1.Translate(ign3_0config)))) outConfig = ign3.Merge(outConfig, ign3_2config) } diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 46ff61e1a6..4a368e47bd 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -7,10 +7,8 @@ import ( "github.com/clarketm/json" ign2types "github.com/coreos/ignition/config/v2_2/types" - ign3 "github.com/coreos/ignition/v2/config/v3_2" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" - ign3_4 "github.com/coreos/ignition/v2/config/v3_4" - ign3_4types "github.com/coreos/ignition/v2/config/v3_4/types" + ign3 "github.com/coreos/ignition/v2/config/v3_4" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" validate3 "github.com/coreos/ignition/v2/config/validate" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -104,7 +102,7 @@ func TestValidateIgnition(t *testing.T) { require.NotNil(t, isValid2) // Test that a valid ignition config returns nil - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion mode := 420 testfiledata := "data:,greatconfigstuff" tempFile := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig"}, @@ -141,7 +139,7 @@ func TestConvertIgnition2to3(t *testing.T) { isValid := ValidateIgnition(testIgn2Config) require.Nil(t, isValid) - convertedIgn, err := convertIgnition2to3(testIgn2Config) + convertedIgn, err := convertIgnition22to34(testIgn2Config) require.Nil(t, err) assert.IsType(t, ign3types.Config{}, convertedIgn) isValid3 := ValidateIgnition(convertedIgn) @@ -153,11 +151,11 @@ func TestConvertIgnition3to2(t *testing.T) { testIgn3Config := ign3types.Config{} tempUser := ign3types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"5678", "abc"}} testIgn3Config.Passwd.Users = []ign3types.PasswdUser{tempUser} - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = "3.4.0" isValid := ValidateIgnition(testIgn3Config) require.Nil(t, isValid) - convertedIgn, err := convertIgnition3to2(testIgn3Config) + convertedIgn, err := convertIgnition34to22(testIgn3Config) require.Nil(t, err) assert.IsType(t, ign2types.Config{}, convertedIgn) isValid2 := ValidateIgnition(convertedIgn) @@ -169,7 +167,7 @@ func TestParseAndConvert(t *testing.T) { testIgn3Config := ign3types.Config{} tempUser := ign3types.PasswdUser{Name: "core", SSHAuthorizedKeys: []ign3types.SSHAuthorizedKey{"5678", "abc"}} testIgn3Config.Passwd.Users = []ign3types.PasswdUser{tempUser} - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion // Make a Ign2 comp config testIgn2Config := ign2types.Config{} @@ -197,7 +195,7 @@ func TestParseAndConvert(t *testing.T) { assert.Equal(t, testIgn3Config, convertedIgn) // Make a valid Ign 3.2 cfg - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion // turn it into a raw []byte rawIgn = helpers.MarshalOrDie(testIgn3Config) // check that it was parsed successfully @@ -209,63 +207,40 @@ func TestParseAndConvert(t *testing.T) { testIgn3Config.Ignition.Version = "3.1.0" // turn it into a raw []byte rawIgn = helpers.MarshalOrDie(testIgn3Config) - // check that it was parsed successfully back to 3.2 + // check that it was parsed successfully back to the default version convertedIgn, err = ParseAndConvertConfig(rawIgn) require.Nil(t, err) - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion assert.Equal(t, testIgn3Config, convertedIgn) // Make a valid Ign 3.0 cfg testIgn3Config.Ignition.Version = "3.0.0" // turn it into a raw []byte rawIgn = helpers.MarshalOrDie(testIgn3Config) - // check that it was parsed successfully back to 3.2 + // check that it was parsed successfully back to the default version convertedIgn, err = ParseAndConvertConfig(rawIgn) require.Nil(t, err) - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion assert.Equal(t, testIgn3Config, convertedIgn) // Make a valid Ign 3.3 cfg testIgn3Config.Ignition.Version = "3.3.0" // turn it into a raw []byte rawIgn = helpers.MarshalOrDie(testIgn3Config) - // check that it was parsed successfully back to 3.2 + // check that it was parsed successfully back to the default version convertedIgn, err = ParseAndConvertConfig(rawIgn) require.Nil(t, err) - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion assert.Equal(t, testIgn3Config, convertedIgn) // Make a valid Ign 3.4 cfg testIgn3Config.Ignition.Version = "3.4.0" // turn it into a raw []byte rawIgn = helpers.MarshalOrDie(testIgn3Config) - // check that it was parsed successfully back to 3.2 + // check that it was parsed successfully back to the default version convertedIgn, err = ParseAndConvertConfig(rawIgn) require.Nil(t, err) - testIgn3Config.Ignition.Version = "3.2.0" - assert.Equal(t, testIgn3Config, convertedIgn) - - // Make a an Ign 3.4 cfg with kargs - testIgn3Config.Ignition.Version = "3.4.0" - - var ign34 ign3_4types.Config - // Parse this up to 3.4 specifically so we can test a downgrade that - // is using kargs - ign34, _, err = ign3_4.ParseCompatibleVersion(rawIgn) - require.Nil(t, err) - ign34.KernelArguments = ign3_4types.KernelArguments{ - ShouldExist: []ign3_4types.KernelArgument{"one", "two", "three"}, - ShouldNotExist: []ign3_4types.KernelArgument{"four", "five", "six"}, - } - // turn it into a raw []byte - rawIgn = helpers.MarshalOrDie(ign34) - - // check that it was parsed successfully back to 3.2 - // this should strip out the kernel args - convertedIgn, err = ParseAndConvertConfig(rawIgn) - require.Nil(t, err) - testIgn3Config.Ignition.Version = "3.2.0" - // we compare to testign3Config because kargs should get stripped out + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion assert.Equal(t, testIgn3Config, convertedIgn) // Make a bad Ign3 cfg @@ -368,17 +343,6 @@ func TestMergeMachineConfigs(t *testing.T) { }, }) - // TODO(jkyros): remove this when we raise the ignition default to 3.4 - machineConfigIgnKernelArgsDowngrade := helpers.CreateMachineConfigFromIgnition(ign3_4types.Config{ - Ignition: ign3_4types.Ignition{ - Version: ign3_4types.MaxVersion.String(), - }, - KernelArguments: ign3_4types.KernelArguments{ - ShouldExist: []ign3_4types.KernelArgument{"kargFromIgnitionDowngrade"}, - ShouldNotExist: []ign3_4types.KernelArgument{}, - }, - }) - // we added some v3 specific logic for kargs, make sure we didn't break the v2 path machineConfigIgnV2Merge := helpers.CreateMachineConfigFromIgnition(ign2types.Config{ Ignition: ign2types.Ignition{ @@ -407,7 +371,6 @@ func TestMergeMachineConfigs(t *testing.T) { machineConfigFIPS, machineConfigIgnPasswdHashUser, machineConfigIgnSSHUser, - machineConfigIgnKernelArgsDowngrade, machineConfigIgnV2Merge, } @@ -563,12 +526,12 @@ func TestSetDefaultFileOverwrite(t *testing.T) { // Set up two Ignition configs, one with overwrite: no default, overwrite: false (to be passed to MergeMachineConfigs) // and one with a overwrite: true, overwrite: false (the expected output) testIgn3ConfigPreMerge := ign3types.Config{} - testIgn3ConfigPreMerge.Ignition.Version = "3.2.0" + testIgn3ConfigPreMerge.Ignition.Version = InternalMCOIgnitionVersion testIgn3ConfigPreMerge.Storage.Files = append(testIgn3ConfigPreMerge.Storage.Files, tempFileNoDefault) testIgn3ConfigPreMerge.Storage.Files = append(testIgn3ConfigPreMerge.Storage.Files, tempFileOverwriteFalse) testIgn3ConfigPostMerge := ign3types.Config{} - testIgn3ConfigPostMerge.Ignition.Version = "3.2.0" + testIgn3ConfigPostMerge.Ignition.Version = InternalMCOIgnitionVersion testIgn3ConfigPostMerge.Storage.Files = append(testIgn3ConfigPostMerge.Storage.Files, tempFileOvewriteTrue) testIgn3ConfigPostMerge.Storage.Files = append(testIgn3ConfigPostMerge.Storage.Files, tempFileOverwriteFalse) @@ -606,7 +569,7 @@ func TestSetDefaultFileOverwrite(t *testing.T) { // TestIgnitionMergeCompressed tests https://github.com/coreos/butane/issues/332 func TestIgnitionMergeCompressed(t *testing.T) { testIgn3Config := ign3types.Config{} - testIgn3Config.Ignition.Version = "3.2.0" + testIgn3Config.Ignition.Version = InternalMCOIgnitionVersion mode := 420 testfiledata := "data:;base64,H4sIAAAAAAAAA0vLz+cCAKhlMn4EAAAA" compression := "gzip" @@ -615,7 +578,7 @@ func TestIgnitionMergeCompressed(t *testing.T) { testIgn3Config.Storage.Files = append(testIgn3Config.Storage.Files, tempFile) testIgn3Config2 := ign3types.Config{} - testIgn3Config2.Ignition.Version = "3.2.0" + testIgn3Config2.Ignition.Version = InternalMCOIgnitionVersion testIgn3Config2.Storage.Files = append(testIgn3Config2.Storage.Files, NewIgnFile("/etc/testfileconfig", "hello world")) merged := ign3.Merge(testIgn3Config, testIgn3Config2) @@ -634,11 +597,11 @@ func TestCalculateConfigFileDiffs(t *testing.T) { newTempFile := NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "newcertificates") // Make an "old" config with the existing file in it - testIgn3ConfigOld.Ignition.Version = "3.2.0" + testIgn3ConfigOld.Ignition.Version = InternalMCOIgnitionVersion testIgn3ConfigOld.Storage.Files = append(testIgn3ConfigOld.Storage.Files, oldTempFile) // Make a "new" config with a change to that file - testIgn3ConfigNew.Ignition.Version = "3.2.0" + testIgn3ConfigNew.Ignition.Version = InternalMCOIgnitionVersion testIgn3ConfigNew.Storage.Files = append(testIgn3ConfigNew.Storage.Files, newTempFile) // If it works, it should notice the file changed diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index ea7f8570c7..2ffdcacdac 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -9,7 +9,7 @@ import ( "time" "github.com/clarketm/json" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" configclientset "github.com/openshift/client-go/config/clientset/versioned" diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index 7b2218027b..6193c05dc9 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -25,7 +25,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" fakeconfigv1client "github.com/openshift/client-go/config/clientset/versioned/fake" diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 58c632cab8..e8f5a4ad49 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -17,7 +17,7 @@ import ( signature "github.com/containers/image/v5/signature" "github.com/containers/image/v5/types" storageconfig "github.com/containers/storage/pkg/config" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/runtime-utils/pkg/registries" diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index b76f4ff868..dbf2c825ac 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/pkg/controller/kubelet-config/helpers.go @@ -9,7 +9,7 @@ import ( "strconv" "strings" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/imdario/mergo" osev1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index f75e4315da..c8ff8db2de 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -10,7 +10,7 @@ import ( "time" "github.com/clarketm/json" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/imdario/mergo" corev1 "k8s.io/api/core/v1" macherrors "k8s.io/apimachinery/pkg/api/errors" diff --git a/pkg/controller/kubelet-config/kubelet_config_controller_test.go b/pkg/controller/kubelet-config/kubelet_config_controller_test.go index 495aa15c36..082cd9e1d2 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller_test.go @@ -6,7 +6,7 @@ import ( "reflect" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" osev1 "github.com/openshift/api/config/v1" oseconfigfake "github.com/openshift/client-go/config/clientset/versioned/fake" oseinformersv1 "github.com/openshift/client-go/config/informers/externalversions" diff --git a/pkg/controller/kubelet-config/kubelet_config_features_test.go b/pkg/controller/kubelet-config/kubelet_config_features_test.go index 28869417ce..4bd5bc89bf 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_features_test.go @@ -5,7 +5,7 @@ import ( "reflect" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" configv1 "github.com/openshift/api/config/v1" osev1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" diff --git a/pkg/controller/kubelet-config/kubelet_config_nodes_test.go b/pkg/controller/kubelet-config/kubelet_config_nodes_test.go index a8c1d7d17e..3f910e118e 100644 --- a/pkg/controller/kubelet-config/kubelet_config_nodes_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_nodes_test.go @@ -6,7 +6,7 @@ import ( "reflect" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" configv1 "github.com/openshift/api/config/v1" osev1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index b080956350..235a3fd320 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -7,7 +7,7 @@ import ( "time" "github.com/clarketm/json" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" configv1 "github.com/openshift/api/config/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go index 39e477db6e..f63f286a43 100644 --- a/pkg/controller/template/render_test.go +++ b/pkg/controller/template/render_test.go @@ -10,7 +10,7 @@ import ( "strings" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/cloudprovider" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" diff --git a/pkg/daemon/config_drift_monitor.go b/pkg/daemon/config_drift_monitor.go index 258ff0d04c..9b287c81a9 100644 --- a/pkg/daemon/config_drift_monitor.go +++ b/pkg/daemon/config_drift_monitor.go @@ -8,7 +8,7 @@ import ( "sync" ign2types "github.com/coreos/ignition/config/v2_2/types" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/fsnotify/fsnotify" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" diff --git a/pkg/daemon/config_drift_monitor_test.go b/pkg/daemon/config_drift_monitor_test.go index 18adf65309..c8a9fb1994 100644 --- a/pkg/daemon/config_drift_monitor_test.go +++ b/pkg/daemon/config_drift_monitor_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/test/helpers" "github.com/stretchr/testify/assert" diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index c367fa03ce..8247a123e0 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -17,7 +17,7 @@ import ( "syscall" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/google/renameio" "golang.org/x/time/rate" corev1 "k8s.io/api/core/v1" diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index 8dcb8b4efb..caab5ae39b 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -9,7 +9,7 @@ import ( "time" ign2types "github.com/coreos/ignition/config/v2_2/types" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/stretchr/testify/require" "github.com/vincent-petithory/dataurl" corev1 "k8s.io/api/core/v1" diff --git a/pkg/daemon/drain.go b/pkg/daemon/drain.go index 834d0635b0..238b02d64f 100644 --- a/pkg/daemon/drain.go +++ b/pkg/daemon/drain.go @@ -8,7 +8,7 @@ import ( "github.com/BurntSushi/toml" "github.com/containers/image/v5/pkg/sysregistriesv2" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/daemon/constants" corev1 "k8s.io/api/core/v1" diff --git a/pkg/daemon/drain_test.go b/pkg/daemon/drain_test.go index ff84eb668c..1b87a95e25 100644 --- a/pkg/daemon/drain_test.go +++ b/pkg/daemon/drain_test.go @@ -5,7 +5,7 @@ import ( "reflect" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/test/helpers" diff --git a/pkg/daemon/file_writers.go b/pkg/daemon/file_writers.go index c116ae8297..017d61c7a0 100644 --- a/pkg/daemon/file_writers.go +++ b/pkg/daemon/file_writers.go @@ -9,7 +9,7 @@ import ( "path/filepath" "strconv" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/google/renameio" "k8s.io/klog/v2" diff --git a/pkg/daemon/on_disk_validation.go b/pkg/daemon/on_disk_validation.go index 365ce79bd7..c094280d3b 100644 --- a/pkg/daemon/on_disk_validation.go +++ b/pkg/daemon/on_disk_validation.go @@ -7,7 +7,7 @@ import ( "path/filepath" ign2types "github.com/coreos/ignition/config/v2_2/types" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/google/go-cmp/cmp" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 7c56ff7ab6..53b2eb36e8 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -16,7 +16,7 @@ import ( "time" "github.com/clarketm/json" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -756,6 +756,13 @@ func reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*machineConfigDif } } + // Kernel args + + // ignition now supports kernel args, but the MCO doesn't implement them yet + if !reflect.DeepEqual(oldIgn.KernelArguments, newIgn.KernelArguments) { + return nil, fmt.Errorf("ignition kargs section contains changes") + } + // Storage section // we can only reconcile files right now. make sure the sections we can't diff --git a/pkg/daemon/update_test.go b/pkg/daemon/update_test.go index 1af78bbdbd..7bda012603 100644 --- a/pkg/daemon/update_test.go +++ b/pkg/daemon/update_test.go @@ -13,7 +13,7 @@ import ( "testing" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/daemon/osrelease" @@ -136,10 +136,11 @@ func TestReconcilable(t *testing.T) { checkReconcilableResults(t, "Filesystem", isReconcilable) // Verify Raid changes react as expected + var stripe = "stripe" oldIgnCfg.Storage.Raid = []ign3types.Raid{ { Name: "data", - Level: "stripe", + Level: &stripe, Devices: []ign3types.Device{"/dev/vda", "/dev/vdb"}, }, } @@ -168,6 +169,43 @@ func TestReconcilable(t *testing.T) { newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "PasswdGroups", isReconcilable) + + // Verify Ignition kernelArguments changes unsupported + oldIgnCfg = ctrlcommon.NewIgnConfig() + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + newIgnCfg = ctrlcommon.NewIgnConfig() + newIgnCfg.KernelArguments.ShouldExist = []ign3types.KernelArgument{"foo=bar"} + newIgnCfg.KernelArguments.ShouldNotExist = []ign3types.KernelArgument{"baz=foo"} + + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) + + _, isReconcilable = reconcilable(oldConfig, newConfig) + checkIrreconcilableResults(t, "KernelArguments", isReconcilable) + + // Verify Tang changes are supported (even though we don't do anything with them yet) + oldIgnCfg = ctrlcommon.NewIgnConfig() + oldIgnCfg.Storage.Luks = []ign3types.Luks{ + { + Clevis: ign3types.Clevis{ + Custom: ign3types.ClevisCustom{}, + Tang: []ign3types.Tang{ + { + URL: "https://tang.example.com", + Advertisement: helpers.StrToPtr(`{"payload": "...", "protected": "...", "signature": "..."}`), + Thumbprint: helpers.StrToPtr("TREPLACE-THIS-WITH-YOUR-TANG-THUMBPRINT"), + }, + }, + }, + Device: helpers.StrToPtr("/dev/sdb"), + Name: "luks-tang", + }, + } + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + newIgnCfg = ctrlcommon.NewIgnConfig() + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) + + _, isReconcilable = reconcilable(oldConfig, newConfig) + checkReconcilableResults(t, "LuksClevisTang", isReconcilable) } func TestMachineConfigDiff(t *testing.T) { diff --git a/pkg/server/api.go b/pkg/server/api.go index 217acd2606..ac5030f54e 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -169,7 +169,7 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { serveConf = &converted31 } else { // Can only be 2.2 here - converted2, err := ctrlcommon.ConvertRawExtIgnitionToV2(conf) + converted2, err := ctrlcommon.ConvertRawExtIgnitionToV2_2(conf) if err != nil { w.Header().Set("Content-Length", "0") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/server/server.go b/pkg/server/server.go index 2f7cd1925e..b3038505c6 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -7,7 +7,7 @@ import ( "github.com/clarketm/json" "github.com/coreos/go-semver/semver" ign2types "github.com/coreos/ignition/config/v2_2/types" - igntypes "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/vincent-petithory/dataurl" "k8s.io/apimachinery/pkg/runtime" @@ -35,7 +35,7 @@ const ( type kubeconfigFunc func() (kubeconfigData []byte, rootCAData []byte, err error) // appenderFunc appends Config. -type appenderFunc func(*igntypes.Config, *mcfgv1.MachineConfig) error +type appenderFunc func(*ign3types.Config, *mcfgv1.MachineConfig) error // Server defines the interface that is implemented by different // machine config server implementations. @@ -46,15 +46,15 @@ type Server interface { func getAppenders(currMachineConfig string, version *semver.Version, f kubeconfigFunc) []appenderFunc { appenders := []appenderFunc{ // append machine annotations file. - func(cfg *igntypes.Config, mc *mcfgv1.MachineConfig) error { + func(cfg *ign3types.Config, mc *mcfgv1.MachineConfig) error { return appendNodeAnnotations(cfg, currMachineConfig) }, // append kubeconfig. - func(cfg *igntypes.Config, mc *mcfgv1.MachineConfig) error { return appendKubeConfig(cfg, f) }, + func(cfg *ign3types.Config, mc *mcfgv1.MachineConfig) error { return appendKubeConfig(cfg, f) }, // append the machineconfig content appendInitialMachineConfig, // This has to come last!!! - func(cfg *igntypes.Config, mc *mcfgv1.MachineConfig) error { + func(cfg *ign3types.Config, mc *mcfgv1.MachineConfig) error { return appendEncapsulated(cfg, mc, version) }, } @@ -65,7 +65,7 @@ func getAppenders(currMachineConfig string, version *semver.Version, f kubeconfi // it to /etc/ignition-machine-config-encapsulated.json. This is used by // machine-config-daemon-firstboot.service to process the bits that the main Ignition (that runs in the initramfs) // didn't handle such as kernel arguments. -func appendEncapsulated(conf *igntypes.Config, mc *mcfgv1.MachineConfig, version *semver.Version) error { +func appendEncapsulated(conf *ign3types.Config, mc *mcfgv1.MachineConfig, version *semver.Version) error { var rawTmpIgnCfg []byte var err error // In order to handle old RHCOS versions with the MCD baked in (i.e. before the MCS has @@ -109,7 +109,7 @@ func appendEncapsulated(conf *igntypes.Config, mc *mcfgv1.MachineConfig, version // by the MCS when the node first booted. This currently is only used as a debugging aid // in cases where there is unexpected "drift" between the initial bootstrap MC/Ignition and the one // computed by the cluster. -func appendInitialMachineConfig(conf *igntypes.Config, mc *mcfgv1.MachineConfig) error { +func appendInitialMachineConfig(conf *ign3types.Config, mc *mcfgv1.MachineConfig) error { mcJSON, err := json.MarshalIndent(mc, "", " ") if err != nil { return err @@ -118,7 +118,7 @@ func appendInitialMachineConfig(conf *igntypes.Config, mc *mcfgv1.MachineConfig) return nil } -func appendKubeConfig(conf *igntypes.Config, f kubeconfigFunc) error { +func appendKubeConfig(conf *ign3types.Config, f kubeconfigFunc) error { kcData, _, err := f() if err != nil { return err @@ -130,7 +130,7 @@ func appendKubeConfig(conf *igntypes.Config, f kubeconfigFunc) error { return nil } -func appendNodeAnnotations(conf *igntypes.Config, currConf string) error { +func appendNodeAnnotations(conf *ign3types.Config, currConf string) error { anno, err := getNodeAnnotation(currConf) if err != nil { return err @@ -155,24 +155,24 @@ func getNodeAnnotation(conf string) (string, error) { return string(contents), nil } -func appendFileToIgnition(conf *igntypes.Config, outPath, contents string) error { +func appendFileToIgnition(conf *ign3types.Config, outPath, contents string) error { fileMode := int(420) overwrite := true source := getEncodedContent(contents) - file := igntypes.File{ - Node: igntypes.Node{ + file := ign3types.File{ + Node: ign3types.Node{ Path: outPath, Overwrite: &overwrite, }, - FileEmbedded1: igntypes.FileEmbedded1{ - Contents: igntypes.Resource{ + FileEmbedded1: ign3types.FileEmbedded1{ + Contents: ign3types.Resource{ Source: &source, }, Mode: &fileMode, }, } if len(conf.Storage.Files) == 0 { - conf.Storage.Files = make([]igntypes.File, 0) + conf.Storage.Files = make([]ign3types.File, 0) } conf.Storage.Files = append(conf.Storage.Files, file) return nil diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index a3735afccb..42fc521f68 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -13,9 +13,8 @@ import ( "github.com/coreos/go-semver/semver" ign2 "github.com/coreos/ignition/config/v2_2" - ign3 "github.com/coreos/ignition/v2/config/v3_2" - ign3_1 "github.com/coreos/ignition/v2/config/v3_2" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3 "github.com/coreos/ignition/v2/config/v3_4" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" yaml "github.com/ghodss/yaml" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -79,7 +78,6 @@ func TestEncapsulated(t *testing.T) { t.Logf("vers: %v\n", vers) for _, v := range vers { major := v.Slice()[0] - minor := v.Slice()[1] mcIgnCfg, err = ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) assert.Nil(t, err) err = appendEncapsulated(&mcIgnCfg, mc, v) @@ -102,22 +100,11 @@ func TestEncapsulated(t *testing.T) { var mc mcfgv1.MachineConfig err = json.Unmarshal(encapData, &mc) assert.Nil(t, err) - if major == 3 && minor == 4 { - // TODO(jkyros): this parses to 3.2 now because it's still the default and we're down-translating, - // but we will need to change this when we move the default to a later version + // TODO(jkyros): the encap only supplies what the current internal version is, and 3.1 was able to parse a 3.2 config + // because it's weird so we should probably revisit whether it's okay if the encap is now supplying 3.4 + if major == 3 { _, _, err := ign3.Parse(mc.Spec.Config.Raw) assert.Nil(t, err) - } else if major == 3 && minor == 3 { - // TODO(jkyros): this parses to 3.2 now because it's still the default and we're down-translating, - // but we will need to change this when we move the default to a later version - _, _, err := ign3.Parse(mc.Spec.Config.Raw) - assert.Nil(t, err) - } else if major == 3 && minor == 2 { - _, _, err := ign3.Parse(mc.Spec.Config.Raw) - assert.Nil(t, err) - } else if major == 3 && minor == 1 { - _, _, err := ign3_1.Parse(mc.Spec.Config.Raw) - assert.Nil(t, err) } else { _, _, err := ign2.Parse(mc.Spec.Config.Raw) assert.Nil(t, err) diff --git a/test/e2e-shared-tests/mcd_config_drift.go b/test/e2e-shared-tests/mcd_config_drift.go index 7cadf92c6e..2d2f48e727 100644 --- a/test/e2e-shared-tests/mcd_config_drift.go +++ b/test/e2e-shared-tests/mcd_config_drift.go @@ -8,7 +8,7 @@ import ( "testing" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/test/framework" diff --git a/test/e2e-single-node/sno_mcd_test.go b/test/e2e-single-node/sno_mcd_test.go index 0141746652..48fdb67d00 100644 --- a/test/e2e-single-node/sno_mcd_test.go +++ b/test/e2e-single-node/sno_mcd_test.go @@ -21,7 +21,7 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" corev1 "k8s.io/api/core/v1" diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index 7d073c6dad..d8709876b8 100644 --- a/test/e2e/mcd_test.go +++ b/test/e2e/mcd_test.go @@ -11,7 +11,7 @@ import ( "testing" "time" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" diff --git a/test/e2e/osimageurl_override_test.go b/test/e2e/osimageurl_override_test.go index efde1ad98e..99f16937e7 100644 --- a/test/e2e/osimageurl_override_test.go +++ b/test/e2e/osimageurl_override_test.go @@ -5,7 +5,7 @@ import ( "os" "testing" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/openshift/machine-config-operator/test/framework" "github.com/openshift/machine-config-operator/test/helpers" "github.com/stretchr/testify/assert" diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go index 3fe721cfb8..d35811b1ca 100644 --- a/test/helpers/helpers.go +++ b/test/helpers/helpers.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/clarketm/json" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" diff --git a/test/helpers/utils.go b/test/helpers/utils.go index ef4f011fbb..e63079277e 100644 --- a/test/helpers/utils.go +++ b/test/helpers/utils.go @@ -17,7 +17,7 @@ import ( authenticationv1 "k8s.io/api/authentication/v1" - ign3types "github.com/coreos/ignition/v2/config/v3_2/types" + ign3types "github.com/coreos/ignition/v2/config/v3_4/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/pkg/daemon/osrelease" From 126c16427421dae8982f9b5781391c0254dca376 Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Wed, 26 Jul 2023 20:53:54 -0500 Subject: [PATCH 4/4] remove kargs when mcs downconverts --- pkg/server/bootstrap_server.go | 5 +++++ pkg/server/cluster_server.go | 5 +++++ pkg/server/server.go | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/pkg/server/bootstrap_server.go b/pkg/server/bootstrap_server.go index c8fb0b0bd5..4756a6865d 100644 --- a/pkg/server/bootstrap_server.go +++ b/pkg/server/bootstrap_server.go @@ -102,6 +102,11 @@ func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*runtime.RawExtension, er return nil, fmt.Errorf("parsing Ignition config failed with error: %w", err) } + // strip the kargs out if we're going back to a version that doesn't support it + if err := MigrateKernelArgsIfNecessary(&ignConf, mc, cr.version); err != nil { + return nil, fmt.Errorf("failed to migrate kernel args %w", err) + } + appenders := getAppenders(currConf, nil, bsc.kubeconfigFunc) for _, a := range appenders { if err := a(&ignConf, mc); err != nil { diff --git a/pkg/server/cluster_server.go b/pkg/server/cluster_server.go index c8c95ddc20..7e5e656a98 100644 --- a/pkg/server/cluster_server.go +++ b/pkg/server/cluster_server.go @@ -136,6 +136,11 @@ func (cs *clusterServer) GetConfig(cr poolRequest) (*runtime.RawExtension, error } } + // strip the kargs out if we're going back to a version that doesn't support it + if err := MigrateKernelArgsIfNecessary(&ignConf, mc, cr.version); err != nil { + return nil, fmt.Errorf("failed to migrate kernel args %w", err) + } + appenders := getAppenders(currConf, cr.version, cs.kubeconfigFunc) for _, a := range appenders { if err := a(&ignConf, mc); err != nil { diff --git a/pkg/server/server.go b/pkg/server/server.go index b3038505c6..927244f6f0 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -94,6 +94,7 @@ func appendEncapsulated(conf *ign3types.Config, mc *mcfgv1.MachineConfig, versio tmpcfg := mc.DeepCopy() tmpcfg.Spec.Config.Raw = rawTmpIgnCfg + serialized, err := json.Marshal(tmpcfg) if err != nil { return fmt.Errorf("error marshalling MachineConfig: %w", err) @@ -184,3 +185,27 @@ func getEncodedContent(inp string) string { Opaque: "," + dataurl.Escape([]byte(inp)), }).String() } + +// MigrateKernelArgsIfNecessary moves the kernel arguments back into MachineConfig when going from a version that supports +// ignition kernel arguments to a version that does not. Without this, we would be unable to serve anything < 3.3 because the +// ignition converter fails to downconvert if unsupported fields are populated. If ShouldNotExist is populated in the +// ignition kernel args, we still have to fail because there is nowhere in MachineConfig for us to put those. +func MigrateKernelArgsIfNecessary(conf *ign3types.Config, mc *mcfgv1.MachineConfig, version *semver.Version) error { + // If we're downgrading from a version of ignition that + // support kargs, we need to stuff them back into MachineConfig so they still end up in the encapsulation + // and they still get applied + if version != nil && version.LessThan(*semver.New("3.3.0")) { + // we're converting to a version that doesn't support kargs, so we need to stuff them in machineconfig + if len(conf.KernelArguments.ShouldNotExist) > 0 { + return fmt.Errorf("Can't serve version %s with ignition KernelArguments.ShouldNotExist populated", version) + } + + for _, karg := range conf.KernelArguments.ShouldExist { + // TODO(jkyros): we probably need to parse/split them because they might be combined + mc.Spec.KernelArguments = append(mc.Spec.KernelArguments, string(karg)) + } + // and then we take them out of ignition + conf.KernelArguments = ign3types.KernelArguments{} + } + return nil +}