Skip to content

Commit

Permalink
Create a machineconfig for IPI pointer ignition customizations
Browse files Browse the repository at this point in the history
In the case where an IPI user customizes the pointer config, this
config is only persisted via the user-data secret, which was an
issue when moving to an MCO templated pointer config:

openshift#4228

It was also a problem for attempts for IPI baremetal to consume
the MCO rendered config directly, with the aim of enabling
network configuration via MachineConfig:

openshift#3589

With this new approach, we detect the case where a change has
been made to the pointer config by the user, and in that case
it is stored via a MachineConfig manifest, such that any
customizations are reflected in the MCO rendered config.

Implements: openshift/enhancements#540
Co-Authored-By: Steven Hardy <[email protected]>
  • Loading branch information
Kiran Thyagaraja and Steven Hardy committed Nov 24, 2020
1 parent ad31070 commit 484c6b2
Show file tree
Hide file tree
Showing 7 changed files with 325 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/asset/ignition/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/openshift/installer/pkg/asset/ignition"
"github.com/openshift/installer/pkg/asset/ignition/bootstrap/baremetal"
"github.com/openshift/installer/pkg/asset/ignition/bootstrap/vsphere"
mcign "github.com/openshift/installer/pkg/asset/ignition/machine"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/kubeconfig"
"github.com/openshift/installer/pkg/asset/machines"
Expand Down Expand Up @@ -82,6 +83,8 @@ func (a *Bootstrap) Dependencies() []asset.Asset {
&kubeconfig.AdminInternalClient{},
&kubeconfig.Kubelet{},
&kubeconfig.LoopbackClient{},
&mcign.MasterIgnitionCheck{},
&mcign.WorkerIgnitionCheck{},
&machines.Master{},
&machines.Worker{},
&manifests.Manifests{},
Expand Down Expand Up @@ -460,6 +463,8 @@ func (a *Bootstrap) addParentFiles(dependencies asset.Parents) {
&manifests.Openshift{},
&machines.Master{},
&machines.Worker{},
&mcign.MasterIgnitionCheck{},
&mcign.WorkerIgnitionCheck{},
} {
dependencies.Get(asset)

Expand Down
93 changes: 93 additions & 0 deletions pkg/asset/ignition/machine/masterignitioncheck.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package machine

import (
"os"

"github.com/ghodss/yaml"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"github.com/openshift/installer/pkg/asset"
"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"
)

const (
masterMachineConfigFileName = "manifests/99_openshift-installer-ignition_master.yaml"
)

// MasterIgnitionCheck is an asset that checks for any customizations a user might
// have made to the pointer ignition configs before creating the cluster. If customizations
// are made, then the updates are reconciled as a MachineConfig file
type MasterIgnitionCheck struct {
File *asset.File
}

var _ asset.WritableAsset = (*MasterIgnitionCheck)(nil)

// Dependencies returns the dependencies for MasterIgnitionCheck
func (a *MasterIgnitionCheck) Dependencies() []asset.Asset {
return []asset.Asset{
&installconfig.InstallConfig{},
&tls.RootCA{},
&Master{},
}
}

// Generate queries for input from the user.
func (a *MasterIgnitionCheck) Generate(dependencies asset.Parents) error {
installConfig := &installconfig.InstallConfig{}
rootCA := &tls.RootCA{}
master := &Master{}
dependencies.Get(installConfig, rootCA, master)

defaultPointerIgnition := pointerIgnitionConfig(installConfig.Config, rootCA.Cert(), "master")
savedPointerIgnition := master.Config

if savedPointerIgnition != defaultPointerIgnition {
logrus.Infof("Master pointer ignition was modified. Saving contents to a machineconfig")
mc := &mcfgv1.MachineConfig{}
mc, err := generatePointerMachineConfig(*savedPointerIgnition, "master")
if err != nil {
return errors.Wrap(err, "failed to generate master installer machineconfig")
}
configData, err := yaml.Marshal(mc)
if err != nil {
return errors.Wrap(err, "failed to marshal master installer machineconfig")
}
a.File = &asset.File{
Filename: masterMachineConfigFileName,
Data: configData,
}
}

return nil
}

// Name returns the human-friendly name of the asset.
func (a *MasterIgnitionCheck) Name() string {
return "Master Ignition Customization Check"
}

// Files returns the files generated by the asset.
func (a *MasterIgnitionCheck) Files() []*asset.File {
if a.File != nil {
return []*asset.File{a.File}
}
return []*asset.File{}
}

// Load returns the master ignitions from disk.
func (a *MasterIgnitionCheck) Load(f asset.FileFetcher) (found bool, err error) {
file, err := f.FetchByName(masterMachineConfigFileName)
if err != nil {
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
a.File = file

return true, nil
}
50 changes: 50 additions & 0 deletions pkg/asset/ignition/machine/masterignitioncheck_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package machine

import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/tls"
"github.com/openshift/installer/pkg/ipnet"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
)

// TestMasterIgnitionCheckGenerate tests generating the master ignition check asset.
func TestMasterIgnitionCheckGenerate(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)

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

parents.Add(master)
masterIgnCheck := &MasterIgnitionCheck{}
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")
}
31 changes: 31 additions & 0 deletions pkg/asset/ignition/machine/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@ import (
ignutil "github.com/coreos/ignition/v2/config/util"
igntypes "github.com/coreos/ignition/v2/config/v3_1/types"
"github.com/vincent-petithory/dataurl"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/openshift/installer/pkg/asset/ignition"
"github.com/openshift/installer/pkg/types"
baremetaltypes "github.com/openshift/installer/pkg/types/baremetal"
openstacktypes "github.com/openshift/installer/pkg/types/openstack"
ovirttypes "github.com/openshift/installer/pkg/types/ovirt"
vspheretypes "github.com/openshift/installer/pkg/types/vsphere"
mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
)

// pointerIgnitionConfig generates a config which references the remote config
Expand Down Expand Up @@ -61,3 +64,31 @@ func pointerIgnitionConfig(installConfig *types.InstallConfig, rootCA []byte, ro
},
}
}

