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