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.

Author: smarterclayton

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