fix: use reconcile mode in addon specs #1401
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1401 +/- ##
=======================================
Coverage 75.52% 75.52%
=======================================
Files 128 128
Lines 18132 18132
=======================================
Hits 13695 13695
Misses 3643 3643
Partials 794 794 |
| @@ -52,7 +52,7 @@ metadata: | |||
| name: kube-dns | |||
| namespace: kube-system | |||
| labels: | |||
| addonmanager.kubernetes.io/mode: EnsureExists | |||
| addonmanager.kubernetes.io/mode: Reconcile | |||
There was a problem hiding this comment.
I think we probably want to keep this as EnsureExists as a Reconcile might blow away a pre-existing ConfigMap with data in it.
Ref:
https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/dns/kube-dns/kube-dns.yaml.in
There was a problem hiding this comment.
updated all ConfigMaps to use EnsureExists
There was a problem hiding this comment.
I'm torn about the omsagent ConfigMap since it data that is part of the ConfigMap resource definition, wondering I'm thinking we want to leave it to Reconcile so changes to that data get picked up by updates (it's currently set to Reconcile):
parts/k8s/addons/coredns.yaml
Outdated
| @@ -35,7 +35,7 @@ metadata: | |||
| rbac.authorization.kubernetes.io/autoupdate: "true" | |||
| labels: | |||
| kubernetes.io/bootstrapping: rbac-defaults | |||
| addonmanager.kubernetes.io/mode: EnsureExists | |||
There was a problem hiding this comment.
So, same comment about ConfigMap resources everywhere, but I'm torn on this particular ClusterRoleBinding as it is set to EnsureExists in the reference yaml:
https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/dns/coredns/coredns.yaml.in
But I don't know why you wouldn't want to set this to Reconcile to allow the spec to adapt over time. @lachie83 @khenidak Any idea why you would bootstrap an addon spec w/ a ClusterRoleBinding set to EnsureExists? (That means essentially that the next time the addon manager picks up a change to this resource from /etc/kubernetes/addons/* it will not automatically reconcile it in the runtime)
There was a problem hiding this comment.
I left this as EnsureExists for now
|
lgtm |
CecileRobertMichon
force-pushed
the
addons-reconcile
branch
from
201c779 to
90f6a70
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon, mboersma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reason for Change:
Addon specs delivered via aks-engine should use
Reconcilemode in order for clusters to use the latest spec after an upgrade or a scale (ie. whenever the addon spec gets rewritten on disk in /etc/kubernetes/addons). Otherwise, new values in the addon spec such as new image versions won't be applied after an upgrade.Issue Fixed:
Related to #1372
Requirements:
Notes:
Omitted Heapster purposefully because of #275