Skip to content

Commit

Permalink
Merge pull request #4455 from hardys/pointer_mc_fix
Browse files Browse the repository at this point in the history 
Bug 1904663: Fix comparison in pointer ignition customization asset
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Dec 9, 2020
2 parents 35d7aa2 + 2da500a commit 83d52fb
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 54 deletions.
13 changes: 11 additions & 2 deletions pkg/asset/ignition/machine/master_ignition_customizations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -45,10 +46,18 @@ 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)
if err != nil {
return errors.Wrap(err, "failed to Marshal savedPointerIgnition")
}
defaultPointerIgnitionJSON, err := ignition.Marshal(defaultPointerIgnition)
if err != nil {
return errors.Wrap(err, "failed to 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")
}
Expand Down
82 changes: 57 additions & 25 deletions pkg/asset/ignition/machine/master_ignition_customizations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
},
},
},
}

rootCA := &tls.RootCA{}
err := rootCA.Generate(nil)
assert.NoError(t, err, "unexpected error generating root CA")

parents := asset.Parents{}
parents.Add(installConfig, rootCA)

parents := asset.Parents{}
parents.Add(installConfig, rootCA)
master := &Master{}
err = master.Generate(parents)
assert.NoError(t, err, "unexpected error generating master asset")

master := &Master{}
err = master.Generate(parents)
assert.NoError(t, err, "unexpected error generating master 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"))
}

parents.Add(master)
masterIgnCheck := &MasterIgnitionCustomizations{}
err = masterIgnCheck.Generate(parents)
assert.NoError(t, err, "unexpected error generating master ignition check asset")
parents.Add(master)
masterIgnCheck := &MasterIgnitionCustomizations{}
err = masterIgnCheck.Generate(parents)
assert.NoError(t, err, "unexpected error generating master ignition check asset")

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")
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")
}
})
}
}
13 changes: 11 additions & 2 deletions pkg/asset/ignition/machine/worker_ignition_customizations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -46,10 +47,18 @@ 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)
if err != nil {
return errors.Wrap(err, "failed Marshal savedPointerIgnition")
}
defaultPointerIgnitionJSON, err := ignition.Marshal(defaultPointerIgnition)
if err != nil {
return errors.Wrap(err, "failed 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")
}
Expand Down
82 changes: 57 additions & 25 deletions pkg/asset/ignition/machine/worker_ignition_customizations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
},
},
},
}

rootCA := &tls.RootCA{}
err := rootCA.Generate(nil)
assert.NoError(t, err, "unexpected error generating root CA")

parents := asset.Parents{}
parents.Add(installConfig, rootCA)

parents := asset.Parents{}
parents.Add(installConfig, rootCA)
worker := &Worker{}
err = worker.Generate(parents)
assert.NoError(t, err, "unexpected error generating worker asset")

worker := &Worker{}
err = worker.Generate(parents)
assert.NoError(t, err, "unexpected error generating worker 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"))
}

parents.Add(worker)
workerIgnCheck := &WorkerIgnitionCustomizations{}
err = workerIgnCheck.Generate(parents)
assert.NoError(t, err, "unexpected error generating worker ignition check asset")
parents.Add(worker)
workerIgnCheck := &WorkerIgnitionCustomizations{}
err = workerIgnCheck.Generate(parents)
assert.NoError(t, err, "unexpected error generating worker ignition check asset")

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")
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")
}
})
}
}

0 comments on commit 83d52fb

Please sign in to comment.