From da21a3a4ef874f386a641c522d2c2b145f71f544 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 22 Jan 2019 19:49:49 -0500 Subject: [PATCH] mco: If the platform is unrecognized, treat the platform as 'none' In order to incrementally roll new platforms out across the project we must be able to provision platforms before all support is added. Components should treat an unrecognized platform as "None", performing the same function as they would in that scenario, and print a warning to the logs. This commit changes the generation so that a ControllerConfig with an empty platform returns an error instead of silently running. Ensure we test config on multiple platforms. --- .../kubelet_config_controller_test.go | 45 ++++++++++--------- pkg/controller/template/render.go | 30 +++++++++---- pkg/controller/template/render_test.go | 22 +++++---- pkg/operator/render.go | 21 ++++++--- 4 files changed, 74 insertions(+), 44 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_controller_test.go b/pkg/controller/kubelet-config/kubelet_config_controller_test.go index d07597c751..63c1056a10 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller_test.go @@ -262,26 +262,31 @@ func (f *fixture) expectUpdateKubeletConfig(config *mcfgv1.KubeletConfig) { } func TestKubeletConfigCreate(t *testing.T) { - f := newFixture(t) - - cc := newControllerConfig("test-cluster") - mcp := newMachineConfigPool("master", map[string]string{"kubeletType": "small-pods"}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role", "master"), "v0") - mcp2 := newMachineConfigPool("worker", map[string]string{"kubeletType": "large-pods"}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role", "worker"), "v0") - kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "kubeletType", "small-pods")) - mcs := newMachineConfig(getManagedKey(mcp, kc1), map[string]string{"node-role": "master"}, "dummy://", []ignv2_2types.File{{}}) - - f.ccLister = append(f.ccLister, cc) - f.mcpLister = append(f.mcpLister, mcp) - f.mcpLister = append(f.mcpLister, mcp2) - f.mckLister = append(f.mckLister, kc1) - f.objects = append(f.objects, kc1) - - f.expectGetMachineConfigAction(mcs) - f.expectCreateMachineConfigAction(mcs) - f.expectPatchKubeletConfig(kc1, []uint8{0x7b, 0x22, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x22, 0x3a, 0x7b, 0x22, 0x66, 0x69, 0x6e, 0x61, 0x6c, 0x69, 0x7a, 0x65, 0x72, 0x73, 0x22, 0x3a, 0x5b, 0x22, 0x39, 0x39, 0x2d, 0x6d, 0x61, 0x73, 0x74, 0x65, 0x72, 0x2d, 0x68, 0x35, 0x35, 0x32, 0x6d, 0x2d, 0x73, 0x6d, 0x61, 0x6c, 0x6c, 0x65, 0x72, 0x2d, 0x6d, 0x61, 0x78, 0x2d, 0x70, 0x6f, 0x64, 0x73, 0x2d, 0x6b, 0x75, 0x62, 0x65, 0x6c, 0x65, 0x74, 0x22, 0x5d, 0x7d, 0x7d}) - f.expectUpdateKubeletConfig(kc1) - - f.run(getKey(kc1, t)) + for _, platform := range []string{"aws", "none", "unrecognized"} { + t.Run(platform, func(t *testing.T) { + f := newFixture(t) + + cc := newControllerConfig("test-cluster") + cc.Spec.Platform = platform + mcp := newMachineConfigPool("master", map[string]string{"kubeletType": "small-pods"}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role", "master"), "v0") + mcp2 := newMachineConfigPool("worker", map[string]string{"kubeletType": "large-pods"}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role", "worker"), "v0") + kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "kubeletType", "small-pods")) + mcs := newMachineConfig(getManagedKey(mcp, kc1), map[string]string{"node-role": "master"}, "dummy://", []ignv2_2types.File{{}}) + + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp) + f.mcpLister = append(f.mcpLister, mcp2) + f.mckLister = append(f.mckLister, kc1) + f.objects = append(f.objects, kc1) + + f.expectGetMachineConfigAction(mcs) + f.expectCreateMachineConfigAction(mcs) + f.expectPatchKubeletConfig(kc1, []uint8{0x7b, 0x22, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x22, 0x3a, 0x7b, 0x22, 0x66, 0x69, 0x6e, 0x61, 0x6c, 0x69, 0x7a, 0x65, 0x72, 0x73, 0x22, 0x3a, 0x5b, 0x22, 0x39, 0x39, 0x2d, 0x6d, 0x61, 0x73, 0x74, 0x65, 0x72, 0x2d, 0x68, 0x35, 0x35, 0x32, 0x6d, 0x2d, 0x73, 0x6d, 0x61, 0x6c, 0x6c, 0x65, 0x72, 0x2d, 0x6d, 0x61, 0x78, 0x2d, 0x70, 0x6f, 0x64, 0x73, 0x2d, 0x6b, 0x75, 0x62, 0x65, 0x6c, 0x65, 0x74, 0x22, 0x5d, 0x7d, 0x7d}) + f.expectUpdateKubeletConfig(kc1) + + f.run(getKey(kc1, t)) + }) + } } func TestKubeletConfigBlacklistedOptions(t *testing.T) { diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go index 05758bbae6..54b6958a4b 100644 --- a/pkg/controller/template/render.go +++ b/pkg/controller/template/render.go @@ -49,14 +49,6 @@ const ( // /files/hostname.tmpl // func generateMachineConfigs(config *RenderConfig, templateDir string) ([]*mcfgv1.MachineConfig, error) { - if config.Platform == "" { - return nil, fmt.Errorf("cannot generateMachineConfigs with an empty Platform") - } - - if config.Platform == "_base" { - return nil, fmt.Errorf("platform _base unsupported") - } - infos, err := ioutil.ReadDir(templateDir) if err != nil { return nil, fmt.Errorf("failed to read dir %q: %v", templateDir, err) @@ -124,9 +116,29 @@ func GenerateMachineConfigsForRole(config *RenderConfig, role string, path strin return cfgs, nil } +func platformFromControllerConfigSpec(ic *mcfgv1.ControllerConfigSpec) (string, error) { + switch ic.Platform { + case "": + return "", fmt.Errorf("cannot generateMachineConfigs with an empty platform field") + case "_base": + return "", fmt.Errorf("platform _base unsupported") + case "aws", "openstack", "libvirt", "none": + // TODO: these constants are wrong, they should match what is reported by the infrastructure provider + return ic.Platform, nil + default: + glog.Warningf("Warning: the controller config referenced a platform other than 'aws', 'libvirt', 'openstack', or 'none': %s", ic.Platform) + return "none", nil + } +} + func generateMachineConfigForName(config *RenderConfig, role, name, path string) (*mcfgv1.MachineConfig, error) { + platform, err := platformFromControllerConfigSpec(config.ControllerConfigSpec) + if err != nil { + return nil, err + } + platformDirs := []string{} - for _, dir := range []string{"_base", config.Platform} { + for _, dir := range []string{"_base", platform} { platformPath := filepath.Join(path, dir) exists, err := existsDir(platformPath) if err != nil { diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go index 6fa9e5bcea..ad52174c0a 100644 --- a/pkg/controller/template/render_test.go +++ b/pkg/controller/template/render_test.go @@ -274,13 +274,17 @@ func TestInvalidPlatform(t *testing.T) { } } + // we must treat unrecognized constants as "none" controllerConfig.Spec.Platform = "_bad_" _, err = generateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`}, templateDir) - expectErr(err, "failed to create MachineConfig for role master: platform _bad_ unsupported") + if err != nil { + t.Errorf("expect nil error, got: %v", err) + } + // explicitly blocked controllerConfig.Spec.Platform = "_base" _, err = generateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`}, templateDir) - expectErr(err, "platform _base unsupported") + expectErr(err, "failed to create MachineConfig for role master: platform _base unsupported") } func TestGenerateMachineConfigs(t *testing.T) { @@ -335,12 +339,14 @@ func TestGenerateMachineConfigsSSH(t *testing.T) { for _, cfg := range cfgs { name := cfg.ObjectMeta.Name switch name { - case "00-master-ssh": { - masterSsh = cfg - } - case "00-worker-ssh": { - workerSsh = cfg - } + case "00-master-ssh": + { + masterSsh = cfg + } + case "00-worker-ssh": + { + workerSsh = cfg + } } } if masterSsh == nil { diff --git a/pkg/operator/render.go b/pkg/operator/render.go index d22f2dbc8d..387bb3d6fc 100644 --- a/pkg/operator/render.go +++ b/pkg/operator/render.go @@ -9,6 +9,7 @@ import ( "github.com/Masterminds/sprig" "github.com/apparentlymart/go-cidr/cidr" "github.com/ghodss/yaml" + "github.com/golang/glog" installertypes "github.com/openshift/installer/pkg/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/pkg/operator/assets" @@ -62,30 +63,36 @@ func discoverMCOConfig(f installConfigGetter) (*mcfgv1.MCOConfig, error) { return nil, err } + platform, err := platformFromInstallConfig(ic) + if err != nil { + glog.Warningf("Warning: %v, using %s", err, platform) + } + return &mcfgv1.MCOConfig{ Spec: mcfgv1.MCOConfigSpec{ ClusterDNSIP: dnsIP, CloudProviderConfig: "", ClusterName: ic.ObjectMeta.Name, - Platform: platformFromInstallConfig(ic), + Platform: platform, BaseDomain: ic.BaseDomain, SSHKey: ic.SSHKey, }, }, nil } -func platformFromInstallConfig(ic installertypes.InstallConfig) string { +func platformFromInstallConfig(ic installertypes.InstallConfig) (string, error) { + // TODO: these constants are wrong, they should match what is reported by the infrastructure provider switch { case ic.Platform.AWS != nil: - return "aws" + return "aws", nil case ic.Platform.OpenStack != nil: - return "openstack" + return "openstack", nil case ic.Libvirt != nil: - return "libvirt" + return "libvirt", nil case ic.None != nil: - return "none" + return "none", nil default: - panic("invalid platform") + return "none", fmt.Errorf("the install config referenced a platform other than 'aws', 'libvirt', 'openstack', or 'none'") } }