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 1ad0e2eb82..1dd4ec1a39 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -30,16 +30,17 @@ import ( 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" - 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" - ign3_4types "github.com/coreos/ignition/v2/config/v3_4/types" + 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" @@ -146,23 +147,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...) @@ -263,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) } @@ -297,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 } @@ -344,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 } @@ -354,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 } @@ -372,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) } @@ -395,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": "/", @@ -407,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) } @@ -425,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) @@ -552,66 +592,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 @@ -653,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) } @@ -857,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) } @@ -874,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 646d753731..ac5030f54e 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 { @@ -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/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 2f7cd1925e..927244f6f0 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 @@ -94,6 +94,7 @@ func appendEncapsulated(conf *igntypes.Config, mc *mcfgv1.MachineConfig, version tmpcfg := mc.DeepCopy() tmpcfg.Spec.Config.Raw = rawTmpIgnCfg + serialized, err := json.Marshal(tmpcfg) if err != nil { return fmt.Errorf("error marshalling MachineConfig: %w", err) @@ -109,7 +110,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 +119,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 +131,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 +156,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 @@ -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 +} 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"