diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go b/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go index 62cbd0bef1..3bdca61775 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/helpers.go @@ -12,26 +12,21 @@ import ( // MergeMachineConfigs combines multiple machineconfig objects into one object. // It sorts all the configs in increasing order of their name. // It uses the Ign config from first object as base and appends all the rest. -// It uses the first non-empty OSImageURL. -func MergeMachineConfigs(configs []*MachineConfig) *MachineConfig { +// 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 }) - outOSImageURL := configs[0].Spec.OSImageURL outIgn := configs[0].Spec.Config for idx := 1; idx < len(configs); idx++ { - config := configs[idx] - if outOSImageURL == "" { - outOSImageURL = config.Spec.OSImageURL - } - outIgn = ignv2_2.Append(outIgn, config.Spec.Config) + outIgn = ignv2_2.Append(outIgn, configs[idx].Spec.Config) } return &MachineConfig{ Spec: MachineConfigSpec{ - OSImageURL: outOSImageURL, + OSImageURL: osImageURL, Config: outIgn, }, } diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/helpers_test.go b/pkg/apis/machineconfiguration.openshift.io/v1/helpers_test.go index be31c91c29..23f8cc5f25 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/helpers_test.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/helpers_test.go @@ -122,51 +122,6 @@ func TestRemoveMachineConfigPoolCondition(t *testing.T) { }) } } - -func TestMergeMachineConfigs(t *testing.T) { - var configs []*MachineConfig - configs = append(configs, &MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "4"}, - Spec: MachineConfigSpec{ - OSImageURL: "", - }, - }) - - configs = append(configs, &MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "3"}, - Spec: MachineConfigSpec{ - OSImageURL: "example.com/os@sha256:notthetarget", - }, - }) - - configs = append(configs, &MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "0"}, - Spec: MachineConfigSpec{ - OSImageURL: "", - }, - }) - - targetOSImageURL := "example.com/os@sha256:thetarget" - configs = append(configs, &MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "1"}, - Spec: MachineConfigSpec{ - OSImageURL: targetOSImageURL, - }, - }) - - configs = append(configs, &MachineConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "2"}, - Spec: MachineConfigSpec{ - OSImageURL: "", - }, - }) - - merged := MergeMachineConfigs(configs) - if merged.Spec.OSImageURL != targetOSImageURL { - t.Errorf("OSImageURL expected: %s, received: %s", targetOSImageURL, merged.Spec.OSImageURL) - } -} - func TestIsControllerConfigCompleted(t *testing.T) { tests := []struct { obsrvdGen int64 diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index aabb861e0f..0814643b1c 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -97,7 +97,7 @@ func (b *Bootstrap) Run(destDir string) error { } configs = append(configs, iconfigs...) - fpools, gconfigs, err := render.RunBootstrap(pools, configs) + fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig) if err != nil { return err } diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go index fdeedcdff1..dd6f9af86e 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -466,7 +466,15 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo return nil } - generated, err := generateRenderedMachineConfig(pool, configs) + cc, err := ctrl.ccLister.List(labels.Everything()) + if err != nil { + return fmt.Errorf("could not enumerate ControllerConfig %s", err) + } + if len(cc) == 0 { + return fmt.Errorf("ControllerConfigList is empty") + } + + generated, err := generateRenderedMachineConfig(pool, configs, cc[0]) if err != nil { return err } @@ -505,7 +513,7 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo } // generateRenderedMachineConfig takes all MCs for a given pool and returns a single rendered MC. For ex master-XXXX or worker-XXXX -func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig) (*mcfgv1.MachineConfig, error) { +func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) { // Before merging all MCs for a specific pool, let's make sure each contains a valid Ignition Config for _, config := range configs { rpt := validate.ValidateWithoutSource(reflect.ValueOf(config.Spec.Config.Ignition)) @@ -513,7 +521,7 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc return nil, fmt.Errorf("machine config: %v contains invalid ignition config: %v", config.ObjectMeta.Name, rpt) } } - merged := mcfgv1.MergeMachineConfigs(configs) + merged := mcfgv1.MergeMachineConfigs(configs, cconfig.Spec.OSImageURL) hashedName, err := getMachineConfigHashedName(pool, merged) if err != nil { return nil, err @@ -533,7 +541,7 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc // RunBootstrap runs the render controller in bootstrap mode. // For each pool, it matches the machineconfigs based on label selector and // returns the generated machineconfigs and pool with CurrentMachineConfig status field set. -func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig) ([]*mcfgv1.MachineConfigPool, []*mcfgv1.MachineConfig, error) { +func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfigPool, []*mcfgv1.MachineConfig, error) { var ( opools []*mcfgv1.MachineConfigPool oconfigs []*mcfgv1.MachineConfig @@ -544,7 +552,7 @@ func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineCo return nil, nil, err } - generated, err := generateRenderedMachineConfig(pool, pcs) + generated, err := generateRenderedMachineConfig(pool, pcs, cconfig) if err != nil { return nil, nil, err } diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index d393bc8794..0d7b211998 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -11,6 +11,7 @@ import ( ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" informers "github.com/openshift/machine-config-operator/pkg/generated/informers/externalversions" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -84,8 +85,9 @@ func (f *fixture) newController() *Controller { f.client = fake.NewSimpleClientset(f.objects...) i := informers.NewSharedInformerFactory(f.client, noResyncPeriodFunc()) - c := New(i.Machineconfiguration().V1().MachineConfigPools(), i.Machineconfiguration().V1().MachineConfigs(), i.Machineconfiguration().V1().ControllerConfigs(), - k8sfake.NewSimpleClientset(), f.client) + + c := New(i.Machineconfiguration().V1().MachineConfigPools(), i.Machineconfiguration().V1().MachineConfigs(), + i.Machineconfiguration().V1().ControllerConfigs(), k8sfake.NewSimpleClientset(), f.client) c.mcpListerSynced = alwaysReady c.mcListerSynced = alwaysReady @@ -107,6 +109,10 @@ func (f *fixture) newController() *Controller { i.Machineconfiguration().V1().MachineConfigs().Informer().GetIndexer().Add(m) } + for _, m := range f.ccLister { + i.Machineconfiguration().V1().ControllerConfigs().Informer().GetIndexer().Add(m) + } + return c } @@ -235,6 +241,7 @@ func newControllerConfig(name string) *mcfgv1.ControllerConfig { ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(utilrand.String(5))}, Spec: mcfgv1.ControllerConfigSpec{ EtcdDiscoveryDomain: fmt.Sprintf("%s.tt.testing", name), + OSImageURL: "dummy", }, Status: mcfgv1.ControllerConfigStatus{ Conditions: []mcfgv1.ControllerConfigStatusCondition{ @@ -263,9 +270,9 @@ func TestCreatesGeneratedMachineConfig(t *testing.T) { newMachineConfig("00-test-cluster-master", map[string]string{"node-role": "master"}, "dummy://", []ignv2_2types.File{files[0]}), newMachineConfig("05-extra-master", map[string]string{"node-role": "master"}, "dummy://1", []ignv2_2types.File{files[1]}), } - cc := newControllerConfig("test-cluster-configcreate") - f.ccLister = append(f.ccLister, cc) + cc := newControllerConfig("test") + f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp) f.objects = append(f.objects, mcp) f.mcLister = append(f.mcLister, mcs...) @@ -291,13 +298,16 @@ func TestIgnValidationGenerateRenderedMachineConfig(t *testing.T) { newMachineConfig("00-test-cluster-master", map[string]string{"node-role": "master"}, "dummy://", []ignv2_2types.File{files[0]}), newMachineConfig("05-extra-master", map[string]string{"node-role": "master"}, "dummy://1", []ignv2_2types.File{files[1]}), } - _, err := generateRenderedMachineConfig(mcp, mcs) + cc := newControllerConfig("test") + + + _, err := generateRenderedMachineConfig(mcp, mcs, cc) if err != nil { t.Fatalf("expected no error. Got: %v", err) } mcs[1].Spec.Config.Ignition.Version = "" - _, err = generateRenderedMachineConfig(mcp, mcs) + _, err = generateRenderedMachineConfig(mcp, mcs, cc) if err == nil { t.Fatalf("expected error. mcs contains a machine config with invalid ignconfig version") } @@ -319,14 +329,15 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) { newMachineConfig("00-test-cluster-master", map[string]string{"node-role": "master"}, "dummy://", []ignv2_2types.File{files[0]}), newMachineConfig("05-extra-master", map[string]string{"node-role": "master"}, "dummy://1", []ignv2_2types.File{files[1]}), } - gmc, err := generateRenderedMachineConfig(mcp, mcs) + cc := newControllerConfig("test") + + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) if err != nil { t.Fatal(err) } gmc.Spec.OSImageURL = "why-did-you-change-it" mcp.Status.Configuration.Name = gmc.Name - cc := newControllerConfig("test-cluster-configcreate") f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp) @@ -338,7 +349,7 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) { f.mcLister = append(f.mcLister, gmc) f.objects = append(f.objects, gmc) - expmc, err := generateRenderedMachineConfig(mcp, mcs) + expmc, err := generateRenderedMachineConfig(mcp, mcs, cc) if err != nil { t.Fatal(err) } @@ -349,6 +360,22 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) { f.run(getKey(mcp, t)) } +func TestGenerateMachineConfigNoOverrideOSImageURL(t *testing.T) { + mcp := newMachineConfigPool("test-cluster-master", metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role", "master"), "") + mcs := []*mcfgv1.MachineConfig{ + newMachineConfig("00-test-cluster-master", map[string]string{"node-role": "master"}, "dummy-test-1", []ignv2_2types.File{}), + newMachineConfig("00-test-cluster-master-0", map[string]string{"node-role": "master"}, "dummy-change", []ignv2_2types.File{}), + } + + cc := newControllerConfig("test") + + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, "dummy", gmc.Spec.OSImageURL) +} + func TestDoNothing(t *testing.T) { f := newFixture(t) mcp := newMachineConfigPool("test-cluster-master", metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role", "master"), "") @@ -365,13 +392,14 @@ func TestDoNothing(t *testing.T) { newMachineConfig("00-test-cluster-master", map[string]string{"node-role": "master"}, "dummy://", []ignv2_2types.File{files[0]}), newMachineConfig("05-extra-master", map[string]string{"node-role": "master"}, "dummy://1", []ignv2_2types.File{files[1]}), } - gmc, err := generateRenderedMachineConfig(mcp, mcs) + cc := newControllerConfig("test") + + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) if err != nil { t.Fatal(err) } mcp.Status.Configuration.Name = gmc.Name - cc := newControllerConfig("test-cluster-configcreate") f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp)