diff --git a/lib/resourceapply/machineconfig_test.go b/lib/resourceapply/machineconfig_test.go index 6f72300a0d..9feb0b59f8 100644 --- a/lib/resourceapply/machineconfig_test.go +++ b/lib/resourceapply/machineconfig_test.go @@ -8,6 +8,7 @@ import ( "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" + "github.com/openshift/machine-config-operator/test/helpers" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -175,12 +176,14 @@ func TestApplyMachineConfig(t *testing.T) { input: &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: mcfgv1.MachineConfigSpec{ - Config: igntypes.Config{ - Passwd: igntypes.Passwd{ - Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", - }}, - }, + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(&igntypes.Config{ + Passwd: igntypes.Passwd{ + Users: []igntypes.PasswdUser{{ + HomeDir: "/home/dummy", + }}, + }, + }), }, }, }, @@ -199,12 +202,14 @@ func TestApplyMachineConfig(t *testing.T) { expected := &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{"extra": "leave-alone"}}, Spec: mcfgv1.MachineConfigSpec{ - Config: igntypes.Config{ - Passwd: igntypes.Passwd{ - Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", - }}, - }, + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(&igntypes.Config{ + Passwd: igntypes.Passwd{ + Users: []igntypes.PasswdUser{{ + HomeDir: "/home/dummy", + }}, + }, + }), }, }, } @@ -218,12 +223,14 @@ func TestApplyMachineConfig(t *testing.T) { &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{"extra": "leave-alone"}}, Spec: mcfgv1.MachineConfigSpec{ - Config: igntypes.Config{ - Passwd: igntypes.Passwd{ - Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy-prev", - }}, - }, + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(&igntypes.Config{ + Passwd: igntypes.Passwd{ + Users: []igntypes.PasswdUser{{ + HomeDir: "/home/dummy-prev", + }}, + }, + }), }, }, }, @@ -231,12 +238,14 @@ func TestApplyMachineConfig(t *testing.T) { input: &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: mcfgv1.MachineConfigSpec{ - Config: igntypes.Config{ - Passwd: igntypes.Passwd{ - Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", - }}, - }, + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(&igntypes.Config{ + Passwd: igntypes.Passwd{ + Users: []igntypes.PasswdUser{{ + HomeDir: "/home/dummy", + }}, + }, + }), }, }, }, @@ -255,12 +264,14 @@ func TestApplyMachineConfig(t *testing.T) { expected := &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{"extra": "leave-alone"}}, Spec: mcfgv1.MachineConfigSpec{ - Config: igntypes.Config{ - Passwd: igntypes.Passwd{ - Users: []igntypes.PasswdUser{{ - HomeDir: "/home/dummy", - }}, - }, + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(&igntypes.Config{ + Passwd: igntypes.Passwd{ + Users: []igntypes.PasswdUser{{ + HomeDir: "/home/dummy", + }}, + }, + }), }, }, } diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go b/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go index dbab8ec7f9..c32d1db957 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go @@ -2,67 +2,11 @@ package v1 import ( "fmt" - "sort" - ign "github.com/coreos/ignition/config/v2_2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// MergeMachineConfigs combines multiple machineconfig objects into one object. -// It sorts all the configs in increasing order of their name. -// It uses the Ignition config from first object as base and appends all the rest. -// Kernel arguments are concatenated. -// It uses only the OSImageURL provided by the CVO and ignores any MC provided OSImageURL. -func MergeMachineConfigs(configs []*MachineConfig, osImageURL string) *MachineConfig { - if len(configs) == 0 { - return nil - } - sort.Slice(configs, func(i, j int) bool { return configs[i].Name < configs[j].Name }) - - var fips bool - var kernelType string - outIgn := configs[0].Spec.Config - for idx := 1; idx < len(configs); idx++ { - // if any of the config has FIPS enabled, it'll be set - if configs[idx].Spec.FIPS { - fips = true - } - outIgn = ign.Append(outIgn, configs[idx].Spec.Config) - } - - // sets the KernelType if specified in any of the MachineConfig - // Setting kerneType to realtime in any of MachineConfig takes priority - for _, cfg := range configs { - if cfg.Spec.KernelType == "realtime" { - kernelType = cfg.Spec.KernelType - break - } else if kernelType == "default" { - kernelType = cfg.Spec.KernelType - } - } - - // If no MC sets kerneType, then set it to 'default' since that's what it is using - if kernelType == "" { - kernelType = "default" - } - - kargs := []string{} - for _, cfg := range configs { - kargs = append(kargs, cfg.Spec.KernelArguments...) - } - - return &MachineConfig{ - Spec: MachineConfigSpec{ - OSImageURL: osImageURL, - KernelArguments: kargs, - Config: outIgn, - FIPS: fips, - KernelType: kernelType, - }, - } -} - // NewMachineConfigPoolCondition creates a new MachineConfigPool condition. func NewMachineConfigPoolCondition(condType MachineConfigPoolConditionType, status corev1.ConditionStatus, reason, message string) *MachineConfigPoolCondition { return &MachineConfigPoolCondition{ diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/machineconfig.deepcopy.go b/pkg/apis/machineconfiguration.openshift.io/v1/machineconfig.deepcopy.go deleted file mode 100644 index 07b6854c9f..0000000000 --- a/pkg/apis/machineconfiguration.openshift.io/v1/machineconfig.deepcopy.go +++ /dev/null @@ -1,57 +0,0 @@ -package v1 - -import ( - ign "github.com/coreos/ignition/config/v2_2" - igntypes "github.com/coreos/ignition/config/v2_2/types" - runtime "k8s.io/apimachinery/pkg/runtime" -) - -// DeepCopyInto copying the receiver, writing into out. in must be non-nil. -func (in *MachineConfig) DeepCopyInto(out *MachineConfig) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - return -} - -// DeepCopy copying the receiver, creating a new MachineConfig. -func (in *MachineConfig) DeepCopy() *MachineConfig { - if in == nil { - return nil - } - out := new(MachineConfig) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject copying the receiver, creating a new runtime.Object. -func (in *MachineConfig) DeepCopyObject() runtime.Object { - return in.DeepCopy() -} - -// DeepCopyInto copying the receiver, writing into out. in must be non-nil. -func (in *MachineConfigSpec) DeepCopyInto(out *MachineConfigSpec) { - *out = *in - out.Config = deepCopyIgnConfig(in.Config) - return -} - -func deepCopyIgnConfig(in igntypes.Config) igntypes.Config { - var out igntypes.Config - - // https://github.com/coreos/ignition/blob/d19b2021cf397de7c31774c13805bbc3aa655646/config/v2_2/append.go#L41 - out.Ignition.Version = in.Ignition.Version - - return ign.Append(out, in) -} - -// DeepCopy copying the receiver, creating a new MachineConfigSpec. -func (in *MachineConfigSpec) DeepCopy() *MachineConfigSpec { - if in == nil { - return nil - } - out := new(MachineConfigSpec) - in.DeepCopyInto(out) - return out -} diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/types.go b/pkg/apis/machineconfiguration.openshift.io/v1/types.go index b90d6f94b2..4f623880c6 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/types.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/types.go @@ -1,7 +1,6 @@ package v1 import ( - igntypes "github.com/coreos/ignition/config/v2_2/types" configv1 "github.com/openshift/api/config/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -153,7 +152,7 @@ type ControllerConfigList struct { // +genclient // +genclient:noStatus // +genclient:nonNamespaced -// +k8s:deepcopy-gen=false +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // MachineConfig defines the configuration for a machine type MachineConfig struct { @@ -169,7 +168,7 @@ type MachineConfigSpec struct { // fetch the OS. OSImageURL string `json:"osImageURL"` // Config is a Ignition Config object. - Config igntypes.Config `json:"config"` + Config runtime.RawExtension `json:"config"` // +nullable KernelArguments []string `json:"kernelArguments"` diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go b/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go index e11796a90c..14e903d60d 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go @@ -453,6 +453,33 @@ func (in *KubeletConfigStatus) DeepCopy() *KubeletConfigStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineConfig) DeepCopyInto(out *MachineConfig) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineConfig. +func (in *MachineConfig) DeepCopy() *MachineConfig { + if in == nil { + return nil + } + out := new(MachineConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *MachineConfig) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineConfigList) DeepCopyInto(out *MachineConfigList) { *out = *in @@ -641,3 +668,25 @@ func (in *MachineConfigPoolStatusConfiguration) DeepCopy() *MachineConfigPoolSta in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineConfigSpec) DeepCopyInto(out *MachineConfigSpec) { + *out = *in + in.Config.DeepCopyInto(&out.Config) + if in.KernelArguments != nil { + in, out := &in.KernelArguments, &out.KernelArguments + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineConfigSpec. +func (in *MachineConfigSpec) DeepCopy() *MachineConfigSpec { + if in == nil { + return nil + } + out := new(MachineConfigSpec) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/controller/bootstrap/bootstrap_test.go b/pkg/controller/bootstrap/bootstrap_test.go index d03b87f63f..159a28b1fc 100644 --- a/pkg/controller/bootstrap/bootstrap_test.go +++ b/pkg/controller/bootstrap/bootstrap_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/util/diff" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" "github.com/openshift/machine-config-operator/lib/resourceread" ) @@ -150,8 +151,10 @@ func TestBootstrapRun(t *testing.T) { // Ensure that generated registries.conf corresponds to the testdata ImageContentSourcePolicy var registriesConfig *igntypes.File - for i := range mc.Spec.Config.Storage.Files { - f := &mc.Spec.Config.Storage.Files[i] + ignCfg, _, err := ign.Parse(mc.Spec.Config.Raw) + require.NoError(t, err) + for i := range ignCfg.Storage.Files { + f := &ignCfg.Storage.Files[i] if f.Path == "/etc/containers/registries.conf" { registriesConfig = f } diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index 448e8572d4..c53790bfb7 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -6,4 +6,10 @@ const ( // ControllerConfigName is the name of the ControllerConfig object that controllers use ControllerConfigName = "machine-config-controller" + + // KernelTypeDefault denominates the default kernel type + KernelTypeDefault = "default" + + // KernelTypeRealtime denominates the realtime kernel type + KernelTypeRealtime = "realtime" ) diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index 4ffae16f1b..75de24d3c5 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -1,16 +1,89 @@ package common import ( + "encoding/json" "io/ioutil" "reflect" + "sort" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" validate "github.com/coreos/ignition/config/validate" "github.com/golang/glog" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" errors "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" ) +// MergeMachineConfigs combines multiple machineconfig objects into one object. +// It sorts all the configs in increasing order of their name. +// It uses the Ignition config from first object as base and appends all the rest. +// Kernel arguments are concatenated. +// It uses only the OSImageURL provided by the CVO and ignores any MC provided OSImageURL. +func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*mcfgv1.MachineConfig, error) { + if len(configs) == 0 { + return nil, nil + } + sort.Slice(configs, func(i, j int) bool { return configs[i].Name < configs[j].Name }) + + var fips bool + var kernelType string + + outIgn, report, err := ign.Parse(configs[0].Spec.Config.Raw) + if err != nil { + return nil, errors.Errorf("parsing Ignition config failed with error: %v\nReport: %v", err, report) + } + + for idx := 1; idx < len(configs); idx++ { + // if any of the config has FIPS enabled, it'll be set + if configs[idx].Spec.FIPS { + fips = true + } + appendIgn, report, err := ign.Parse(configs[idx].Spec.Config.Raw) + if err != nil { + return nil, errors.Errorf("parsing appendix Ignition config failed with error: %v\nReport: %v", err, report) + } + outIgn = ign.Append(outIgn, appendIgn) + } + rawOutIgn, err := json.Marshal(outIgn) + if err != nil { + return nil, err + } + + // sets the KernelType if specified in any of the MachineConfig + // Setting kerneType to realtime in any of MachineConfig takes priority + for _, cfg := range configs { + if cfg.Spec.KernelType == KernelTypeRealtime { + kernelType = cfg.Spec.KernelType + break + } else if kernelType == KernelTypeDefault { + kernelType = cfg.Spec.KernelType + } + } + + // If no MC sets kerneType, then set it to 'default' since that's what it is using + if kernelType == "" { + kernelType = KernelTypeDefault + } + + kargs := []string{} + for _, cfg := range configs { + kargs = append(kargs, cfg.Spec.KernelArguments...) + } + + return &mcfgv1.MachineConfig{ + Spec: mcfgv1.MachineConfigSpec{ + OSImageURL: osImageURL, + KernelArguments: kargs, + Config: runtime.RawExtension{ + Raw: rawOutIgn, + }, + FIPS: fips, + KernelType: kernelType, + }, + }, nil +} + // NewIgnConfig returns an empty ignition config with version set as latest version func NewIgnConfig() igntypes.Config { return igntypes.Config{ @@ -45,10 +118,14 @@ func ValidateIgnition(cfg igntypes.Config) error { // ValidateMachineConfig validates that given MachineConfig Spec is valid. func ValidateMachineConfig(cfg mcfgv1.MachineConfigSpec) error { - if !(cfg.KernelType == "" || cfg.KernelType == "default" || cfg.KernelType == "realtime") { + if !(cfg.KernelType == "" || cfg.KernelType == KernelTypeDefault || cfg.KernelType == KernelTypeRealtime) { return errors.Errorf("kernelType=%s is invalid", cfg.KernelType) } - if err := ValidateIgnition(cfg.Config); err != nil { + ignCfg, report, err := ign.Parse(cfg.Config.Raw) + if err != nil { + return errors.Errorf("parsing Ignition config failed with error: %v\nReport: %v", err, report) + } + if err := ValidateIgnition(ignCfg); err != nil { return err } return nil 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 f62ac44b60..e07446e1c1 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -546,12 +546,21 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { } if isNotFound { tempIgnCfg := ctrlcommon.NewIgnConfig() - mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &tempIgnCfg) + mc, err = mtmpl.MachineConfigFromIgnConfig(role, managedKey, tempIgnCfg) + if err != nil { + return ctrl.syncStatusOnly(cfg, err, "could not create MachineConfig from new Ignition config: %v", err) + } } - mc.Spec.Config = createNewIgnition(map[string][]byte{ + + ctrRuntimeConfigIgn := createNewIgnition(map[string][]byte{ storageConfigPath: storageTOML, crioConfigPath: crioTOML, }) + rawCtrRuntimeConfigIgn, err := json.Marshal(ctrRuntimeConfigIgn) + if err != nil { + return ctrl.syncStatusOnly(cfg, err, "error marshalling container runtime config Ignition: %v", err) + } + mc.Spec.Config.Raw = rawCtrRuntimeConfigIgn mc.SetAnnotations(map[string]string{ ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, @@ -667,12 +676,16 @@ func (ctrl *Controller) syncImageConfig(key string) error { if err != nil { return err } + rawRegistriesIgn, err := json.Marshal(registriesIgn) + if err != nil { + return fmt.Errorf("could not encode registries Ignition config: %v", err) + } mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(managedKey, metav1.GetOptions{}) if err != nil && !errors.IsNotFound(err) { return fmt.Errorf("could not find MachineConfig: %v", err) } isNotFound := errors.IsNotFound(err) - if !isNotFound && equality.Semantic.DeepEqual(*registriesIgn, mc.Spec.Config) { + if !isNotFound && equality.Semantic.DeepEqual(rawRegistriesIgn, mc.Spec.Config.Raw) { // if the configuration for the registries is equal, we still need to compare // the generated controller version because during an upgrade we need a new one mcCtrlVersion := mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] @@ -683,9 +696,12 @@ func (ctrl *Controller) syncImageConfig(key string) error { } if isNotFound { tempIgnCfg := ctrlcommon.NewIgnConfig() - mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &tempIgnCfg) + mc, err = mtmpl.MachineConfigFromIgnConfig(role, managedKey, tempIgnCfg) + if err != nil { + return fmt.Errorf("could not create MachineConfig from new Ignition config: %v", err) + } } - mc.Spec.Config = *registriesIgn + mc.Spec.Config.Raw = rawRegistriesIgn mc.ObjectMeta.Annotations = map[string]string{ ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, } @@ -775,10 +791,10 @@ func RunImageBootstrap(templateDir string, controllerConfig *mcfgv1.ControllerCo if err != nil { return nil, err } - - tempIgnCfg := ctrlcommon.NewIgnConfig() - mc := mtmpl.MachineConfigFromIgnConfig(role, managedKey, &tempIgnCfg) - mc.Spec.Config = *registriesIgn + mc, err := mtmpl.MachineConfigFromIgnConfig(role, managedKey, registriesIgn) + if err != nil { + return nil, err + } // Explicitly do NOT set GeneratedByControllerVersionAnnotationKey so that the first run of the non-bootstrap controller // always rebuilds registries.conf (with the insecureRegs/blockedRegs values actually available). mc.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ 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 2205659bde..b02cc8f308 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,6 +25,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" apicfgv1 "github.com/openshift/api/config/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" @@ -350,15 +351,18 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin imgcfg.Spec.RegistrySources.BlockedRegistries, icsps) require.NoError(t, err) assert.Equal(t, mcName, mc.ObjectMeta.Name) + + ignCfg, _, err := ign.Parse(mc.Spec.Config.Raw) + require.NoError(t, err) if verifyPolicyJSON { // If there is a change to the policy.json file then there will be 2 files - require.Len(t, mc.Spec.Config.Storage.Files, 2) + require.Len(t, ignCfg.Storage.Files, 2) } else { - require.Len(t, mc.Spec.Config.Storage.Files, 1) + require.Len(t, ignCfg.Storage.Files, 1) } - regfile := mc.Spec.Config.Storage.Files[0] + regfile := ignCfg.Storage.Files[0] if regfile.Node.Path != registriesConfigPath { - regfile = mc.Spec.Config.Storage.Files[1] + regfile = ignCfg.Storage.Files[1] } assert.Equal(t, registriesConfigPath, regfile.Node.Path) registriesConf, err := dataurl.DecodeString(regfile.Contents.Source) @@ -371,9 +375,9 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin imgcfg.Spec.RegistrySources.BlockedRegistries, imgcfg.Spec.RegistrySources.AllowedRegistries) require.NoError(t, err) - policyfile := mc.Spec.Config.Storage.Files[1] + policyfile := ignCfg.Storage.Files[1] if policyfile.Node.Path != policyConfigPath { - policyfile = mc.Spec.Config.Storage.Files[0] + policyfile = ignCfg.Storage.Files[0] } assert.Equal(t, policyConfigPath, policyfile.Node.Path) policyJSON, err := dataurl.DecodeString(policyfile.Contents.Source) diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 37827cd47e..899f8b459e 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -12,6 +12,7 @@ import ( "github.com/containers/image/pkg/sysregistriesv2" signature "github.com/containers/image/signature" storageconfig "github.com/containers/storage/pkg/config" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" crioconfig "github.com/cri-o/cri-o/pkg/config" apicfgv1 "github.com/openshift/api/config/v1" @@ -94,7 +95,11 @@ func createNewIgnition(configs map[string][]byte) igntypes.Config { } func findStorageConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { - for _, c := range mc.Spec.Config.Storage.Files { + ignCfg, report, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing Storage Ignition config failed with error: %v\nReport: %v", err, report) + } + for _, c := range ignCfg.Storage.Files { if c.Path == storageConfigPath { c := c return &c, nil @@ -104,7 +109,11 @@ func findStorageConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { } func findCRIOConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { - for _, c := range mc.Spec.Config.Storage.Files { + ignCfg, report, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing CRI-O Ignition config failed with error: %v\nReport: %v", err, report) + } + for _, c := range ignCfg.Storage.Files { if c.Path == crioConfigPath { c := c return &c, nil @@ -114,7 +123,11 @@ func findCRIOConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { } func findRegistriesConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { - for _, c := range mc.Spec.Config.Storage.Files { + ignCfg, report, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing Registries Ignition config failed with error: %v\nReport: %v", err, report) + } + for _, c := range ignCfg.Storage.Files { if c.Path == registriesConfigPath { return &c, nil } @@ -123,7 +136,11 @@ func findRegistriesConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { } func findPolicyJSON(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { - for _, c := range mc.Spec.Config.Storage.Files { + ignCfg, report, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing Ignition config failed with error: %v\nReport: %v", err, report) + } + for _, c := range ignCfg.Storage.Files { if c.Path == policyConfigPath { return &c, nil } diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index efdafe6fb1..0efe0c0f24 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/pkg/controller/kubelet-config/helpers.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" osev1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" @@ -49,7 +50,11 @@ func createNewDefaultFeatureGate() *osev1.FeatureGate { } func findKubeletConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { - for _, c := range mc.Spec.Config.Storage.Files { + ignCfg, report, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing Ignition config failed with error: %v\nReport: %v", err, report) + } + for _, c := range ignCfg.Storage.Files { if c.Path == "/etc/kubernetes/kubelet.conf" { return &c, nil } diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 2b1493102b..93429cc2a6 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -474,10 +474,18 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { } if isNotFound { ignConfig := ctrlcommon.NewIgnConfig() - mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &ignConfig) + mc, err = mtmpl.MachineConfigFromIgnConfig(role, managedKey, ignConfig) + if err != nil { + return ctrl.syncStatusOnly(cfg, err, "could not create MachineConfig from new Ignition config: %v", err) + } mc.ObjectMeta.UID = uuid.NewUUID() } - mc.Spec.Config = createNewKubeletIgnition(cfgJSON) + cfgIgn := createNewKubeletIgnition(cfgJSON) + rawIgn, err := json.Marshal(cfgIgn) + if err != nil { + return ctrl.syncStatusOnly(cfg, err, "could not marshal kubelet config Ignition: %v", err) + } + mc.Spec.Config.Raw = rawIgn mc.SetAnnotations(map[string]string{ ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index 7138808c69..2624bc9ad4 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -1,6 +1,7 @@ package kubeletconfig import ( + "encoding/json" "fmt" "reflect" "time" @@ -93,7 +94,10 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { isNotFound := errors.IsNotFound(err) if isNotFound { ignConfig := ctrlcommon.NewIgnConfig() - mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &ignConfig) + mc, err = mtmpl.MachineConfigFromIgnConfig(role, managedKey, ignConfig) + if err != nil { + return err + } } // Generate the original KubeletConfig originalKubeletIgn, err := ctrl.generateOriginalKubeletConfig(role) @@ -122,7 +126,12 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { if err != nil { return err } - mc.Spec.Config = createNewKubeletIgnition(cfgJSON) + cfgIgn := createNewKubeletIgnition(cfgJSON) + rawCfgIgn, err := json.Marshal(cfgIgn) + if err != nil { + return err + } + mc.Spec.Config.Raw = rawCfgIgn mc.ObjectMeta.Annotations = map[string]string{ ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, } diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go index f1d5bcca29..058ae76d59 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -529,7 +529,10 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc return nil, err } } - merged := mcfgv1.MergeMachineConfigs(configs, cconfig.Spec.OSImageURL) + merged, err := ctrlcommon.MergeMachineConfigs(configs, cconfig.Spec.OSImageURL) + if err != nil { + return nil, err + } hashedName, err := getMachineConfigHashedName(pool, merged) if err != nil { return nil, err diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index 4aeb17acf5..1eec495121 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -1,11 +1,13 @@ package render import ( + "encoding/json" "fmt" "reflect" "testing" "time" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -285,12 +287,21 @@ func TestIgnValidationGenerateRenderedMachineConfig(t *testing.T) { // verify that an invalid igntion config (here a config with content and an empty version, // will fail validation - mcs[1].Spec.Config.Ignition.Version = "" + ignCfg, _, err := ign.Parse(mcs[1].Spec.Config.Raw) + require.Nil(t, err) + ignCfg.Ignition.Version = "" + rawIgnCfg, err := json.Marshal(ignCfg) + require.Nil(t, err) + mcs[1].Spec.Config.Raw = rawIgnCfg + _, err = generateRenderedMachineConfig(mcp, mcs, cc) require.NotNil(t, err) // verify that a machine config with no ignition content will not fail validation - mcs[1].Spec.Config = igntypes.Config{} + emptyIgnCfg := ctrlcommon.NewIgnConfig() + rawEmptyIgnCfg, err := json.Marshal(emptyIgnCfg) + require.Nil(t, err) + mcs[1].Spec.Config.Raw = rawEmptyIgnCfg mcs[1].Spec.KernelArguments = append(mcs[1].Spec.KernelArguments, "test1") _, err = generateRenderedMachineConfig(mcp, mcs, cc) require.Nil(t, err) diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go index 50f64b6d70..b0a0782304 100644 --- a/pkg/controller/template/render.go +++ b/pkg/controller/template/render.go @@ -2,6 +2,7 @@ package template import ( "bytes" + "encoding/json" "fmt" "io/ioutil" "os" @@ -21,6 +22,7 @@ import ( ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/version" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtime "k8s.io/apimachinery/pkg/runtime" ) // RenderConfig is wrapper around ControllerConfigSpec. @@ -267,8 +269,10 @@ func generateMachineConfigForName(config *RenderConfig, role, name, templateDir, if err != nil { return nil, fmt.Errorf("error transpiling ct config to Ignition config: %v", err) } - - mcfg := MachineConfigFromIgnConfig(role, name, ignCfg) + mcfg, err := MachineConfigFromIgnConfig(role, name, ignCfg) + if err != nil { + return nil, fmt.Errorf("error creating MachineConfig from Ignition config: %v", err) + } // And inject the osimageurl here mcfg.Spec.OSImageURL = config.OSImageURL @@ -276,7 +280,11 @@ func generateMachineConfigForName(config *RenderConfig, role, name, templateDir, } // MachineConfigFromIgnConfig creates a MachineConfig with the provided Ignition config -func MachineConfigFromIgnConfig(role, name string, ignCfg *igntypes.Config) *mcfgv1.MachineConfig { +func MachineConfigFromIgnConfig(role, name string, ignCfg interface{}) (*mcfgv1.MachineConfig, error) { + rawIgnCfg, err := json.Marshal(ignCfg) + if err != nil { + return nil, fmt.Errorf("error marshalling Ignition config: %v", err) + } labels := map[string]string{ mcfgv1.MachineConfigRoleLabelKey: role, } @@ -287,9 +295,11 @@ func MachineConfigFromIgnConfig(role, name string, ignCfg *igntypes.Config) *mcf }, Spec: mcfgv1.MachineConfigSpec{ OSImageURL: "", - Config: *ignCfg, + Config: runtime.RawExtension{ + Raw: rawIgnCfg, + }, }, - } + }, nil } func transpileToIgn(files, units []string) (*igntypes.Config, error) { diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go index f01c0817d4..ab4a155667 100644 --- a/pkg/controller/template/render_test.go +++ b/pkg/controller/template/render_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "testing" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "k8s.io/client-go/kubernetes/scheme" @@ -301,7 +302,10 @@ func TestGenerateMachineConfigs(t *testing.T) { t.Fatal("role label missing") } - ign := cfg.Spec.Config + ign, _, err := ign.Parse(cfg.Spec.Config.Raw) + if err != nil { + t.Errorf("Failed to parse Ignition config") + } if role == "master" { if !foundPullSecretMaster { foundPullSecretMaster = findIgnFile(ign.Storage.Files, "/var/lib/kubelet/config.json", t) diff --git a/pkg/controller/template/template_controller_test.go b/pkg/controller/template/template_controller_test.go index 0551ea638e..4b7720c2a0 100644 --- a/pkg/controller/template/template_controller_test.go +++ b/pkg/controller/template/template_controller_test.go @@ -1,6 +1,7 @@ package template import ( + "encoding/json" "fmt" "reflect" "testing" @@ -371,7 +372,12 @@ func TestUpdateMachineConfig(t *testing.T) { t.Fatal(err) } //update machineconfig - mcs[len(mcs)-1].Spec.Config = ctrlcommon.NewIgnConfig() + newIgnCfg := ctrlcommon.NewIgnConfig() + newRawIgnCfg, err := json.Marshal(newIgnCfg) + if err != nil { + t.Fatal(err) + } + mcs[len(mcs)-1].Spec.Config.Raw = newRawIgnCfg f.ccLister = append(f.ccLister, cc) f.kubeobjects = append(f.kubeobjects, ps) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index a41d947683..1f76eea927 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -501,7 +501,11 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig() error { // Currently, we generally expect the bootimage to be older, but in the special // case of having bootimage == machine-os-content, and no kernel arguments // specified, then we don't need to do anything here. - if !dn.compareMachineConfig(oldConfig, &mc) { + mcDiffNotEmpty, err := dn.compareMachineConfig(oldConfig, &mc) + if err != nil { + return errors.Wrapf(err, "failed to compare MachineConfig") + } + if !mcDiffNotEmpty { // Removing this file signals completion of the initial MC processing. if err := os.Remove(constants.MachineConfigEncapsulatedPath); err != nil { return errors.Wrapf(err, "failed to remove %s", constants.MachineConfigEncapsulatedPath) @@ -1220,10 +1224,15 @@ func (dn *Daemon) validateOnDiskState(currentConfig *mcfgv1.MachineConfig) bool return false } // And the rest of the disk state - if !checkFiles(currentConfig.Spec.Config.Storage.Files) { + currentIgnConfig, report, err := ign.Parse(currentConfig.Spec.Config.Raw) + if err != nil { + glog.Errorf("Failed to parse Ignition for validation: %s\nReport: %v", err, report) + return false + } + if !checkFiles(currentIgnConfig.Storage.Files) { return false } - if !checkUnits(currentConfig.Spec.Config.Systemd.Units) { + if !checkUnits(currentIgnConfig.Systemd.Units) { return false } return true @@ -1438,13 +1447,13 @@ func (dn *Daemon) senseAndLoadOnceFrom(onceFrom string) (interface{}, onceFromOr } // Try each supported parser - ignConfig, _, err := ign.Parse(content) + ignConfig, report, err := ign.Parse(content) if err == nil && ignConfig.Ignition.Version != "" { glog.V(2).Info("onceFrom file is of type Ignition") return ignConfig, contentFrom, nil } - glog.V(2).Infof("%s is not an Ignition config: %v. Trying MachineConfig.", onceFrom, err) + glog.V(2).Infof("%s is not an Ignition config: %v\nReport: %v\n Trying MachineConfig.", onceFrom, err, report) // Try to parse as a machine config mc, err := resourceread.ReadMachineConfigV1(content) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 8a91fef94f..68bd45328d 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -17,6 +17,7 @@ import ( "syscall" "time" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" "github.com/golang/glog" "github.com/google/renameio" @@ -28,6 +29,7 @@ import ( "github.com/vincent-petithory/dataurl" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubectl/pkg/drain" @@ -44,10 +46,6 @@ const ( coreUserSSHPath = "/home/core/.ssh/" // fipsFile is the file to check if FIPS is enabled fipsFile = "/proc/sys/crypto/fips_enabled" - // Traditional kernel - defaultKernelType = "default" - // Realtime kernel - realtimeKernelType = "realtime" ) // HostInfo contains information of an OSTree based system @@ -214,10 +212,18 @@ func canonicalizeEmptyMC(config *mcfgv1.MachineConfig) *mcfgv1.MachineConfig { if config != nil { return config } + newIgnCfg := ctrlcommon.NewIgnConfig() + rawNewIgnCfg, err := json.Marshal(newIgnCfg) + if err != nil { + // This should never happen + panic(err) + } return &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{Name: "mco-empty-mc"}, Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), + Config: runtime.RawExtension{ + Raw: rawNewIgnCfg, + }, }, } } @@ -248,17 +254,20 @@ func rtKernelUpdateAvailable(rpms []os.FileInfo, rtKernelPkg []string) (bool, er return false, nil } -func (dn *Daemon) compareMachineConfig(oldConfig, newConfig *mcfgv1.MachineConfig) bool { - var diff *MachineConfigDiff +// return true if the MachineConfigDiff is not empty +func (dn *Daemon) compareMachineConfig(oldConfig, newConfig *mcfgv1.MachineConfig) (bool, error) { oldConfig = canonicalizeEmptyMC(oldConfig) oldConfigName := oldConfig.GetName() newConfigName := newConfig.GetName() - diff = NewMachineConfigDiff(oldConfig, newConfig) - if diff.IsEmpty() { + mcDiff, err := NewMachineConfigDiff(oldConfig, newConfig) + if err != nil { + return true, errors.Wrapf(err, "error creating MachineConfigDiff for comparison") + } + if mcDiff.IsEmpty() { glog.Infof("No changes from %s to %s", oldConfigName, newConfigName) - return false + return false, nil } - return true + return true, nil } // update the node to the provided node configuration. @@ -325,13 +334,22 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig) (retErr err } }() - if err := dn.updateSSHKeys(newConfig.Spec.Config.Passwd.Users); err != nil { + oldIgnConfig, report, err := ign.Parse(oldConfig.Spec.Config.Raw) + if err != nil { + return fmt.Errorf("parsing old Ignition config failed with error: %v\nReport: %v", err, report) + } + newIgnConfig, report, err := ign.Parse(newConfig.Spec.Config.Raw) + if err != nil { + return fmt.Errorf("parsing new Ignition config failed with error: %v\nReport: %v", err, report) + } + + if err := dn.updateSSHKeys(newIgnConfig.Passwd.Users); err != nil { return err } defer func() { if retErr != nil { - if err := dn.updateSSHKeys(oldConfig.Spec.Config.Passwd.Users); err != nil { + if err := dn.updateSSHKeys(oldIgnConfig.Passwd.Users); err != nil { retErr = errors.Wrapf(retErr, "error rolling back SSH keys updates %v", err) return } @@ -396,16 +414,22 @@ type MachineConfigDiff struct { // canonicalizeKernelType returns a valid kernelType. We consider empty("") and default kernelType as same func canonicalizeKernelType(kernelType string) string { - if kernelType == realtimeKernelType { - return realtimeKernelType + if kernelType == ctrlcommon.KernelTypeRealtime { + return ctrlcommon.KernelTypeRealtime } - return defaultKernelType + return ctrlcommon.KernelTypeDefault } // NewMachineConfigDiff compares two MachineConfig objects. -func NewMachineConfigDiff(oldConfig, newConfig *mcfgv1.MachineConfig) *MachineConfigDiff { - oldIgn := oldConfig.Spec.Config - newIgn := newConfig.Spec.Config +func NewMachineConfigDiff(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDiff, error) { + oldIgn, report, err := ign.Parse(oldConfig.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing old Ignition config failed with error: %v\nReport: %v", err, report) + } + newIgn, report, err := ign.Parse(newConfig.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing new Ignition config failed with error: %v\nReport: %v", err, report) + } // Both nil and empty slices are of zero length, // consider them as equal while comparing KernelArguments in both MachineConfigs @@ -419,7 +443,7 @@ func NewMachineConfigDiff(oldConfig, newConfig *mcfgv1.MachineConfig) *MachineCo files: !reflect.DeepEqual(oldIgn.Storage.Files, newIgn.Storage.Files), units: !reflect.DeepEqual(oldIgn.Systemd.Units, newIgn.Systemd.Units), kernelType: canonicalizeKernelType(oldConfig.Spec.KernelType) != canonicalizeKernelType(newConfig.Spec.KernelType), - } + }, nil } // IsEmpty returns true if the MachineConfigDiff has no changes, or @@ -441,26 +465,22 @@ func (d *MachineConfigDiff) IsEmpty() bool { // directories, links, and systemd units sections of the included ignition // config currently. func Reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDiff, error) { - oldIgn := oldConfig.Spec.Config - newIgn := newConfig.Spec.Config + // The parser will try to translate versions less than maxVersion to maxVersion, or output an err. + // The ignition output in case of success will always have maxVersion + oldIgn, report, err := ign.Parse(oldConfig.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing old Ignition config failed with error: %v\nReport: %v", err, report) + } + newIgn, report, err := ign.Parse(newConfig.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing new Ignition config failed with error: %v\nReport: %v", err, report) + } - // Ignition section - // First check if this is a generally valid Ignition Config + // Check if this is a generally valid Ignition Config if err := ctrlcommon.ValidateIgnition(newIgn); err != nil { return nil, err } - // if the config versions are different, all bets are off. this probably - // shouldn't happen, but if it does, we can't deal with it. - if oldIgn.Ignition.Version != newIgn.Ignition.Version { - return nil, fmt.Errorf("ignition version mismatch between old and new config: old: %s new: %s", - oldIgn.Ignition.Version, newIgn.Ignition.Version) - } - // everything else in the ignition section doesn't matter to us, since the - // rest of the stuff in this section has to do with fetching remote - // resources, and the mcc should've fully rendered those out before the - // config gets here. - // Networkd section // we don't currently configure the network in place. we can't fix it if @@ -543,7 +563,11 @@ func Reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDif // we made it through all the checks. reconcile away! glog.V(2).Info("Configs are reconcilable") - return NewMachineConfigDiff(oldConfig, newConfig), nil + mcDiff, err := NewMachineConfigDiff(oldConfig, newConfig) + if err != nil { + return nil, errors.Wrapf(err, "error creating MachineConfigDiff") + } + return mcDiff, nil } // verifyUserFields returns nil if the user Name = "core", if 1 or more SSHKeys exist for @@ -673,7 +697,7 @@ func (dn *Daemon) mountOSContainer(container string) (mnt, containerName string, // Right now it supports default (traditional) and realtime kernel func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error { // Do nothing if both old and new KernelType are of type default - if canonicalizeKernelType(oldConfig.Spec.KernelType) == defaultKernelType && canonicalizeKernelType(newConfig.Spec.KernelType) == defaultKernelType { + if canonicalizeKernelType(oldConfig.Spec.KernelType) == ctrlcommon.KernelTypeDefault && canonicalizeKernelType(newConfig.Spec.KernelType) == ctrlcommon.KernelTypeDefault { return nil } // We support Kernel update only on RHCOS nodes @@ -686,7 +710,7 @@ func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error dn.logSystem("Initiating switch from kernel %s to %s", canonicalizeKernelType(oldConfig.Spec.KernelType), canonicalizeKernelType(newConfig.Spec.KernelType)) - if canonicalizeKernelType(oldConfig.Spec.KernelType) == realtimeKernelType && canonicalizeKernelType(newConfig.Spec.KernelType) == defaultKernelType { + if canonicalizeKernelType(oldConfig.Spec.KernelType) == ctrlcommon.KernelTypeRealtime && canonicalizeKernelType(newConfig.Spec.KernelType) == ctrlcommon.KernelTypeDefault { var installedRTKernelRpms []string var err error args = []string{"override", "reset"} @@ -739,7 +763,7 @@ func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error return fmt.Errorf("No kernel-rt package available in the OSContainer with URL %s", newConfig.Spec.OSImageURL) } - if canonicalizeKernelType(oldConfig.Spec.KernelType) == defaultKernelType && canonicalizeKernelType(newConfig.Spec.KernelType) == realtimeKernelType { + if canonicalizeKernelType(oldConfig.Spec.KernelType) == ctrlcommon.KernelTypeDefault && canonicalizeKernelType(newConfig.Spec.KernelType) == ctrlcommon.KernelTypeRealtime { // Switch to RT kernel args = []string{"override", "remove"} args = append(args, defaultKernel...) @@ -753,7 +777,7 @@ func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error } } - if canonicalizeKernelType(oldConfig.Spec.KernelType) == realtimeKernelType && canonicalizeKernelType(newConfig.Spec.KernelType) == realtimeKernelType { + if canonicalizeKernelType(oldConfig.Spec.KernelType) == ctrlcommon.KernelTypeRealtime && canonicalizeKernelType(newConfig.Spec.KernelType) == ctrlcommon.KernelTypeRealtime { if oldConfig.Spec.OSImageURL != newConfig.Spec.OSImageURL { var installedRTKernelRpms []string var err error @@ -806,27 +830,34 @@ func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error // touched. func (dn *Daemon) updateFiles(oldConfig, newConfig *mcfgv1.MachineConfig) error { glog.Info("Updating files") - - if err := dn.writeFiles(newConfig.Spec.Config.Storage.Files); err != nil { + oldIgnConfig, report, err := ign.Parse(oldConfig.Spec.Config.Raw) + if err != nil { + return fmt.Errorf("failed to update files. Parsing old Ignition config failed with error: %v\nReport: %v", err, report) + } + newIgnConfig, report, err := ign.Parse(newConfig.Spec.Config.Raw) + if err != nil { + return fmt.Errorf("failed to update files. Parsing new Ignition config failed with error: %v\nReport: %v", err, report) + } + if err := dn.writeFiles(newIgnConfig.Storage.Files); err != nil { return err } - if err := dn.writeUnits(newConfig.Spec.Config.Systemd.Units); err != nil { + if err := dn.writeUnits(newIgnConfig.Systemd.Units); err != nil { return err } - if err := dn.deleteStaleData(oldConfig, newConfig); err != nil { + if err := dn.deleteStaleData(&oldIgnConfig, &newIgnConfig); err != nil { return err } return nil } -// deleteStaleData performs a diff of the new and the old config. It then deletes +// deleteStaleData performs a diff of the new and the old Ignition config. It then deletes // all the files, units that are present in the old config but not in the new one. // this function will error out if it fails to delete a file (with the exception // of simply warning if the error is ENOENT since that's the desired state). -func (dn *Daemon) deleteStaleData(oldConfig, newConfig *mcfgv1.MachineConfig) error { +func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *igntypes.Config) error { glog.Info("Deleting stale data") newFileSet := make(map[string]struct{}) - for _, f := range newConfig.Spec.Config.Storage.Files { + for _, f := range newIgnConfig.Storage.Files { newFileSet[f.Path] = struct{}{} } @@ -835,7 +866,7 @@ func (dn *Daemon) deleteStaleData(oldConfig, newConfig *mcfgv1.MachineConfig) er return errors.Wrapf(err, "checking operating system") } - for _, f := range oldConfig.Spec.Config.Storage.Files { + for _, f := range oldIgnConfig.Storage.Files { if _, ok := newFileSet[f.Path]; !ok { if _, err := os.Stat(noOrigFileStampName(f.Path)); err == nil { if err := os.Remove(noOrigFileStampName(f.Path)); err != nil { @@ -881,7 +912,7 @@ func (dn *Daemon) deleteStaleData(oldConfig, newConfig *mcfgv1.MachineConfig) er newUnitSet := make(map[string]struct{}) newDropinSet := make(map[string]struct{}) - for _, u := range newConfig.Spec.Config.Systemd.Units { + for _, u := range newIgnConfig.Systemd.Units { for j := range u.Dropins { path := filepath.Join(pathSystemd, u.Name+".d", u.Dropins[j].Name) newDropinSet[path] = struct{}{} @@ -890,7 +921,7 @@ func (dn *Daemon) deleteStaleData(oldConfig, newConfig *mcfgv1.MachineConfig) er newUnitSet[path] = struct{}{} } - for _, u := range oldConfig.Spec.Config.Systemd.Units { + for _, u := range oldIgnConfig.Systemd.Units { for j := range u.Dropins { path := filepath.Join(pathSystemd, u.Name+".d", u.Dropins[j].Name) if _, ok := newDropinSet[path]; !ok { diff --git a/pkg/daemon/update_test.go b/pkg/daemon/update_test.go index 4c2a4de130..ea3452b50d 100644 --- a/pkg/daemon/update_test.go +++ b/pkg/daemon/update_test.go @@ -10,6 +10,7 @@ import ( igntypes "github.com/coreos/ignition/config/v2_2/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" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sfake "k8s.io/client-go/kubernetes/fake" @@ -64,136 +65,127 @@ func TestUpdateOS(t *testing.T) { // TestReconcilable attempts to verify the conditions in which configs would and would not be // reconcilable. Welcome to the longest unittest you've ever read. func TestReconcilable(t *testing.T) { + oldIgnCfg := ctrlcommon.NewIgnConfig() // oldConfig is the current config of the fake system - oldConfig := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } + oldConfig := helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + newIgnCfg := ctrlcommon.NewIgnConfig() - // newConfig is the config that is being requested to apply to the system - newConfig := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } + // Set improper version + newIgnCfg.Ignition.Version = "3.0.0" + // newConfig is the config that is being requested to apply to the system + newConfig := helpers.CreateMachineConfigFromIgnition(newIgnCfg) // Verify Ignition version mismatch react as expected - newConfig.Spec.Config.Ignition.Version = "2.0.0" _, isReconcilable := Reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "Ignition", isReconcilable) //reset to proper Ignition version - newConfig.Spec.Config.Ignition.Version = igntypes.MaxVersion.String() + newIgnCfg.Ignition.Version = igntypes.MaxVersion.String() + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "Ignition", isReconcilable) // Verify Networkd unit changes react as expected - oldConfig.Spec.Config.Networkd = igntypes.Networkd{} - newConfig.Spec.Config.Networkd = igntypes.Networkd{ + oldIgnCfg.Networkd = igntypes.Networkd{} + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + newIgnCfg.Networkd = igntypes.Networkd{ Units: []igntypes.Networkdunit{ igntypes.Networkdunit{ Name: "test.network", }, }, } + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "Networkd", isReconcilable) // Match Networkd - oldConfig.Spec.Config.Networkd = newConfig.Spec.Config.Networkd - + oldIgnCfg.Networkd = newIgnCfg.Networkd + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "Networkd", isReconcilable) // Verify Disk changes react as expected - oldConfig.Spec.Config.Storage.Disks = []igntypes.Disk{ + oldIgnCfg.Storage.Disks = []igntypes.Disk{ igntypes.Disk{ Device: "/one", }, } - + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "Disk", isReconcilable) // Match storage disks - newConfig.Spec.Config.Storage.Disks = oldConfig.Spec.Config.Storage.Disks + newIgnCfg.Storage.Disks = oldIgnCfg.Storage.Disks + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "Disk", isReconcilable) // Verify Filesystems changes react as expected oldFSPath := "/foo/bar" - oldConfig.Spec.Config.Storage.Filesystems = []igntypes.Filesystem{ + oldIgnCfg.Storage.Filesystems = []igntypes.Filesystem{ igntypes.Filesystem{ Name: "user", Path: &oldFSPath, }, } - + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "Filesystem", isReconcilable) // Match Storage filesystems - newConfig.Spec.Config.Storage.Filesystems = oldConfig.Spec.Config.Storage.Filesystems + newIgnCfg.Storage.Filesystems = oldIgnCfg.Storage.Filesystems + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "Filesystem", isReconcilable) // Verify Raid changes react as expected - oldConfig.Spec.Config.Storage.Raid = []igntypes.Raid{ + oldIgnCfg.Storage.Raid = []igntypes.Raid{ igntypes.Raid{ Name: "data", Level: "stripe", }, } - + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "Raid", isReconcilable) // Match storage raid - newConfig.Spec.Config.Storage.Raid = oldConfig.Spec.Config.Storage.Raid + newIgnCfg.Storage.Raid = oldIgnCfg.Storage.Raid + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "Raid", isReconcilable) // Verify Passwd Groups changes unsupported - oldConfig = &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - newConfig = &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } + oldIgnCfg = ctrlcommon.NewIgnConfig() + oldConfig = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + newIgnCfg = ctrlcommon.NewIgnConfig() + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkReconcilableResults(t, "PasswdGroups", isReconcilable) tempGroup := igntypes.PasswdGroup{} tempGroup.Name = "testGroup" - newConfig.Spec.Config.Passwd.Groups = []igntypes.PasswdGroup{tempGroup} - + newIgnCfg.Passwd.Groups = []igntypes.PasswdGroup{tempGroup} + newConfig = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, isReconcilable = Reconcilable(oldConfig, newConfig) checkIrreconcilableResults(t, "PasswdGroups", isReconcilable) } func TestMachineConfigDiff(t *testing.T) { - oldConfig := &mcfgv1.MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "oldconfig"}, - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - newConfig := &mcfgv1.MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "newconfig"}, - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - diff := NewMachineConfigDiff(oldConfig, newConfig) + oldIgnCfg := ctrlcommon.NewIgnConfig() + oldConfig := helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + oldConfig.ObjectMeta = metav1.ObjectMeta{Name: "oldconfig"} + newIgnCfg := ctrlcommon.NewIgnConfig() + newConfig := helpers.CreateMachineConfigFromIgnition(newIgnCfg) + newConfig.ObjectMeta = metav1.ObjectMeta{Name: "newconfig"} + diff, err := NewMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) assert.True(t, diff.IsEmpty()) newConfig.Spec.OSImageURL = "quay.io/example/foo@sha256:b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c" - diff = NewMachineConfigDiff(oldConfig, newConfig) + diff, err = NewMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) assert.False(t, diff.IsEmpty()) assert.True(t, diff.osUpdate) @@ -201,7 +193,8 @@ func TestMachineConfigDiff(t *testing.T) { otherEmptyMc := canonicalizeEmptyMC(nil) emptyMc.Spec.KernelArguments = nil otherEmptyMc.Spec.KernelArguments = []string{} - diff = NewMachineConfigDiff(emptyMc, otherEmptyMc) + diff, err = NewMachineConfigDiff(emptyMc, otherEmptyMc) + assert.Nil(t, err) assert.True(t, diff.IsEmpty()) } @@ -212,12 +205,9 @@ func newTestIgnitionFile(i uint) igntypes.File { } func newMachineConfigFromFiles(files []igntypes.File) *mcfgv1.MachineConfig { - newConfig := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - newConfig.Spec.Config.Storage.Files = files + newIgnCfg := ctrlcommon.NewIgnConfig() + newIgnCfg.Storage.Files = files + newConfig := helpers.CreateMachineConfigFromIgnition(newIgnCfg) return newConfig } @@ -256,12 +246,9 @@ func TestReconcilableDiff(t *testing.T) { } func TestKernelAguments(t *testing.T) { - oldMcfg := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - KernelArguments: []string{"nosmt", "foo", "baz=test"}, - }, - } + oldIgnCfg := ctrlcommon.NewIgnConfig() + oldMcfg := helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + oldMcfg.Spec.KernelArguments = []string{"nosmt", "foo", "baz=test"} tests := []struct { args []string deletions []string @@ -296,12 +283,9 @@ func TestKernelAguments(t *testing.T) { rand.Seed(time.Now().UnixNano()) for idx, test := range tests { t.Run(fmt.Sprintf("case#%d", idx), func(t *testing.T) { - newMcfg := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - KernelArguments: test.args, - }, - } + newIgnCfg := ctrlcommon.NewIgnConfig() + newMcfg := helpers.CreateMachineConfigFromIgnition(newIgnCfg) + newMcfg.Spec.KernelArguments = test.args // Randomize the order of both old/new to sort out ordering issues. rand.Shuffle(len(test.args), func(i, j int) { test.args[i], test.args[j] = test.args[j], test.args[i] @@ -343,57 +327,50 @@ func TestKernelAguments(t *testing.T) { func TestReconcilableSSH(t *testing.T) { // Check that updating SSH Key of user core supported - oldMcfg := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - + oldIgnCfg := ctrlcommon.NewIgnConfig() + oldMcfg := helpers.CreateMachineConfigFromIgnition(oldIgnCfg) tempUser1 := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"5678", "abc"}} - newMcfg := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - newMcfg.Spec.Config.Passwd.Users = []igntypes.PasswdUser{tempUser1} - + newIgnCfg := ctrlcommon.NewIgnConfig() + newIgnCfg.Passwd.Users = []igntypes.PasswdUser{tempUser1} + newMcfg := helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg := Reconcilable(oldMcfg, newMcfg) checkReconcilableResults(t, "SSH", errMsg) // Check that updating User with User that is not core is not supported tempUser2 := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"1234"}} - oldMcfg.Spec.Config.Passwd.Users = append(oldMcfg.Spec.Config.Passwd.Users, tempUser2) + oldIgnCfg.Passwd.Users = append(oldIgnCfg.Passwd.Users, tempUser2) + oldMcfg = helpers.CreateMachineConfigFromIgnition(oldIgnCfg) tempUser3 := igntypes.PasswdUser{Name: "another user", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"5678"}} - newMcfg.Spec.Config.Passwd.Users[0] = tempUser3 - + newIgnCfg.Passwd.Users[0] = tempUser3 + newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg = Reconcilable(oldMcfg, newMcfg) checkIrreconcilableResults(t, "SSH", errMsg) // check that we cannot make updates if any other Passwd.User field is changed. tempUser4 := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"5678"}, HomeDir: "somedir"} - newMcfg.Spec.Config.Passwd.Users[0] = tempUser4 - + newIgnCfg.Passwd.Users[0] = tempUser4 + newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg = Reconcilable(oldMcfg, newMcfg) checkIrreconcilableResults(t, "SSH", errMsg) // check that we cannot add a user or have len(Passwd.Users)> 1 tempUser5 := igntypes.PasswdUser{Name: "some user", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"5678"}} - newMcfg.Spec.Config.Passwd.Users = append(newMcfg.Spec.Config.Passwd.Users, tempUser5) - + newIgnCfg.Passwd.Users = append(newIgnCfg.Passwd.Users, tempUser5) + newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg = Reconcilable(oldMcfg, newMcfg) checkIrreconcilableResults(t, "SSH", errMsg) // check that user is not attempting to remove the only sshkey from core user tempUser6 := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{}} - newMcfg.Spec.Config.Passwd.Users[0] = tempUser6 - newMcfg.Spec.Config.Passwd.Users = newMcfg.Spec.Config.Passwd.Users[:len(newMcfg.Spec.Config.Passwd.Users)-1] - + newIgnCfg.Passwd.Users[0] = tempUser6 + newIgnCfg.Passwd.Users = newIgnCfg.Passwd.Users[:len(newIgnCfg.Passwd.Users)-1] + newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg = Reconcilable(oldMcfg, newMcfg) checkIrreconcilableResults(t, "SSH", errMsg) //check that empty Users does not generate error/degrade node - newMcfg.Spec.Config.Passwd.Users = nil - + newIgnCfg.Passwd.Users = nil + newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnCfg) _, errMsg = Reconcilable(oldMcfg, newMcfg) checkReconcilableResults(t, "SSH", errMsg) } @@ -422,22 +399,18 @@ func TestUpdateSSHKeys(t *testing.T) { } // Set up machineconfigs that are identical except for SSH keys tempUser := igntypes.PasswdUser{Name: "core", SSHAuthorizedKeys: []igntypes.SSHAuthorizedKey{"1234", "4567"}} - newMcfg := &mcfgv1.MachineConfig{ - Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), - }, - } - newMcfg.Spec.Config.Passwd.Users = []igntypes.PasswdUser{tempUser} - - err := d.updateSSHKeys(newMcfg.Spec.Config.Passwd.Users) + newIgnCfg := ctrlcommon.NewIgnConfig() + newIgnCfg.Passwd.Users = []igntypes.PasswdUser{tempUser} + err := d.updateSSHKeys(newIgnCfg.Passwd.Users) if err != nil { t.Errorf("Expected no error. Got %s.", err) } // if Users is empty, nothing should happen and no error should ever be generated - newMcfg2 := &mcfgv1.MachineConfig{} - err = d.updateSSHKeys(newMcfg2.Spec.Config.Passwd.Users) + newIgnCfg2 := ctrlcommon.NewIgnConfig() + newIgnCfg2.Passwd.Users = []igntypes.PasswdUser{} + err = d.updateSSHKeys(newIgnCfg2.Passwd.Users) if err != nil { t.Errorf("Expected no error. Got: %s", err) } @@ -447,7 +420,7 @@ func TestUpdateSSHKeys(t *testing.T) { // Ignition validation does not permit writing files to relative paths. func TestInvalidIgnConfig(t *testing.T) { oldIgnConfig := ctrlcommon.NewIgnConfig() - oldMcfg := &mcfgv1.MachineConfig{Spec: mcfgv1.MachineConfigSpec{Config: oldIgnConfig}} + oldMcfg := helpers.CreateMachineConfigFromIgnition(oldIgnConfig) // create file to write that contains an impermissable relative path tempFileContents := igntypes.FileContents{Source: "data:,hello%20world%0A"} @@ -458,11 +431,12 @@ func TestInvalidIgnConfig(t *testing.T) { FileEmbedded1: igntypes.FileEmbedded1{Contents: tempFileContents, Mode: &tempMode}, } newIgnConfig.Storage.Files = append(newIgnConfig.Storage.Files, newIgnFile) - newMcfg := &mcfgv1.MachineConfig{Spec: mcfgv1.MachineConfigSpec{Config: newIgnConfig}} + newMcfg := helpers.CreateMachineConfigFromIgnition(newIgnConfig) _, err := Reconcilable(oldMcfg, newMcfg) assert.NotNil(t, err, "Expected error. Relative Paths should fail general ignition validation") - newMcfg.Spec.Config.Storage.Files[0].Node.Path = "/home/core/test" + newIgnConfig.Storage.Files[0].Node.Path = "/home/core/test" + newMcfg = helpers.CreateMachineConfigFromIgnition(newIgnConfig) diff, err := Reconcilable(oldMcfg, newMcfg) assert.Nil(t, err, "Expected no error. Absolute paths should not fail general ignition validation") assert.Equal(t, diff.files, true) diff --git a/pkg/server/api_test.go b/pkg/server/api_test.go index cf7665956c..62e249dd93 100644 --- a/pkg/server/api_test.go +++ b/pkg/server/api_test.go @@ -8,13 +8,15 @@ import ( "testing" igntypes "github.com/coreos/ignition/config/v2_2/types" + "github.com/openshift/machine-config-operator/test/helpers" + "k8s.io/apimachinery/pkg/runtime" ) type mockServer struct { - GetConfigFn func(poolRequest) (*igntypes.Config, error) + GetConfigFn func(poolRequest) (*runtime.RawExtension, error) } -func (ms *mockServer) GetConfig(pr poolRequest) (*igntypes.Config, error) { +func (ms *mockServer) GetConfig(pr poolRequest) (*runtime.RawExtension, error) { return ms.GetConfigFn(pr) } @@ -23,7 +25,7 @@ type checkResponse func(t *testing.T, response *http.Response) type scenario struct { name string request *http.Request - serverFunc func(poolRequest) (*igntypes.Config, error) + serverFunc func(poolRequest) (*runtime.RawExtension, error) checkResponse checkResponse } @@ -32,8 +34,10 @@ func TestAPIHandler(t *testing.T) { { name: "get config path that does not exist", request: httptest.NewRequest(http.MethodGet, "http://testrequest/config/does-not-exist", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), fmt.Errorf("not acceptable") + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, fmt.Errorf("not acceptable") }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusInternalServerError) @@ -44,8 +48,10 @@ func TestAPIHandler(t *testing.T) { { name: "get config path that exists", request: httptest.NewRequest(http.MethodGet, "http://testrequest/config/master", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -57,8 +63,10 @@ func TestAPIHandler(t *testing.T) { { name: "head config path that exists", request: httptest.NewRequest(http.MethodHead, "http://testrequest/config/master", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -70,8 +78,10 @@ func TestAPIHandler(t *testing.T) { { name: "post config path that exists", request: httptest.NewRequest(http.MethodPost, "http://testrequest/config/master", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusMethodNotAllowed) @@ -102,8 +112,10 @@ func TestHealthzHandler(t *testing.T) { { name: "get healthz", request: httptest.NewRequest(http.MethodGet, "http://testrequest/healthz", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -114,8 +126,10 @@ func TestHealthzHandler(t *testing.T) { { name: "head healthz", request: httptest.NewRequest(http.MethodHead, "http://testrequest/healthz", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -126,8 +140,10 @@ func TestHealthzHandler(t *testing.T) { { name: "post healthz", request: httptest.NewRequest(http.MethodPost, "http://testrequest/healthz", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusMethodNotAllowed) @@ -154,8 +170,10 @@ func TestDefaultHandler(t *testing.T) { { name: "get root", request: httptest.NewRequest(http.MethodGet, "http://testrequest/", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusNotFound) @@ -166,8 +184,10 @@ func TestDefaultHandler(t *testing.T) { { name: "head root", request: httptest.NewRequest(http.MethodHead, "http://testrequest/", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusNotFound) @@ -178,8 +198,10 @@ func TestDefaultHandler(t *testing.T) { { name: "post root", request: httptest.NewRequest(http.MethodPost, "http://testrequest/", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusMethodNotAllowed) @@ -206,8 +228,10 @@ func TestAPIServer(t *testing.T) { { name: "get config path that does not exist", request: httptest.NewRequest(http.MethodGet, "http://testrequest/config/does-not-exist", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), fmt.Errorf("not acceptable") + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, fmt.Errorf("not acceptable") }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusInternalServerError) @@ -218,8 +242,10 @@ func TestAPIServer(t *testing.T) { { name: "get config path that exists", request: httptest.NewRequest(http.MethodGet, "http://testrequest/config/master", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -231,8 +257,10 @@ func TestAPIServer(t *testing.T) { { name: "head config path that exists", request: httptest.NewRequest(http.MethodHead, "http://testrequest/config/master", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -244,8 +272,10 @@ func TestAPIServer(t *testing.T) { { name: "post config path that exists", request: httptest.NewRequest(http.MethodPost, "http://testrequest/config/master", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusMethodNotAllowed) @@ -256,8 +286,10 @@ func TestAPIServer(t *testing.T) { { name: "get healthz", request: httptest.NewRequest(http.MethodGet, "http://testrequest/healthz", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -268,8 +300,10 @@ func TestAPIServer(t *testing.T) { { name: "head healthz", request: httptest.NewRequest(http.MethodHead, "http://testrequest/healthz", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusOK) @@ -280,8 +314,10 @@ func TestAPIServer(t *testing.T) { { name: "post healthz", request: httptest.NewRequest(http.MethodPost, "http://testrequest/healthz", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusMethodNotAllowed) @@ -292,8 +328,10 @@ func TestAPIServer(t *testing.T) { { name: "get root", request: httptest.NewRequest(http.MethodGet, "http://testrequest/", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusNotFound) @@ -304,8 +342,10 @@ func TestAPIServer(t *testing.T) { { name: "head root", request: httptest.NewRequest(http.MethodHead, "http://testrequest/", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusNotFound) @@ -316,8 +356,10 @@ func TestAPIServer(t *testing.T) { { name: "post root", request: httptest.NewRequest(http.MethodPost, "http://testrequest/", nil), - serverFunc: func(poolRequest) (*igntypes.Config, error) { - return new(igntypes.Config), nil + serverFunc: func(poolRequest) (*runtime.RawExtension, error) { + return &runtime.RawExtension{ + Raw: helpers.MarshalOrDie(new(igntypes.Config)), + }, nil }, checkResponse: func(t *testing.T, response *http.Response) { checkStatus(t, response, http.StatusMethodNotAllowed) diff --git a/pkg/server/bootstrap_server.go b/pkg/server/bootstrap_server.go index 399d41d3a9..cd7710b892 100644 --- a/pkg/server/bootstrap_server.go +++ b/pkg/server/bootstrap_server.go @@ -6,9 +6,9 @@ import ( "os" "path" - igntypes "github.com/coreos/ignition/config/v2_2/types" yaml "github.com/ghodss/yaml" "github.com/golang/glog" + "k8s.io/apimachinery/pkg/runtime" clientcmd "k8s.io/client-go/tools/clientcmd/api/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" @@ -55,7 +55,7 @@ func NewBootstrapServer(dir, kubeconfig string) (Server, error) { // 3. Load the machine config. // 4. Append the machine annotations file. // 5. Append the KubeConfig file. -func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*igntypes.Config, error) { +func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*runtime.RawExtension, error) { if cr.machineConfigPool != "master" { return nil, fmt.Errorf("refusing to serve bootstrap configuration to pool %q", cr.machineConfigPool) } @@ -104,7 +104,12 @@ func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*igntypes.Config, error) } } - return machineConfigToIgnition(mc), nil + rawIgn, err := machineConfigToRawIgnition(mc) + if err != nil { + return nil, fmt.Errorf("server: could not convert MachineConfig to raw Ignition: %v", err) + } + + return rawIgn, nil } func kubeconfigFromFile(path string) ([]byte, []byte, error) { diff --git a/pkg/server/cluster_server.go b/pkg/server/cluster_server.go index 56bee8c129..34e93fd74b 100644 --- a/pkg/server/cluster_server.go +++ b/pkg/server/cluster_server.go @@ -5,10 +5,10 @@ import ( "io/ioutil" "path/filepath" - igntypes "github.com/coreos/ignition/config/v2_2/types" yaml "github.com/ghodss/yaml" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" rest "k8s.io/client-go/rest" clientcmd "k8s.io/client-go/tools/clientcmd" clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1" @@ -58,7 +58,7 @@ func NewClusterServer(kubeConfig, apiserverURL string) (Server, error) { // GetConfig fetches the machine config(type - Ignition) from the cluster, // based on the pool request. -func (cs *clusterServer) GetConfig(cr poolRequest) (*igntypes.Config, error) { +func (cs *clusterServer) GetConfig(cr poolRequest) (*runtime.RawExtension, error) { mp, err := cs.machineClient.MachineConfigPools().Get(cr.machineConfigPool, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("could not fetch pool. err: %v", err) @@ -77,7 +77,13 @@ func (cs *clusterServer) GetConfig(cr poolRequest) (*igntypes.Config, error) { return nil, err } } - return machineConfigToIgnition(mc), nil + + rawIgn, err := machineConfigToRawIgnition(mc) + if err != nil { + return nil, fmt.Errorf("server: could not convert MachineConfig to raw Ignition: %v", err) + } + + return rawIgn, nil } // getClientConfig returns a Kubernetes client Config. diff --git a/pkg/server/server.go b/pkg/server/server.go index 455caab041..3af099708b 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -5,11 +5,13 @@ import ( "fmt" "net/url" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/vincent-petithory/dataurl" + "k8s.io/apimachinery/pkg/runtime" ) const ( @@ -41,7 +43,7 @@ type appenderFunc func(*mcfgv1.MachineConfig) error // Server defines the interface that is implemented by different // machine config server implementations. type Server interface { - GetConfig(poolRequest) (*igntypes.Config, error) + GetConfig(poolRequest) (*runtime.RawExtension, error) } func getAppenders(currMachineConfig string, f kubeconfigFunc, osimageurl string) []appenderFunc { @@ -59,16 +61,24 @@ func getAppenders(currMachineConfig string, f kubeconfigFunc, osimageurl string) return appenders } -// machineConfigToIgnition converts a MachineConfig object into Ignition. -func machineConfigToIgnition(mccfg *mcfgv1.MachineConfig) *igntypes.Config { +// machineConfigToRawIgnition converts a MachineConfig object into raw Ignition. +func machineConfigToRawIgnition(mccfg *mcfgv1.MachineConfig) (*runtime.RawExtension, error) { tmpcfg := mccfg.DeepCopy() - tmpcfg.Spec.Config = ctrlcommon.NewIgnConfig() + tmpIgnCfg := ctrlcommon.NewIgnConfig() + rawTmpIgnCfg, err := json.Marshal(tmpIgnCfg) + if err != nil { + return nil, fmt.Errorf("error marshalling Ignition config: %v", err) + } + tmpcfg.Spec.Config.Raw = rawTmpIgnCfg serialized, err := json.Marshal(tmpcfg) if err != nil { - panic(err.Error()) + return nil, fmt.Errorf("error marshalling MachineConfig: %v", err) + } + err = appendFileToRawIgnition(&mccfg.Spec.Config, daemonconsts.MachineConfigEncapsulatedPath, string(serialized)) + if err != nil { + return nil, fmt.Errorf("error appending file to raw Ignition config: %v", err) } - appendFileToIgnition(&mccfg.Spec.Config, daemonconsts.MachineConfigEncapsulatedPath, string(serialized)) - return &mccfg.Spec.Config + return &mccfg.Spec.Config, nil } // Golang :cry: @@ -76,13 +86,20 @@ func boolToPtr(b bool) *bool { return &b } -func appendInitialPivot(conf *igntypes.Config, osimageurl string) error { +func appendInitialPivot(rawExt *runtime.RawExtension, osimageurl string) error { if osimageurl == "" { return nil } // Tell pivot.service to pivot early - appendFileToIgnition(conf, daemonconsts.EtcPivotFile, osimageurl+"\n") + err := appendFileToRawIgnition(rawExt, daemonconsts.EtcPivotFile, osimageurl+"\n") + if err != nil { + return err + } + conf, report, err := ign.Parse(rawExt.Raw) + if err != nil { + return fmt.Errorf("failed to append initial pivot. Parsing Ignition config failed with error: %v\nReport: %v", err, report) + } // Awful hack to create a file in /run // https://github.com/openshift/machine-config-operator/pull/363#issuecomment-463397373 // "So one gotcha here is that Ignition will actually write `/run/pivot/image-pullspec` to the filesystem rather than the `/run` tmpfs" @@ -101,6 +118,10 @@ ExecStart=/bin/sh -c 'mkdir /run/pivot && touch /run/pivot/reboot-needed' WantedBy=multi-user.target `} conf.Systemd.Units = append(conf.Systemd.Units, unit) + rawExt.Raw, err = json.Marshal(conf) + if err != nil { + return err + } return nil } @@ -113,25 +134,32 @@ func appendInitialMachineConfig(mc *mcfgv1.MachineConfig) error { if err != nil { return err } - appendFileToIgnition(&mc.Spec.Config, machineConfigContentPath, string(mcJSON)) + appendFileToRawIgnition(&mc.Spec.Config, machineConfigContentPath, string(mcJSON)) return nil } -func appendKubeConfig(conf *igntypes.Config, f kubeconfigFunc) error { +func appendKubeConfig(rawExt *runtime.RawExtension, f kubeconfigFunc) error { kcData, _, err := f() if err != nil { return err } - appendFileToIgnition(conf, defaultMachineKubeConfPath, string(kcData)) + err = appendFileToRawIgnition(rawExt, defaultMachineKubeConfPath, string(kcData)) + if err != nil { + return err + } return nil } -func appendNodeAnnotations(conf *igntypes.Config, currConf string) error { +func appendNodeAnnotations(rawExt *runtime.RawExtension, currConf string) error { + anno, err := getNodeAnnotation(currConf) if err != nil { return err } - appendFileToIgnition(conf, daemonconsts.InitialNodeAnnotationsFilePath, anno) + err = appendFileToRawIgnition(rawExt, daemonconsts.InitialNodeAnnotationsFilePath, anno) + if err != nil { + return err + } return nil } @@ -148,7 +176,11 @@ func getNodeAnnotation(conf string) (string, error) { return string(contents), nil } -func appendFileToIgnition(conf *igntypes.Config, outPath, contents string) { +func appendFileToRawIgnition(rawExt *runtime.RawExtension, outPath, contents string) error { + conf, report, err := ign.Parse(rawExt.Raw) + if err != nil { + return fmt.Errorf("failed to append file. Parsing Ignition config failed with error: %v\nReport: %v", err, report) + } fileMode := int(420) file := igntypes.File{ Node: igntypes.Node{ @@ -166,6 +198,11 @@ func appendFileToIgnition(conf *igntypes.Config, outPath, contents string) { conf.Storage.Files = make([]igntypes.File, 0) } conf.Storage.Files = append(conf.Storage.Files, file) + rawExt.Raw, err = json.Marshal(conf) + if err != nil { + return err + } + return nil } func getDecodedContent(inp string) (string, error) { diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 69683d7afc..2ba4215642 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/types" yaml "github.com/ghodss/yaml" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" @@ -49,23 +50,42 @@ func TestStringEncode(t *testing.T) { } } -func TestMachineConfigToIgnition(t *testing.T) { +func TestMachineConfigToRawIgnition(t *testing.T) { mcPath := filepath.Join(testDir, "machine-configs", testConfig+".yaml") mcData, err := ioutil.ReadFile(mcPath) assert.Nil(t, err) mc := new(mcfgv1.MachineConfig) err = yaml.Unmarshal([]byte(mcData), mc) assert.Nil(t, err) - assert.Equal(t, 1, len(mc.Spec.Config.Storage.Files)) - assert.Equal(t, mc.Spec.Config.Storage.Files[0].Path, "/etc/coreos/update.conf") + mcIgnCfg, _, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + t.Errorf("decoding Ignition Config failed: %s", err) + } + assert.Equal(t, 1, len(mcIgnCfg.Storage.Files)) + assert.Equal(t, mcIgnCfg.Storage.Files[0].Path, "/etc/coreos/update.conf") origMc := mc.DeepCopy() - ign := machineConfigToIgnition(mc) - assert.Equal(t, 1, len(origMc.Spec.Config.Storage.Files)) - assert.Equal(t, 2, len(mc.Spec.Config.Storage.Files)) - assert.Equal(t, 2, len(ign.Storage.Files)) - assert.Equal(t, ign.Storage.Files[0].Path, "/etc/coreos/update.conf") - assert.Equal(t, ign.Storage.Files[1].Path, daemonconsts.MachineConfigEncapsulatedPath) + origIgnCfg, _, err := ign.Parse(origMc.Spec.Config.Raw) + if err != nil { + t.Errorf("decoding Ignition Config failed: %s", err) + } + rawIgn, err := machineConfigToRawIgnition(mc) + if err != nil { + t.Errorf("converting MachineConfig to raw Ignition config failed: %s", err) + } + ignCfg, _, err := ign.Parse(rawIgn.Raw) + if err != nil { + t.Errorf("decoding Ignition Config failed: %s", err) + } + mcIgnCfg, _, err = ign.Parse(mc.Spec.Config.Raw) + if err != nil { + t.Errorf("decoding Ignition Config failed: %s", err) + } + assert.Equal(t, 1, len(origIgnCfg.Storage.Files)) + assert.Equal(t, 2, len(mcIgnCfg.Storage.Files)) + assert.Equal(t, 2, len(ignCfg.Storage.Files)) + assert.Equal(t, ignCfg.Storage.Files[0].Path, "/etc/coreos/update.conf") + assert.Equal(t, ignCfg.Storage.Files[1].Path, daemonconsts.MachineConfigEncapsulatedPath) } // TestBootstrapServer tests the behavior of the machine config server @@ -104,12 +124,18 @@ func TestBootstrapServer(t *testing.T) { if err != nil { t.Fatal(err) } - appendFileToIgnition(&mc.Spec.Config, defaultMachineKubeConfPath, string(kc)) + err = appendFileToRawIgnition(&mc.Spec.Config, defaultMachineKubeConfPath, string(kc)) + if err != nil { + t.Fatalf("unexpected error while appending file to ignition: %v", err) + } anno, err := getNodeAnnotation(mp.Status.Configuration.Name) if err != nil { t.Fatalf("unexpected error while creating annotations err: %v", err) } - appendFileToIgnition(&mc.Spec.Config, daemonconsts.InitialNodeAnnotationsFilePath, anno) + err = appendFileToRawIgnition(&mc.Spec.Config, daemonconsts.InitialNodeAnnotationsFilePath, anno) + if err != nil { + t.Fatalf("unexpected error while appending file to ignition: %v", err) + } // initialize bootstrap server and get config. bs := &bootstrapServer{ @@ -127,8 +153,16 @@ func TestBootstrapServer(t *testing.T) { } // assert on the output. - validateIgnitionFiles(t, mc.Spec.Config.Storage.Files, res.Storage.Files) - validateIgnitionSystemd(t, mc.Spec.Config.Systemd.Units, res.Systemd.Units) + ignCfg, _, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + t.Fatal(err) + } + resCfg, _, err := ign.Parse(res.Raw) + if err != nil { + t.Fatal(err) + } + validateIgnitionFiles(t, ignCfg.Storage.Files, resCfg.Storage.Files) + validateIgnitionSystemd(t, ignCfg.Systemd.Units, resCfg.Systemd.Units) // verify bootstrap cannot serve ignition to other pool than master res, err = bs.GetConfig(poolRequest{ @@ -195,12 +229,18 @@ func TestClusterServer(t *testing.T) { if err != nil { t.Fatal(err) } - appendFileToIgnition(&mc.Spec.Config, defaultMachineKubeConfPath, string(kc)) + err = appendFileToRawIgnition(&mc.Spec.Config, defaultMachineKubeConfPath, string(kc)) + if err != nil { + t.Fatalf("unexpected error while appending file to ignition: %v", err) + } anno, err := getNodeAnnotation(mp.Status.Configuration.Name) if err != nil { t.Fatalf("unexpected error while creating annotations err: %v", err) } - appendFileToIgnition(&mc.Spec.Config, daemonconsts.InitialNodeAnnotationsFilePath, anno) + err = appendFileToRawIgnition(&mc.Spec.Config, daemonconsts.InitialNodeAnnotationsFilePath, anno) + if err != nil { + t.Fatalf("unexpected error while appending file to ignition: %v", err) + } res, err := csc.GetConfig(poolRequest{ machineConfigPool: testPool, @@ -209,11 +249,19 @@ func TestClusterServer(t *testing.T) { t.Fatalf("expected err to be nil, received: %v", err) } - validateIgnitionFiles(t, mc.Spec.Config.Storage.Files, res.Storage.Files) - validateIgnitionSystemd(t, mc.Spec.Config.Systemd.Units, res.Systemd.Units) + ignCfg, _, err := ign.Parse(mc.Spec.Config.Raw) + if err != nil { + t.Fatal(err) + } + resCfg, _, err := ign.Parse(res.Raw) + if err != nil { + t.Fatal(err) + } + validateIgnitionFiles(t, ignCfg.Storage.Files, resCfg.Storage.Files) + validateIgnitionSystemd(t, ignCfg.Systemd.Units, resCfg.Systemd.Units) foundEncapsulated := false - for _, f := range res.Storage.Files { + for _, f := range resCfg.Storage.Files { if f.Path != daemonconsts.MachineConfigEncapsulatedPath { continue } diff --git a/pkg/server/testdata/machine-configs/test-config.yaml b/pkg/server/testdata/machine-configs/test-config.yaml index 998b6c4a9e..ebd9e1a656 100644 --- a/pkg/server/testdata/machine-configs/test-config.yaml +++ b/pkg/server/testdata/machine-configs/test-config.yaml @@ -9,6 +9,7 @@ spec: - nosmt config: ignition: + version: "2.2.0" storage: files: - diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index 45703f9d9b..97f887e84b 100644 --- a/test/e2e/mcd_test.go +++ b/test/e2e/mcd_test.go @@ -6,17 +6,20 @@ import ( "testing" "time" + ign "github.com/coreos/ignition/config/v2_2" igntypes "github.com/coreos/ignition/config/v2_2/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/constants" "github.com/openshift/machine-config-operator/test/e2e/framework" + "github.com/openshift/machine-config-operator/test/helpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" ) @@ -66,6 +69,9 @@ func createIgnFile(path, content, fs string, mode int) igntypes.File { Node: igntypes.Node{ Filesystem: fs, Path: path, + User: &igntypes.NodeUser{ + Name: "root", + }, }, } } @@ -76,7 +82,8 @@ func createMCToAddFileForRole(name, role, filename, data, fs string) *mcfgv1.Mac ignConfig := ctrlcommon.NewIgnConfig() ignFile := createIgnFile(filename, "data:,"+data, fs, 420) ignConfig.Storage.Files = append(ignConfig.Storage.Files, ignFile) - mcadd.Spec.Config = ignConfig + rawIgnConfig := helpers.MarshalOrDie(ignConfig) + mcadd.Spec.Config.Raw = rawIgnConfig return mcadd } @@ -153,7 +160,8 @@ func TestUpdateSSH(t *testing.T) { } ignConfig := ctrlcommon.NewIgnConfig() ignConfig.Passwd.Users = append(ignConfig.Passwd.Users, tempUser) - mcadd.Spec.Config = ignConfig + rawIgnConfig := helpers.MarshalOrDie(ignConfig) + mcadd.Spec.Config.Raw = rawIgnConfig _, err := cs.MachineConfigs().Create(mcadd) require.Nil(t, err, "failed to create MC") @@ -188,7 +196,9 @@ func TestKernelArguments(t *testing.T) { Labels: mcLabelForWorkers(), }, Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(ctrlcommon.NewIgnConfig()), + }, KernelArguments: []string{"nosmt", "foo=bar"}, }, } @@ -229,7 +239,9 @@ func TestKernelType(t *testing.T) { Labels: mcLabelForWorkers(), }, Spec: mcfgv1.MachineConfigSpec{ - Config: ctrlcommon.NewIgnConfig(), + Config: runtime.RawExtension{ + Raw: helpers.MarshalOrDie(ctrlcommon.NewIgnConfig()), + }, KernelType: "realtime", }, } @@ -285,11 +297,15 @@ func TestKernelType(t *testing.T) { func TestPoolDegradedOnFailToRender(t *testing.T) { cs := framework.NewClientSet("") - mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test", "") - mcadd.Spec.Config.Ignition.Version = "" // invalid, won't render + mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test", "root") + ignCfg, _, err := ign.Parse(mcadd.Spec.Config.Raw) + require.Nil(t, err, "failed to parse ignition config") + ignCfg.Ignition.Version = "" // invalid, won't render + rawIgnCfg := helpers.MarshalOrDie(ignCfg) + mcadd.Spec.Config.Raw = rawIgnCfg // create the dummy MC now - _, err := cs.MachineConfigs().Create(mcadd) + _, err = cs.MachineConfigs().Create(mcadd) require.Nil(t, err, "failed to create machine config") // verify the pool goes degraded @@ -331,7 +347,9 @@ func TestReconcileAfterBadMC(t *testing.T) { // create a MC that contains a valid ignition config but is not reconcilable mcadd := createMCToAddFile("add-a-file", "/etc/mytestconfs", "test", "root") - mcadd.Spec.Config.Networkd = igntypes.Networkd{ + ignCfg, _, err := ign.Parse(mcadd.Spec.Config.Raw) + require.Nil(t, err, "failed to parse ignition config") + ignCfg.Networkd = igntypes.Networkd{ Units: []igntypes.Networkdunit{ { Name: "test.network", @@ -339,11 +357,13 @@ func TestReconcileAfterBadMC(t *testing.T) { }, }, } + rawIgnCfg := helpers.MarshalOrDie(ignCfg) + mcadd.Spec.Config.Raw = rawIgnCfg workerOldMc := getMcName(t, cs, "worker") // create the dummy MC now - _, err := cs.MachineConfigs().Create(mcadd) + _, err = cs.MachineConfigs().Create(mcadd) if err != nil { t.Errorf("failed to create machine config %v", err) } diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 50af737bf5..5ffad9af23 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -9,6 +9,7 @@ import ( mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/test/e2e/framework" + "github.com/openshift/machine-config-operator/test/helpers" "github.com/pkg/errors" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -153,13 +154,7 @@ func mcpNameToRole(mcpName string) string { // createMC creates a machine config object with name and role func createMC(name, role string) *mcfgv1.MachineConfig { - // create a dummy MC - mcadd := &mcfgv1.MachineConfig{} - mcadd.ObjectMeta = metav1.ObjectMeta{ - Name: name, - Labels: mcLabelForRole(role), - } - return mcadd + return helpers.NewMachineConfig(name, mcLabelForRole(role), "", nil) } // execCmdOnNode finds a node's mcd, and oc rsh's into it to execute a command on the node diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go index 3534f03d0c..ff03dc98ca 100644 --- a/test/helpers/helpers.go +++ b/test/helpers/helpers.go @@ -1,9 +1,12 @@ package helpers import ( + "encoding/json" + igntypes "github.com/coreos/ignition/config/v2_2/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" @@ -24,6 +27,17 @@ func NewMachineConfig(name string, labels map[string]string, osurl string, files if labels == nil { labels = map[string]string{} } + rawIgnition := MarshalOrDie( + &igntypes.Config{ + Ignition: igntypes.Ignition{ + Version: igntypes.MaxVersion.String(), + }, + Storage: igntypes.Storage{ + Files: files, + }, + }, + ) + return &mcfgv1.MachineConfig{ TypeMeta: metav1.TypeMeta{ APIVersion: mcfgv1.SchemeGroupVersion.String(), @@ -35,13 +49,8 @@ func NewMachineConfig(name string, labels map[string]string, osurl string, files }, Spec: mcfgv1.MachineConfigSpec{ OSImageURL: osurl, - Config: igntypes.Config{ - Ignition: igntypes.Ignition{ - Version: igntypes.MaxVersion.String(), - }, - Storage: igntypes.Storage{ - Files: files, - }, + Config: runtime.RawExtension{ + Raw: rawIgnition, }, }, } @@ -87,3 +96,23 @@ func NewMachineConfigPool(name string, mcSelector, nodeSelector *metav1.LabelSel }, } } + +// CreateMachineConfigFromIgnition returns a MachineConfig object from an Ignition config passed to it +func CreateMachineConfigFromIgnition(ignCfg interface{}) *mcfgv1.MachineConfig { + return &mcfgv1.MachineConfig{ + Spec: mcfgv1.MachineConfigSpec{ + Config: runtime.RawExtension{ + Raw: MarshalOrDie(ignCfg), + }, + }, + } +} + +// MarshalOrDie returns a marshalled interface or panics +func MarshalOrDie(input interface{}) []byte { + bytes, err := json.Marshal(input) + if err != nil { + panic(err) + } + return bytes +}