From 5c338d3956c6b8b0e9ddfb27f5614c2a3f109566 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 28 May 2019 19:10:27 +0000 Subject: [PATCH 1/3] server: Add a helper to extract Ignition from MachineConfig Prep for futher work. --- pkg/server/bootstrap_server.go | 3 ++- pkg/server/cluster_server.go | 2 +- pkg/server/server.go | 6 ++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/server/bootstrap_server.go b/pkg/server/bootstrap_server.go index 34a85c9c4a..e00e0b2e5f 100644 --- a/pkg/server/bootstrap_server.go +++ b/pkg/server/bootstrap_server.go @@ -101,7 +101,8 @@ func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*igntypes.Config, error) return nil, err } } - return &mc.Spec.Config, nil + + return machineConfigToIgnition(mc), nil } func kubeconfigFromFile(path string) ([]byte, []byte, error) { diff --git a/pkg/server/cluster_server.go b/pkg/server/cluster_server.go index e1e229f23f..0c27f72c05 100644 --- a/pkg/server/cluster_server.go +++ b/pkg/server/cluster_server.go @@ -77,7 +77,7 @@ func (cs *clusterServer) GetConfig(cr poolRequest) (*igntypes.Config, error) { return nil, err } } - return &mc.Spec.Config, nil + return machineConfigToIgnition(mc), nil } // getClientConfig returns a Kubernetes client Config. diff --git a/pkg/server/server.go b/pkg/server/server.go index e8d43fad78..b8e61d264d 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -6,6 +6,7 @@ import ( "net/url" igntypes "github.com/coreos/ignition/config/v2_2/types" + mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/vincent-petithory/dataurl" ) @@ -45,6 +46,11 @@ func getAppenders(currMachineConfig string, f kubeconfigFunc, osimageurl string) return appenders } +// machineConfigToIgnition converts a MachineConfig object into Ignition. +func machineConfigToIgnition(mccfg *mcfgv1.MachineConfig) *igntypes.Config { + return &mccfg.Spec.Config +} + // Golang :cry: func boolToPtr(b bool) *bool { return &b From c3bfc8b4291c39501a390b08a78e6fce1d8c8277 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jun 2019 19:38:18 +0000 Subject: [PATCH 2/3] server: Add spec/configuration to test data Missed this as part of https://github.com/openshift/machine-config-operator/pull/773 (Doesn't matter yet, just prep for any future changes) --- pkg/server/testdata/machine-pools/test-pool.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/server/testdata/machine-pools/test-pool.yaml b/pkg/server/testdata/machine-pools/test-pool.yaml index 2e086ddeb9..098fc55140 100644 --- a/pkg/server/testdata/machine-pools/test-pool.yaml +++ b/pkg/server/testdata/machine-pools/test-pool.yaml @@ -4,6 +4,8 @@ metadata: creationTimestamp: null name: test-pool spec: + configuration: + name: test-config machineConfigSelector: matchLabels: machineconfiguration.openshift.io/role: test From ceeb1260215c95c955a2dd3c16b4bc2fe2e6323d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 28 May 2019 20:14:38 +0000 Subject: [PATCH 3/3] server: Add /etc/ignition-machine-config-encapsulated.json This effectively "inverts" a `MachineConfig` object so that when it boots, Ignition processes the subset of MachineConfig that is Ignition, and then we have the other stuff (`osImageURL`, `kernelArguments` and in the future more) as a file on disk that the MCD can read. The way to think about this is we're aiming to have RHCOS boot into `MachineConfig`, not just Ignition. --- pkg/daemon/constants/constants.go | 6 ++ pkg/server/server.go | 8 +++ pkg/server/server_test.go | 65 +++++++++++++++---- .../testdata/machine-configs/test-config.yaml | 4 ++ 4 files changed, 71 insertions(+), 12 deletions(-) diff --git a/pkg/daemon/constants/constants.go b/pkg/daemon/constants/constants.go index 3c81958027..f25fd4f3c2 100644 --- a/pkg/daemon/constants/constants.go +++ b/pkg/daemon/constants/constants.go @@ -34,4 +34,10 @@ const ( // EtcPivotFile is used by the `pivot` command // For more information, see https://github.com/openshift/pivot/pull/25/commits/c77788a35d7ee4058d1410e89e6c7937bca89f6c#diff-04c6e90faac2675aa89e2176d2eec7d8R44 EtcPivotFile = "/etc/pivot/image-pullspec" + + // MachineConfigEncapsulatedPath contains all of the data from a MachineConfig object + // except the Spec/Config object; this supports inverting+encapsulating a MachineConfig + // object so that Ignition can process it on first boot, and then the MCD can act on + // non-Ignition fields such as the osImageURL and kernelArguments. + MachineConfigEncapsulatedPath = "/etc/ignition-machine-config-encapsulated.json" ) diff --git a/pkg/server/server.go b/pkg/server/server.go index b8e61d264d..de529b41bb 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -7,6 +7,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" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/vincent-petithory/dataurl" ) @@ -48,6 +49,13 @@ func getAppenders(currMachineConfig string, f kubeconfigFunc, osimageurl string) // machineConfigToIgnition converts a MachineConfig object into Ignition. func machineConfigToIgnition(mccfg *mcfgv1.MachineConfig) *igntypes.Config { + tmpcfg := mccfg.DeepCopy() + tmpcfg.Spec.Config = ctrlcommon.NewIgnConfig() + serialized, err := json.Marshal(tmpcfg) + if err != nil { + panic(err.Error()) + } + appendFileToIgnition(&mccfg.Spec.Config, daemonconsts.MachineConfigEncapsulatedPath, string(serialized)) return &mccfg.Spec.Config } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 217831c5fe..22ae2612e8 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -6,12 +6,12 @@ import ( "io/ioutil" "path" "path/filepath" - "reflect" "strings" "testing" igntypes "github.com/coreos/ignition/config/v2_2/types" yaml "github.com/ghodss/yaml" + "github.com/stretchr/testify/assert" v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" @@ -49,6 +49,25 @@ func TestStringEncode(t *testing.T) { } } +func TestMachineConfigToIgnition(t *testing.T) { + mcPath := filepath.Join(testDir, "machine-configs", testConfig+".yaml") + mcData, err := ioutil.ReadFile(mcPath) + assert.Nil(t, err) + mc := new(v1.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") + + 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) +} + // TestBootstrapServer tests the behavior of the machine config server // when it's running in bootstrap mode. // The test does the following: @@ -108,8 +127,8 @@ func TestBootstrapServer(t *testing.T) { } // assert on the output. - validateIgnitionFiles(t, res.Storage.Files, mc.Spec.Config.Storage.Files) - validateIgnitionSystemd(t, res.Systemd.Units, mc.Spec.Config.Systemd.Units) + validateIgnitionFiles(t, mc.Spec.Config.Storage.Files, res.Storage.Files) + validateIgnitionSystemd(t, mc.Spec.Config.Systemd.Units, res.Systemd.Units) } // TestClusterServer tests the behavior of the machine config server @@ -182,8 +201,26 @@ func TestClusterServer(t *testing.T) { t.Fatalf("expected err to be nil, received: %v", err) } - validateIgnitionFiles(t, res.Storage.Files, mc.Spec.Config.Storage.Files) - validateIgnitionSystemd(t, res.Systemd.Units, mc.Spec.Config.Systemd.Units) + validateIgnitionFiles(t, mc.Spec.Config.Storage.Files, res.Storage.Files) + validateIgnitionSystemd(t, mc.Spec.Config.Systemd.Units, res.Systemd.Units) + + foundEncapsulated := false + for _, f := range res.Storage.Files { + if f.Path != daemonconsts.MachineConfigEncapsulatedPath { + continue + } + foundEncapsulated = true + encapMc := new(v1.MachineConfig) + contents, err := getDecodedContent(f.Contents.Source) + assert.Nil(t, err) + err = yaml.Unmarshal([]byte(contents), encapMc) + assert.Nil(t, err) + assert.Equal(t, encapMc.Spec.KernelArguments, mc.Spec.KernelArguments) + assert.Equal(t, encapMc.Spec.OSImageURL, mc.Spec.OSImageURL) + } + if !foundEncapsulated { + t.Errorf("missing %s", daemonconsts.MachineConfigEncapsulatedPath) + } } func getKubeConfigContent(t *testing.T) ([]byte, []byte, error) { @@ -194,15 +231,20 @@ func validateIgnitionFiles(t *testing.T, exp, got []igntypes.File) { expMap := createFileMap(exp) gotMap := createFileMap(got) + encapsulatedKey := "root" + daemonconsts.MachineConfigEncapsulatedPath for k, v := range expMap { + // This special value is injected + if k == encapsulatedKey { + continue + } f, ok := gotMap[k] if !ok { t.Errorf("could not find file: %s", k) + continue } - if !reflect.DeepEqual(v, f) { - t.Errorf("file validation failed for: %s, exp: %v, got: %v", k, v, f) - } + assert.Equal(t, v, f) } + } func validateIgnitionSystemd(t *testing.T, exp, got []igntypes.Unit) { @@ -212,11 +254,10 @@ func validateIgnitionSystemd(t *testing.T, exp, got []igntypes.Unit) { for k, v := range expMap { f, ok := gotMap[k] if !ok { - t.Errorf("could not find file: %s", k) - } - if !reflect.DeepEqual(v, f) { - t.Errorf("file validation failed for: %s, exp: %v, got: %v", k, v, f) + t.Errorf("could not find unit: %s", k) + continue } + assert.Equal(t, v, f) } } diff --git a/pkg/server/testdata/machine-configs/test-config.yaml b/pkg/server/testdata/machine-configs/test-config.yaml index 232e193fdc..fb218d68a6 100644 --- a/pkg/server/testdata/machine-configs/test-config.yaml +++ b/pkg/server/testdata/machine-configs/test-config.yaml @@ -3,6 +3,10 @@ kind: MachineConfig metadata: name: test-config spec: + osImageURL: registry.example.com/oscontainer@sha256:b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c + kernelArguments: + - foo=bar + - nosmt config: ignition: storage: