Add Rescheduler addon #1780

CecileRobertMichon · 2017-11-16T00:16:23Z

What this PR does / why we need it: Guaranteed scheduling of critical add-ons requires a Rescheduler pod to be running, this PR adds that Rescheduler.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #944

Release note:

This reverts commit ab8d33d.

…gine

This reverts commit ab8d33d.

…cs-engine into rescheduler

CecileRobertMichon · 2017-11-16T22:44:04Z

pkg/acsengine/defaults.go

+	// DefaultReschedulerAddonsConfig is the default rescheduler Kubernetes addon Config
+	DefaultReschedulerAddonsConfig = api.KubernetesAddon{
+		Name:    DefaultReschedulerAddonName,
+		Enabled: pointerToBool(true),


Should this be enabled by default?

Let's ship this as false to be cautious, and then figure out how we want to bake the feature before turning default to true.

Also, you could pay back some of my tech debt by changing these boolean literals inside pointerToBool() to api.DefaultTillerAddonEnabled and api.DefaultDashboardAddonEnabled :)

CecileRobertMichon · 2017-11-16T22:44:36Z

pkg/acsengine/defaults.go

+				CPURequests:    "300m",
+				MemoryRequests: "150Mi",
+				CPULimits:      "300m",
+				MemoryLimits:   "150Mi",


^^ need to discuss what default limits to use for rescheduler

We should copy what's here:

https://github.com/kubernetes/kubernetes/blob/master/cluster/saltbase/salt/rescheduler/rescheduler.manifest

(limits can copy requests)

CecileRobertMichon · 2017-11-16T22:45:16Z

parts/kubernetesmasteraddons-kube-rescheduler-deployment.yaml

+        name: rescheduler
+        resources:
+          requests:
+            cpu: 10m


// todo change this to param

CecileRobertMichon · 2017-11-16T22:46:40Z

pkg/acsengine/engine.go

