render: add --cloud-provider #105

kalbasit · 2016-08-19T17:19:01Z

No description provided.

aaronlevy · 2016-08-20T01:01:57Z

@kalbasit I opened #106 to track some thoughts about how we can make configuration options like these potentially not require internal code changes. Let me know if you have any thoughts

kalbasit · 2016-08-21T04:31:31Z

@aaronlevy I like the idea of #106 I'll think more about it and give my feedback there. --cloud-provider is going to be a very common flag, shouldn't be part of the default templates?

kalbasit · 2016-08-22T00:57:22Z

pkg/asset/templates/kube-apiserver.yaml

@@ -41,6 +41,9 @@ spec:
        - --tls-private-key-file=/etc/kubernetes/secrets/apiserver.key
        - --service-account-key-file=/etc/kubernetes/secrets/service-account.pub
        - --client-ca-file=/etc/kubernetes/secrets/ca.crt
+        {{ with .CloudProvider -}}


@aaronlevy the -}} is a Go 1.6+ feature, see golang/go#9969. Is that OK?

kalbasit · 2016-11-03T17:18:00Z

@aaronlevy bumping this to see if we can get it merged while waiting for a better solution than having to create a fork.

aaronlevy · 2016-11-18T17:38:32Z

Hi @kalbasit

This is being partially implemented in: #186 -- but not yet exposed as a cli flag - as we might still need a bit more work for this to be plumbed through as a cli option. See my comments here: #186 (comment)

kalbasit · 2016-11-18T17:41:51Z

@aaronlevy why re-implement the same thing in another PR?

aaronlevy · 2016-11-18T17:46:58Z

If you want to make the changes here, happy to merge this PR (yours was first). I'll make inline comments for changes that would be needed.

