diff --git a/pkg/asset/ignition/machine/master_ignition_customizations.go b/pkg/asset/ignition/machine/master_ignition_customizations.go index 6f42fc612f5..01f1973255b 100644 --- a/pkg/asset/ignition/machine/master_ignition_customizations.go +++ b/pkg/asset/ignition/machine/master_ignition_customizations.go @@ -8,6 +8,7 @@ import ( "github.com/sirupsen/logrus" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/ignition" "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/tls" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" @@ -45,10 +46,12 @@ func (a *MasterIgnitionCustomizations) Generate(dependencies asset.Parents) erro defaultPointerIgnition := pointerIgnitionConfig(installConfig.Config, rootCA.Cert(), "master") savedPointerIgnition := master.Config - if savedPointerIgnition != defaultPointerIgnition { + savedPointerIgnitionJson, err := ignition.Marshal(savedPointerIgnition) + defaultPointerIgnitionJson, err := ignition.Marshal(defaultPointerIgnition) + if string(savedPointerIgnitionJson) != string(defaultPointerIgnitionJson) { logrus.Infof("Master pointer ignition was modified. Saving contents to a machineconfig") mc := &mcfgv1.MachineConfig{} - mc, err := generatePointerMachineConfig(*savedPointerIgnition, "master") + mc, err = generatePointerMachineConfig(*savedPointerIgnition, "master") if err != nil { return errors.Wrap(err, "failed to generate master installer machineconfig") } diff --git a/pkg/asset/ignition/machine/master_ignition_customizations_test.go b/pkg/asset/ignition/machine/master_ignition_customizations_test.go index 9bfca8f7469..c770ea7f65e 100644 --- a/pkg/asset/ignition/machine/master_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/master_ignition_customizations_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/ignition" "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/tls" "github.com/openshift/installer/pkg/ipnet" @@ -15,36 +16,67 @@ import ( // TestMasterIgnitionCustomizationsGenerate tests generating the master ignition check asset. func TestMasterIgnitionCustomizationsGenerate(t *testing.T) { - installConfig := &installconfig.InstallConfig{ - Config: &types.InstallConfig{ - Networking: &types.Networking{ - ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, - }, - Platform: types.Platform{ - AWS: &aws.Platform{ - Region: "us-east", - }, - }, + cases := []struct { + name string + customize bool + assetExpected bool + }{ + { + name: "not customized", + customize: false, + assetExpected: false, + }, + { + name: "pointer customized", + customize: true, + assetExpected: true, }, } - rootCA := &tls.RootCA{} - err := rootCA.Generate(nil) - assert.NoError(t, err, "unexpected error generating root CA") + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + installConfig := &installconfig.InstallConfig{ + Config: &types.InstallConfig{ + Networking: &types.Networking{ + ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + Region: "us-east", + }, + }, + }, + } - parents := asset.Parents{} - parents.Add(installConfig, rootCA) + rootCA := &tls.RootCA{} + err := rootCA.Generate(nil) + assert.NoError(t, err, "unexpected error generating root CA") - master := &Master{} - err = master.Generate(parents) - assert.NoError(t, err, "unexpected error generating master asset") + parents := asset.Parents{} + parents.Add(installConfig, rootCA) + + master := &Master{} + err = master.Generate(parents) + assert.NoError(t, err, "unexpected error generating master asset") - parents.Add(master) - masterIgnCheck := &MasterIgnitionCustomizations{} - err = masterIgnCheck.Generate(parents) - assert.NoError(t, err, "unexpected error generating master ignition check asset") + if tc.customize == true { + // Modify the master config, emulating a customization to the pointer + master.Config.Storage.Files = append(master.Config.Storage.Files, + ignition.FileFromString("/etc/foo", "root", 0644, "foo")) + } - actualFiles := masterIgnCheck.Files() - assert.Equal(t, 1, len(actualFiles), "unexpected number of files in master state") - assert.Equal(t, masterMachineConfigFileName, actualFiles[0].Filename, "unexpected name for master ignition config") + parents.Add(master) + masterIgnCheck := &MasterIgnitionCustomizations{} + err = masterIgnCheck.Generate(parents) + assert.NoError(t, err, "unexpected error generating master ignition check asset") + + actualFiles := masterIgnCheck.Files() + if tc.assetExpected == true { + assert.Equal(t, 1, len(actualFiles), "unexpected number of files in master state") + assert.Equal(t, masterMachineConfigFileName, actualFiles[0].Filename, "unexpected name for master ignition config") + } else { + assert.Equal(t, 0, len(actualFiles), "unexpected number of files in master state") + } + }) + } } diff --git a/pkg/asset/ignition/machine/worker_ignition_customizations.go b/pkg/asset/ignition/machine/worker_ignition_customizations.go index f74e4f3fa1b..fb455f03d36 100644 --- a/pkg/asset/ignition/machine/worker_ignition_customizations.go +++ b/pkg/asset/ignition/machine/worker_ignition_customizations.go @@ -8,6 +8,7 @@ import ( "github.com/sirupsen/logrus" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/ignition" "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/tls" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" @@ -46,10 +47,12 @@ func (a *WorkerIgnitionCustomizations) Generate(dependencies asset.Parents) erro savedPointerIgnition := worker.Config // Create a machineconfig if the ignition has been modified - if savedPointerIgnition != defaultPointerIgnition { + savedPointerIgnitionJson, err := ignition.Marshal(savedPointerIgnition) + defaultPointerIgnitionJson, err := ignition.Marshal(defaultPointerIgnition) + if string(savedPointerIgnitionJson) != string(defaultPointerIgnitionJson) { logrus.Infof("Worker pointer ignition was modified. Saving contents to a machineconfig") mc := &mcfgv1.MachineConfig{} - mc, err := generatePointerMachineConfig(*savedPointerIgnition, "worker") + mc, err = generatePointerMachineConfig(*savedPointerIgnition, "worker") if err != nil { return errors.Wrap(err, "failed to generate worker installer machineconfig") } diff --git a/pkg/asset/ignition/machine/worker_ignition_customizations_test.go b/pkg/asset/ignition/machine/worker_ignition_customizations_test.go index 5ea7d08e596..3c9ff274cd3 100644 --- a/pkg/asset/ignition/machine/worker_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/worker_ignition_customizations_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/ignition" "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/tls" "github.com/openshift/installer/pkg/ipnet" @@ -15,36 +16,67 @@ import ( // TestWorkerIgnitionCustomizationsGenerate tests generating the worker ignition check asset. func TestWorkerIgnitionCustomizationsGenerate(t *testing.T) { - installConfig := &installconfig.InstallConfig{ - Config: &types.InstallConfig{ - Networking: &types.Networking{ - ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, - }, - Platform: types.Platform{ - AWS: &aws.Platform{ - Region: "us-east", - }, - }, + cases := []struct { + name string + customize bool + assetExpected bool + }{ + { + name: "not customized", + customize: false, + assetExpected: false, + }, + { + name: "pointer customized", + customize: true, + assetExpected: true, }, } - rootCA := &tls.RootCA{} - err := rootCA.Generate(nil) - assert.NoError(t, err, "unexpected error generating root CA") + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + installConfig := &installconfig.InstallConfig{ + Config: &types.InstallConfig{ + Networking: &types.Networking{ + ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + Region: "us-east", + }, + }, + }, + } - parents := asset.Parents{} - parents.Add(installConfig, rootCA) + rootCA := &tls.RootCA{} + err := rootCA.Generate(nil) + assert.NoError(t, err, "unexpected error generating root CA") - worker := &Worker{} - err = worker.Generate(parents) - assert.NoError(t, err, "unexpected error generating worker asset") + parents := asset.Parents{} + parents.Add(installConfig, rootCA) + + worker := &Worker{} + err = worker.Generate(parents) + assert.NoError(t, err, "unexpected error generating worker asset") - parents.Add(worker) - workerIgnCheck := &WorkerIgnitionCustomizations{} - err = workerIgnCheck.Generate(parents) - assert.NoError(t, err, "unexpected error generating worker ignition check asset") + if tc.customize == true { + // Modify the worker config, emulating a customization to the pointer + worker.Config.Storage.Files = append(worker.Config.Storage.Files, + ignition.FileFromString("/etc/foo", "root", 0644, "foo")) + } - actualFiles := workerIgnCheck.Files() - assert.Equal(t, 1, len(actualFiles), "unexpected number of files in worker state") - assert.Equal(t, workerMachineConfigFileName, actualFiles[0].Filename, "unexpected name for worker ignition config") + parents.Add(worker) + workerIgnCheck := &WorkerIgnitionCustomizations{} + err = workerIgnCheck.Generate(parents) + assert.NoError(t, err, "unexpected error generating worker ignition check asset") + + actualFiles := workerIgnCheck.Files() + if tc.assetExpected == true { + assert.Equal(t, 1, len(actualFiles), "unexpected number of files in worker state") + assert.Equal(t, workerMachineConfigFileName, actualFiles[0].Filename, "unexpected name for worker ignition config") + } else { + assert.Equal(t, 0, len(actualFiles), "unexpected number of files in worker state") + } + }) + } }