@@ -570,7 +572,20 @@ func getParameters(cs *api.ContainerService, isClassicMode bool, generatorCode s
 			if dashboardAddon.Containers[c].Image != "" {
 				addValue(parametersMap, "kubernetesDashboardSpec", dashboardAddon.Containers[c].Image)
 			} else {
-				addValue(parametersMap, "kubernetesDashboardSpec", cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase+KubeConfigs[k8sVersion]["dashboard"])
+				addValue(parametersMap, "kubernetesDashboardSpec", cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase+KubeConfigs[k8sVersion][DefaultDashboardAddonName])


@jackfrancis is there a reason why this wasn't using the name var? is it okay that I changed it? (1/3)

👍 and thanks!

CecileRobertMichon · 2017-11-16T22:46:54Z

pkg/acsengine/engine.go

@@ -1191,7 +1211,7 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
 							val = dashboardAddon.Containers[dC].Image
 						}
 					} else {
-						val = cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase + KubeConfigs[k8sVersion]["dashboard"]
+						val = cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase + KubeConfigs[k8sVersion][DefaultDashboardAddonName]


@jackfrancis same here (2/3)

👍 and thanks ditto

CecileRobertMichon · 2017-11-16T22:47:29Z

pkg/acsengine/k8s_versions.go

-		"gchighthreshold":      strconv.Itoa(DefaultKubernetesGCHighThreshold),
-		"gclowthreshold":       strconv.Itoa(DefaultKubernetesGCLowThreshold),
+		"hyperkube":                 "hyperkube-amd64:v1.8.2",
+		DefaultDashboardAddonName:   "kubernetes-dashboard-amd64:v1.7.1",


@jackfrancis and here (3/3)

CecileRobertMichon · 2017-11-16T22:48:44Z

pkg/api/const.go

@@ -86,4 +86,6 @@ const (
 	DefaultTillerAddonEnabled = true
 	// DefaultDashboardAddonEnabled determines the acs-engine provided default for enabling kubernetes-dashboard addon
 	DefaultDashboardAddonEnabled = true
+	// DefaultReschedulerAddonEnabled determines the acs-engine provided default for enabling kubernetes-rescheduler addon
+	DefaultReschedulerAddonEnabled = true


True or false here?

This should be false to match.

CecileRobertMichon · 2017-11-16T22:49:41Z

pkg/acsengine/const.go

@@ -85,6 +85,10 @@ const (
 	DefaultOrchestratorName = "k8s"
 	// DefaultEtcdDiskSize specifies the default size for Kubernetes master etcd disk volumes in GB
 	DefaultEtcdDiskSize = "128"
+	// DefaultReschedulerImage defines the rescheduler deployment version on Kubernetes Clusters
+	DefaultReschedulerImage = "rescheduler:v0.3.1"


Is the right version to use as default?

let's ship w/ latest

0.3.1 is the latest tag here

jackfrancis · 2017-11-17T04:29:02Z

parts/kubernetesmastercustomdata.yml

+    sed -i "s|<kubernetesReschedulerMemoryRequests>|{{WrapAsVariable "kubernetesReschedulerMemoryRequests"}}|g" "/etc/kubernetes/addons/kube-rescheduler-deployment.yaml"
+    sed -i "s|<kubernetesReschedulerCPULimit>|{{WrapAsVariable "kubernetesReschedulerCPULimit"}}|g" "/etc/kubernetes/addons/kube-rescheduler-deployment.yaml"
+    sed -i "s|<kubernetesReschedulerMemoryLimit>|{{WrapAsVariable "kubernetesReschedulerMemoryLimit"}}|g" "/etc/kubernetes/addons/kube-rescheduler-deployment.yaml"
+    {{end}}


for clarity let's unindent this {{end}} to match the `{{if``

jackfrancis · 2017-11-17T04:32:23Z

pkg/acsengine/defaults.go

@@ -334,6 +355,10 @@ func setOrchestratorDefaults(cs *api.ContainerService) {
 		if a.OrchestratorProfile.KubernetesConfig.Addons[d].IsEnabled(api.DefaultDashboardAddonEnabled) {
 			a.OrchestratorProfile.KubernetesConfig.Addons[d] = assignDefaultAddonVals(a.OrchestratorProfile.KubernetesConfig.Addons[d], DefaultDashboardAddonsConfig)
 		}
+		r := getAddonsIndexByName(a.OrchestratorProfile.KubernetesConfig.Addons, DefaultReschedulerAddonName)
+		if a.OrchestratorProfile.KubernetesConfig.Addons[r].IsEnabled(api.DefaultReschedulerAddonEnabled) {
+			a.OrchestratorProfile.KubernetesConfig.Addons[r] = assignDefaultAddonVals(a.OrchestratorProfile.KubernetesConfig.Addons[r], DefaultDashboardAddonsConfig)


The 2nd arg to assignDefaultAddonVals() here should be DefaultReschedulerAddonsConfig. (I know, copy/paste + tired eye sockets...)

Yes thanks for catching that

jackfrancis · 2017-11-17T04:33:47Z

pkg/acsengine/engine.go

@@ -547,6 +548,7 @@ func getParameters(cs *api.ContainerService, isClassicMode bool, generatorCode s
 		addValue(parametersMap, "kubernetesDNSMasqSpec", cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase+KubeConfigs[k8sVersion]["dnsmasq"])
 		addValue(parametersMap, "kubernetesExecHealthzSpec", cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase+KubeConfigs[k8sVersion]["exechealthz"])
 		addValue(parametersMap, "kubernetesHeapsterSpec", cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase+KubeConfigs[k8sVersion]["heapster"])
+		addValue(parametersMap, "kubernetesReschedulerSpec", cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase+KubeConfigs[k8sVersion]["rescheduler"])


use DefaultReschedulerAddonName as map key here

jackfrancis · 2017-11-17T04:34:05Z

pkg/api/types.go

+func (k *KubernetesConfig) IsReschedulerEnabled() bool {
+	var reschedulerAddon KubernetesAddon
+	for i := range k.Addons {
+		if k.Addons[i].Name == "rescheduler" {


use DefaultReschedulerAddonName here

jackfrancis · 2017-11-17T04:34:26Z

pkg/api/types_test.go

+	if e != DefaultReschedulerAddonEnabled {
+		t.Fatalf("KubernetesConfig.IsReschedulerEnabled() should return %t when no rescheduler addon has been specified, instead returned %t", DefaultReschedulerAddonEnabled, e)
+	}
+	c.Addons = append(c.Addons, getMockAddon("rescheduler"))


use DefaultReschedulerAddonName here

jackfrancis · 2017-11-17T04:34:34Z

pkg/api/types_test.go

+	c = KubernetesConfig{
+		Addons: []KubernetesAddon{
+			{
+				Name:    "rescheduler",


use DefaultReschedulerAddonName here

jackfrancis · 2017-11-17T19:01:12Z

parts/kubernetesmasteraddons-kube-rescheduler-deployment.yaml

+        resources:
+          requests:
+            cpu: <kubernetesReschedulerCPURequests>
+            memory: <kubernetesReschedulerCPULimit>


This needs to be <kubernetesReschedulerMemoryRequests>

oops yes thank you!

…n docs

CecileRobertMichon · 2017-11-17T19:15:03Z

docs/clusterdefinition.md

@@ -49,6 +49,7 @@ Here are the valid values for the orchestrator types:
 |Name of addon|Enabled by default?|How many containers|Description|
 |tiller|true|Delivers the Helm server-side component: tiller. See https://github.com/kubernetes/helm for more info.|
 |kubernetes-dashboard|true|1|Delivers the kubernetes dashboard component. See https://github.com/kubernetes/dashboard for more info.|
+|rescheduler|false|Delivers the kubernetes rescheduler component.|


I couldn't find the kubernetes repo for rescheduler

jackfrancis

lgtm

CecileRobertMichon added 22 commits October 26, 2017 10:11

merge master upstream

ab8d33d

Revert "merge master upstream"

dca734d

This reverts commit ab8d33d.

Merge remote-tracking branch 'upstream/master'

bae42e6

Merge remote-tracking branch 'upstream/master'

8840dec

Merge remote-tracking branch 'upstream/master'

5cf7e98

Merge remote-tracking branch 'upstream/master'

94ab4c1

Merge branch 'master' of https://github.com/Azure/acs-engine

bb5b2ce

Merge remote-tracking branch 'upstream/master'

bedfe00

merge master upstream

02de386

Revert "merge master upstream"

86b004e

This reverts commit ab8d33d.

Merge branch 'master' of https://github.com/CecileRobertMichon/acs-en…

981cf24

…gine

Merge remote-tracking branch 'upstream/master'

989de34

Merge remote-tracking branch 'upstream/master'

be3c624

added rescheduler addon

b840a41

fix typo

9c2a09a

merge master upstream

ae8d620

Revert "merge master upstream"

d31849e

This reverts commit ab8d33d.

merge master upstream

afb09d7

Revert "merge master upstream"

5bdf726

This reverts commit ab8d33d.

added rescheduler addon

5cf36c1

fix typo

900cd02

Merge branch 'rescheduler' of https://github.com/CecileRobertMichon/a…

46de126

…cs-engine into rescheduler

ghost assigned CecileRobertMichon Nov 16, 2017

ghost added the in progress label Nov 16, 2017

CecileRobertMichon added 4 commits November 15, 2017 16:51

add rescheduler spec to mastervars

b04de96

add rescheduler spec to mastervars

1b1bc40

Merge branch 'rescheduler' of https://github.com/CecileRobertMichon/a…

0eb1314

…cs-engine into rescheduler

added rescheduler image to KubeConfigs

d4df8fc

CecileRobertMichon added awaiting review and removed in progress labels Nov 16, 2017

changed to using new addon pattern

93e58e5

CecileRobertMichon commented Nov 16, 2017

View reviewed changes

CecileRobertMichon added 3 commits November 16, 2017 15:34

parametrize limits/requests

e4ef915

fix types test for isReschedulerEnabled

c0b3f35

fix var name in test

e69d7b7

CecileRobertMichon removed the in progress label Nov 16, 2017

added rescheduler limit params

35d65f0

jackfrancis reviewed Nov 17, 2017

View reviewed changes

CecileRobertMichon added 2 commits November 17, 2017 09:56

fixed pr comments

3beedcd

removed extra line

8823cb3

jackfrancis reviewed Nov 17, 2017

View reviewed changes

add rescheduler regression test + added docs + fixed dashboard name i…

083ec7d

…n docs

CecileRobertMichon commented Nov 17, 2017

View reviewed changes

typo fix

ee7e850

CecileRobertMichon added the DO-NOT-MERGE label Nov 17, 2017

jackfrancis approved these changes Nov 18, 2017

View reviewed changes

jackfrancis merged commit 0ec78c7 into Azure:master Nov 18, 2017

ghost removed the awaiting review label Nov 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Rescheduler addon #1780

Add Rescheduler addon #1780

CecileRobertMichon commented Nov 16, 2017

CecileRobertMichon Nov 16, 2017

jackfrancis Nov 16, 2017

jackfrancis Nov 16, 2017

CecileRobertMichon Nov 16, 2017

jackfrancis Nov 16, 2017

CecileRobertMichon Nov 16, 2017

CecileRobertMichon Nov 16, 2017 •

edited

Loading

jackfrancis Nov 16, 2017

CecileRobertMichon Nov 16, 2017 •

edited

Loading

jackfrancis Nov 16, 2017

CecileRobertMichon Nov 16, 2017

CecileRobertMichon Nov 16, 2017

jackfrancis Nov 16, 2017

CecileRobertMichon Nov 16, 2017

jackfrancis Nov 16, 2017

CecileRobertMichon Nov 16, 2017

jackfrancis Nov 17, 2017

jackfrancis Nov 17, 2017

CecileRobertMichon Nov 17, 2017

jackfrancis Nov 17, 2017

jackfrancis Nov 17, 2017

jackfrancis Nov 17, 2017

jackfrancis Nov 17, 2017

jackfrancis Nov 17, 2017

CecileRobertMichon Nov 17, 2017

CecileRobertMichon Nov 17, 2017

jackfrancis left a comment

Add Rescheduler addon #1780

Add Rescheduler addon #1780

Conversation

CecileRobertMichon commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CecileRobertMichon Nov 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CecileRobertMichon Nov 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackfrancis left a comment

Choose a reason for hiding this comment

CecileRobertMichon Nov 16, 2017 •

edited

Loading

CecileRobertMichon Nov 16, 2017 •

edited

Loading