Then as far as exposing the flag -- we can move forward with it, but I do have a concern that the temporary control plane will not reflect the same options (and I'm unsure if this is a safe assumption at this time). I also don't think adding another flag to start is a safe alternative, because then there is just an implied assumption that same flags are used between commands (for it to work), but no actual enforcement of this.

kalbasit · 2016-11-18T17:50:10Z

@aaronlevy I've been running a production cluster with this on since August and I had no problems so far.

I was thinking the other day about the flag and about #106, what if instead of exposing cloud-provider or any of the other flags, maybe we could expose --extra-api-flags, --extra-controller-flags, etc.. Thoughts?

aaronlevy · 2016-11-18T17:48:13Z

pkg/asset/internal/templates.go

@@ -51,6 +51,9 @@ spec:
        - --kubeconfig=/etc/kubernetes/kubeconfig
        - --require-kubeconfig
        - --lock-file=/var/run/lock/kubelet.lock
+        {{ with .CloudProvider -}}
+        - --cloud-provider={{ . }}
+        {{ end -}}


We should drop these changes from the self-hosted kubelet manifest. Otherwise the self-hosted kubelet could behave differently than the on-host kubelet (as documented). The kubelet should use "auto-detect" for both -- so no change should be necessary.

aaronlevy · 2016-11-18T17:49:12Z

pkg/asset/internal/templates.go

@@ -194,20 +200,29 @@ spec:
        - --root-ca-file=/etc/kubernetes/secrets/ca.crt
        - --service-account-private-key-file=/etc/kubernetes/secrets/service-account.key
        - --leader-elect=true
+        {{ with .CloudProvider -}}
+        - --cloud-provider={{ . }}
+        {{ end -}}


nit: Drop the with/end blocks.
The default cloud-provider is an empty string anyway. --cloud-provider={{ .CloudProvider }}

aaronlevy · 2016-11-18T17:50:06Z

pkg/asset/internal/templates.go

@@ -194,20 +200,29 @@ spec:
        - --root-ca-file=/etc/kubernetes/secrets/ca.crt
        - --service-account-private-key-file=/etc/kubernetes/secrets/service-account.key
        - --leader-elect=true
+        {{ with .CloudProvider -}}
+        - --cloud-provider={{ . }}
+        {{ end -}}


Should also add - --configure-cloud-routes=false which is safe to always be set (we assume flannel will handle managing routes).

aaronlevy · 2016-11-18T17:50:34Z

pkg/asset/internal/templates.go

        volumeMounts:
        - name: secrets
          mountPath: /etc/kubernetes/secrets
          readOnly: true
        - name: ssl-host
          mountPath: /etc/ssl/certs
          readOnly: true
+        - name: resolv-conf
+          mountPath: /etc/resolv.conf
+          readOnly: true


Lets use dnsPolicy: Default instead of the explicit host mount.

aaronlevy · 2016-11-18T17:51:23Z

pkg/asset/k8s.go

 		mustCreateAssetFromTemplate(AssetPathAPIServer, internal.APIServerTemplate, conf),
+		mustCreateAssetFromTemplate(AssetPathKubelet, internal.KubeletTemplate, conf),


kubelet shouldn't be templated if we're not going to pass in the cloud-provider flag to it.

kalbasit · 2016-11-18T18:01:32Z

@aaronlevy PTAL

aaronlevy · 2016-11-18T18:02:37Z

pkg/asset/k8s.go

@@ -38,6 +38,7 @@ func newStaticAssets(selfHostKubelet bool) Assets {

 func newDynamicAssets(conf Config) Assets {
 	return Assets{
+		mustCreateAssetFromTemplate(AssetPathControllerManager, internal.ControllerManagerTemplate, conf),


The controller-manager entry should also be removed from newStaticAssets

oops, must have bee a merge mistake at some point.

kalbasit · 2016-11-18T18:06:40Z

@aaronlevy PTAL

aaronlevy · 2016-11-18T18:07:46Z

lgtm. Thanks @kalbasit ! Can you squash your commits and I'll merge.

kalbasit · 2016-11-18T18:10:26Z

@aaronlevy thanks for the quick review, very excited to have this merged (hard to maintain a fork :)). I squashed the commits.

@aaronlevy when will this make be released?

aaronlevy · 2016-11-18T18:38:20Z

@kalbasit I'll try and get a new release out today.

kalbasit · 2016-11-18T19:18:17Z

@aaronlevy thank you!

aaronlevy · 2016-11-22T01:43:52Z

@kalbasit sorry for the delay -- trying to work out some networking issues I'm seeing on vagrant/kube-proxy so want to fully understand issue before cutting a release

kalbasit · 2016-11-22T02:20:13Z

@aaronlevy thank you for letting me know. I need the release at the beginning of December so please take your time and ping me once it has been released.

kalbasit force-pushed the allow-cloud-provider branch from 47b8c96 to b0d8215 Compare August 22, 2016 00:55

kalbasit reviewed Aug 22, 2016
View reviewed changes

aaronlevy mentioned this pull request Aug 22, 2016

Provide a generic mechanism for flag customizations #106

Closed

kalbasit force-pushed the allow-cloud-provider branch 2 times, most recently from 64056fc to 692c119 Compare September 28, 2016 23:18

kalbasit force-pushed the allow-cloud-provider branch 2 times, most recently from b7f16ee to 1709a6f Compare November 3, 2016 16:51

aaronlevy suggested changes Nov 18, 2016

View reviewed changes

kalbasit force-pushed the allow-cloud-provider branch from 1709a6f to 9c815cc Compare November 18, 2016 18:01

aaronlevy reviewed Nov 18, 2016

View reviewed changes

render: add --cloud-provider

4969f4b

kalbasit force-pushed the allow-cloud-provider branch from 3ada290 to 4969f4b Compare November 18, 2016 18:08

aaronlevy approved these changes Nov 18, 2016

View reviewed changes

aaronlevy merged commit cb22058 into kubernetes-retired:master Nov 18, 2016

aaronlevy mentioned this pull request Nov 18, 2016

pkg/asset: Add CloudProvider field to Config #186

Closed

kalbasit deleted the allow-cloud-provider branch November 18, 2016 19:18

Quentin-M mentioned this pull request Dec 14, 2016

Add missing --cloud-provider parameter to kubelet #229

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

render: add --cloud-provider #105

render: add --cloud-provider #105

kalbasit commented Aug 19, 2016

aaronlevy commented Aug 20, 2016

kalbasit commented Aug 21, 2016

kalbasit Aug 22, 2016

kalbasit commented Nov 3, 2016

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016 •

edited

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy Nov 18, 2016 •

edited

aaronlevy Nov 18, 2016

aaronlevy Nov 18, 2016

aaronlevy Nov 18, 2016

aaronlevy Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy Nov 18, 2016

kalbasit Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy commented Nov 22, 2016

kalbasit commented Nov 22, 2016

		mustCreateAssetFromTemplate(AssetPathAPIServer, internal.APIServerTemplate, conf),
		mustCreateAssetFromTemplate(AssetPathKubelet, internal.KubeletTemplate, conf),

render: add --cloud-provider #105

render: add --cloud-provider #105

Conversation

kalbasit commented Aug 19, 2016

aaronlevy commented Aug 20, 2016

kalbasit commented Aug 21, 2016

kalbasit Aug 22, 2016

Choose a reason for hiding this comment

kalbasit commented Nov 3, 2016

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016 • edited

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy Nov 18, 2016 • edited

Choose a reason for hiding this comment

aaronlevy Nov 18, 2016

Choose a reason for hiding this comment

aaronlevy Nov 18, 2016

Choose a reason for hiding this comment

aaronlevy Nov 18, 2016

Choose a reason for hiding this comment

aaronlevy Nov 18, 2016

Choose a reason for hiding this comment

kalbasit commented Nov 18, 2016

aaronlevy Nov 18, 2016

Choose a reason for hiding this comment

kalbasit Nov 18, 2016

Choose a reason for hiding this comment

kalbasit commented Nov 18, 2016

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy commented Nov 18, 2016

kalbasit commented Nov 18, 2016

aaronlevy commented Nov 22, 2016

kalbasit commented Nov 22, 2016

kalbasit commented Nov 18, 2016 •

edited

aaronlevy Nov 18, 2016 •

edited