// generatePointerMachineConfig generates a machineconfig when a user customizes
// the pointer ignition file manually in an IPI deployment
func generatePointerMachineConfig(config igntypes.Config, role string) (*mcfgv1.MachineConfig, error) {
// Remove the merge section from the pointer config
config.Ignition.Config.Merge = nil

rawExt, err := ignition.ConvertToRawExtension(config)
if err != nil {
return nil, err
}

return &mcfgv1.MachineConfig{
TypeMeta: metav1.TypeMeta{
APIVersion: mcfgv1.SchemeGroupVersion.String(),
Kind: "MachineConfig",
},
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("99-installer-ignition-%s", role),
Labels: map[string]string{
"machineconfiguration.openshift.io/role": role,
},
},
Spec: mcfgv1.MachineConfigSpec{
Config: rawExt,
},
}, nil
}
94 changes: 94 additions & 0 deletions pkg/asset/ignition/machine/workerignitioncheck.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package machine

import (
"os"

"github.com/ghodss/yaml"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"github.com/openshift/installer/pkg/asset"
"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"
)

const (
workerMachineConfigFileName = "manifests/99_openshift-installer-ignition_worker.yaml"
)

// WorkerIgnitionCheck is an asset that checks for any customizations a user might
// have made to the pointer ignition configs before creating the cluster. If customizations
// are made, then the updates are reconciled as a MachineConfig file
type WorkerIgnitionCheck struct {
File *asset.File
}

var _ asset.WritableAsset = (*WorkerIgnitionCheck)(nil)

// Dependencies returns the dependencies for WorkerIgnitionCheck
func (a *WorkerIgnitionCheck) Dependencies() []asset.Asset {
return []asset.Asset{
&installconfig.InstallConfig{},
&tls.RootCA{},
&Worker{},
}
}

// Generate queries for input from the user.
func (a *WorkerIgnitionCheck) Generate(dependencies asset.Parents) error {
installConfig := &installconfig.InstallConfig{}
rootCA := &tls.RootCA{}
worker := &Worker{}
dependencies.Get(installConfig, rootCA, worker)

defaultPointerIgnition := pointerIgnitionConfig(installConfig.Config, rootCA.Cert(), "worker")
savedPointerIgnition := worker.Config

// Create a machineconfig if the ignition has been modified
if savedPointerIgnition != defaultPointerIgnition {
logrus.Infof("Worker pointer ignition was modified. Saving contents to a machineconfig")
mc := &mcfgv1.MachineConfig{}
mc, err := generatePointerMachineConfig(*savedPointerIgnition, "worker")
if err != nil {
return errors.Wrap(err, "failed to generate worker installer machineconfig")
}
configData, err := yaml.Marshal(mc)
if err != nil {
return errors.Wrap(err, "failed to marshal worker installer machineconfig")
}
a.File = &asset.File{
Filename: workerMachineConfigFileName,
Data: configData,
}
}

return nil
}

// Name returns the human-friendly name of the asset.
func (a *WorkerIgnitionCheck) Name() string {
return "Worker Ignition Customization Check"
}

// Files returns the files generated by the asset.
func (a *WorkerIgnitionCheck) Files() []*asset.File {
if a.File != nil {
return []*asset.File{a.File}
}
return []*asset.File{}
}

// Load returns the worker ignitions from disk.
func (a *WorkerIgnitionCheck) Load(f asset.FileFetcher) (found bool, err error) {
file, err := f.FetchByName(workerMachineConfigFileName)
if err != nil {
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
a.File = file

return true, nil
}
50 changes: 50 additions & 0 deletions pkg/asset/ignition/machine/workerignitioncheck_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package machine

import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/tls"
"github.com/openshift/installer/pkg/ipnet"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/aws"
)

// TestWorkerIgnitionCheckGenerate tests generating the worker ignition check asset.
func TestWorkerIgnitionCheckGenerate(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)

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

parents.Add(worker)
workerIgnCheck := &WorkerIgnitionCheck{}
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")
}
2 changes: 2 additions & 0 deletions pkg/asset/targets/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ var (
// Cluster are the cluster targeted assets.
Cluster = []asset.WritableAsset{
&cluster.Metadata{},
&machine.MasterIgnitionCheck{},
&machine.WorkerIgnitionCheck{},
&cluster.TerraformVariables{},
&kubeconfig.AdminClient{},
&password.KubeadminPassword{},
Expand Down

0 comments on commit 484c6b2

Please sign in to comment.