From 7b256fc8f6b8b490feab0c4a85e628f02f7a7b26 Mon Sep 17 00:00:00 2001
From: Abhinav Dahiya <abhinav.dahiya@redhat.com>
Date: Fri, 26 Apr 2019 14:01:41 -0700
Subject: [PATCH] controller/template: put the cloudconf for kubelet behind
 platform gate

Previously, the assumption was that all platforms that had CloudProviderConfig set can have kubelet configured to
use the config file. But this created a regression for user-provided-infrastructure VSphere case, as everybody like kube-controller-manager etc. except kubelet can use the cloud conf
file from API for vSphere
---
 pkg/controller/template/render.go             | 16 ++++--
 pkg/controller/template/render_test.go        | 51 +++++++++++++++++++
 .../vsphere/units/kubelet.service             |  2 +-
 .../vsphere/units/kubelet.service             |  2 +-
 4 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go
index 6377fafd9f..1740d12816 100644
--- a/pkg/controller/template/render.go
+++ b/pkg/controller/template/render.go
@@ -365,14 +365,22 @@ func cloudProvider(cfg RenderConfig) (interface{}, error) {
 // Process the {{cloudConfigFlag .}}
 // If the CloudProviderConfig field is set and not empty, this
 // returns the cloud conf flag for kubelet [1] pointing the kubelet to use
-// /etc/kubernetes/cloud.conf for configuring the cloud provider.
+// /etc/kubernetes/cloud.conf for configuring the cloud provider for select platforms.
+// By default even if CloudProviderConfig fields is set, the kubelet will be configured to used for
+// select platforms only.
 //
 // [1]: https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/#options
 func cloudConfigFlag(cfg RenderConfig) interface{} {
-	if len(cfg.CloudProviderConfig) > 0 {
-		return "--cloud-config=/etc/kubernetes/cloud.conf"
+	if len(cfg.CloudProviderConfig) == 0 {
+		return ""
+	}
+	flag := "--cloud-config=/etc/kubernetes/cloud.conf"
+	switch cfg.Platform {
+	case platformAzure, platformOpenstack:
+		return flag
+	default:
+		return ""
 	}
-	return ""
 }
 
 // existsDir returns true if path exists and is a directory, false if the path
diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go
index 3f261679ed..936613cdd9 100644
--- a/pkg/controller/template/render_test.go
+++ b/pkg/controller/template/render_test.go
@@ -65,6 +65,57 @@ func TestCloudProvider(t *testing.T) {
 	}
 }
 
+func TestCloudConfigFlag(t *testing.T) {
+	dummyTemplate := []byte(`{{cloudConfigFlag .}}`)
+
+	cases := []struct {
+		platform string
+		content  string
+		res      string
+	}{{
+		platform: "aws",
+		content:  "",
+		res:      "",
+	}, {
+		platform: "azure",
+		content:  "",
+		res:      "",
+	}, {
+		platform: "aws",
+		content: `
+[dummy-config]
+    option = a
+`,
+		res: "",
+	}, {
+		platform: "azure",
+		content: `
+[dummy-config]
+    option = a
+`,
+		res: "--cloud-config=/etc/kubernetes/cloud.conf",
+	}}
+	for idx, c := range cases {
+		name := fmt.Sprintf("case #%d", idx)
+		t.Run(name, func(t *testing.T) {
+			config := &mcfgv1.ControllerConfig{
+				Spec: mcfgv1.ControllerConfigSpec{
+					Platform:            c.platform,
+					CloudProviderConfig: c.content,
+				},
+			}
+			got, err := renderTemplate(RenderConfig{&config.Spec, `{"dummy":"dummy"}`}, name, dummyTemplate)
+			if err != nil {
+				t.Fatalf("expected nil error %v", err)
+			}
+
+			if string(got) != c.res {
+				t.Fatalf("mismatch got: %s want: %s", got, c.res)
+			}
+		})
+	}
+}
+
 func TestEtcdPeerCertDNSNames(t *testing.T) {
 	dummyTemplate := []byte(`{{etcdPeerCertDNSNames .}}`)
 
diff --git a/pkg/controller/template/test_data/templates/master/01-master-kubelet/vsphere/units/kubelet.service b/pkg/controller/template/test_data/templates/master/01-master-kubelet/vsphere/units/kubelet.service
index 08ff90f0f6..4000603149 100644
--- a/pkg/controller/template/test_data/templates/master/01-master-kubelet/vsphere/units/kubelet.service
+++ b/pkg/controller/template/test_data/templates/master/01-master-kubelet/vsphere/units/kubelet.service
@@ -25,7 +25,7 @@ contents: |
         --client-ca-file=/etc/kubernetes/ca.crt \
         --cloud-provider=vsphere \
         --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
-        --cloud-config=/etc/kubernetes/cloud.conf \
+         \
         --anonymous-auth=false \
         --register-with-taints=node-role.kubernetes.io/master=:NoSchedule \
 
diff --git a/pkg/controller/template/test_data/templates/worker/01-worker-kubelet/vsphere/units/kubelet.service b/pkg/controller/template/test_data/templates/worker/01-worker-kubelet/vsphere/units/kubelet.service
index 0a14df8ec2..a7d8949644 100644
--- a/pkg/controller/template/test_data/templates/worker/01-worker-kubelet/vsphere/units/kubelet.service
+++ b/pkg/controller/template/test_data/templates/worker/01-worker-kubelet/vsphere/units/kubelet.service
@@ -24,7 +24,7 @@ contents: |
         --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
         --client-ca-file=/etc/kubernetes/ca.crt \
         --cloud-provider=vsphere \
-        --cloud-config=/etc/kubernetes/cloud.conf \
+         \
         --anonymous-auth=false \
 
   Restart=always