mco: If the platform is unrecognized, treat the platform as 'none'

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) {

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 {

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 {

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