From 280b833f397f26ac687f65ad3ae19b5db2152bc7 Mon Sep 17 00:00:00 2001 From: Konstantinos Tsakalozos Date: Mon, 27 Nov 2017 21:53:22 +0200 Subject: [PATCH 1/5] Fix master upgrade cornercases --- .../reactive/kubernetes_master.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py index 1547bc8bc93..4b0a94a9abe 100644 --- a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py +++ b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py @@ -102,12 +102,25 @@ def check_for_upgrade_needed(): add_rbac_roles() set_state('reconfigure.authentication.setup') remove_state('authentication.setup') + if should_reinstall_snaps(): + set_upgrade_needed() + + +def should_reinstall_snaps(): + ''' Return true if we should redeploy snaps. ''' + # Snaps should be upgrades if: + # a) channel changed, or + # b) the Charms attached snaps (resources) changed + config = hookenv.config() + previous_channel = config.previous('channel') + new_channel = hookenv.config('channel') + if new_channel != previous_channel: + return True resources = ['kubectl', 'kube-apiserver', 'kube-controller-manager', 'kube-scheduler', 'cdk-addons'] paths = [hookenv.resource_get(resource) for resource in resources] - if any_file_changed(paths): - set_upgrade_needed() + return any_file_changed(paths) def add_rbac_roles(): @@ -360,6 +373,7 @@ def set_app_version(): @when('cdk-addons.configured', 'kube-api-endpoint.available', 'kube-control.connected') +@when_not('kubernetes-master.upgrade-needed') def idle_status(kube_api, kube_control): ''' Signal at the end of the run that we are running. ''' if not all_kube_system_pods_running(): From 1550df99eb4fb1a5c012f68105393419760a5025 Mon Sep 17 00:00:00 2001 From: Konstantinos Tsakalozos Date: Tue, 28 Nov 2017 12:24:40 +0200 Subject: [PATCH 2/5] The change in channels will be caught config change after the upgrade. --- .../reactive/kubernetes_master.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py index 4b0a94a9abe..5cbe89d2e43 100644 --- a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py +++ b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py @@ -102,25 +102,11 @@ def check_for_upgrade_needed(): add_rbac_roles() set_state('reconfigure.authentication.setup') remove_state('authentication.setup') - if should_reinstall_snaps(): - set_upgrade_needed() - - -def should_reinstall_snaps(): - ''' Return true if we should redeploy snaps. ''' - # Snaps should be upgrades if: - # a) channel changed, or - # b) the Charms attached snaps (resources) changed - config = hookenv.config() - previous_channel = config.previous('channel') - new_channel = hookenv.config('channel') - if new_channel != previous_channel: - return True - resources = ['kubectl', 'kube-apiserver', 'kube-controller-manager', 'kube-scheduler', 'cdk-addons'] paths = [hookenv.resource_get(resource) for resource in resources] - return any_file_changed(paths) + if any_file_changed(paths): + set_upgrade_needed() def add_rbac_roles(): From 61d984843852661b56922c9da46e019e2a70cd9e Mon Sep 17 00:00:00 2001 From: Konstantinos Tsakalozos Date: Tue, 28 Nov 2017 20:47:19 +0200 Subject: [PATCH 3/5] Improve handling of snap resources --- .../reactive/kubernetes_master.py | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py index 5cbe89d2e43..95f7a8a2a3b 100644 --- a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py +++ b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py @@ -62,7 +62,6 @@ nrpe.Check.shortname_re = '[\.A-Za-z0-9-_]+$' os.environ['PATH'] += os.pathsep + os.path.join(os.sep, 'snap', 'bin') - def set_upgrade_needed(): set_state('kubernetes-master.upgrade-needed') config = hookenv.config() @@ -102,11 +101,28 @@ def check_for_upgrade_needed(): add_rbac_roles() set_state('reconfigure.authentication.setup') remove_state('authentication.setup') + if snap_resources_changed(): + set_upgrade_needed() + + +def snap_resources_changed(): + ''' + Check if the snapped resources have changed. The first time this method is + called will report no change. + + Returns: True in case a snap resource file has changed + + ''' + db = unitdata.kv() resources = ['kubectl', 'kube-apiserver', 'kube-controller-manager', 'kube-scheduler', 'cdk-addons'] paths = [hookenv.resource_get(resource) for resource in resources] - if any_file_changed(paths): - set_upgrade_needed() + if db.get('snap.resources.fingerprint.initialised'): + return any_file_changed(paths) + else: + db.set('snap.resources.fingerprint.initialised', True) + any_file_changed(paths) + return False def add_rbac_roles(): @@ -221,6 +237,7 @@ def install_snaps(): snap.install('kube-scheduler', channel=channel) hookenv.status_set('maintenance', 'Installing cdk-addons snap') snap.install('cdk-addons', channel=channel) + snap_resources_changed() set_state('kubernetes-master.snaps.installed') remove_state('kubernetes-master.components.started') From f0ace95218825c6218cba954640e830fea4498c3 Mon Sep 17 00:00:00 2001 From: Konstantinos Tsakalozos Date: Wed, 29 Nov 2017 09:56:42 +0200 Subject: [PATCH 4/5] Handling the case of an upgrade from a non-rolling master with resource change --- .../reactive/kubernetes_master.py | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py index 95f7a8a2a3b..a56737e16d7 100644 --- a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py +++ b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py @@ -62,13 +62,14 @@ nrpe.Check.shortname_re = '[\.A-Za-z0-9-_]+$' os.environ['PATH'] += os.pathsep + os.path.join(os.sep, 'snap', 'bin') -def set_upgrade_needed(): + +def set_upgrade_needed(forced=False): set_state('kubernetes-master.upgrade-needed') config = hookenv.config() previous_channel = config.previous('channel') require_manual = config.get('require-manual-upgrade') hookenv.log('set upgrade needed') - if previous_channel is None or not require_manual: + if previous_channel is None or not require_manual or forced: hookenv.log('forcing upgrade') set_state('kubernetes-master.upgrade-specified') @@ -101,16 +102,27 @@ def check_for_upgrade_needed(): add_rbac_roles() set_state('reconfigure.authentication.setup') remove_state('authentication.setup') - if snap_resources_changed(): + changed = snap_resources_changed() + if changed == 'yes': set_upgrade_needed() + elif changed == 'unknown': + # We are here on an upgrade from non-rolling master + # Since this upgrade might also include resource updates eg + # juju upgrade-charm kubernetes-master --resource kube-any=my.snap + # we take no risk and forcibly upgrade the snaps. + # Forcibly means we do not prompt the user to call the upgrade action. + set_upgrade_needed(forced=True) + def snap_resources_changed(): ''' Check if the snapped resources have changed. The first time this method is - called will report no change. + called will report "unknown". - Returns: True in case a snap resource file has changed + Returns: "yes" in case a snap resource file has changed, + "no" in case a snap resources are the same as last call, + "unknown" if it is the first time this method is called ''' db = unitdata.kv() @@ -118,11 +130,12 @@ def snap_resources_changed(): 'kube-scheduler', 'cdk-addons'] paths = [hookenv.resource_get(resource) for resource in resources] if db.get('snap.resources.fingerprint.initialised'): - return any_file_changed(paths) + result = 'yes' if any_file_changed(paths) else 'no' + return result else: db.set('snap.resources.fingerprint.initialised', True) any_file_changed(paths) - return False + return 'unknown' def add_rbac_roles(): From 0f591aeabc2572d245b101676f1a211ac2ac6433 Mon Sep 17 00:00:00 2001 From: Konstantinos Tsakalozos Date: Fri, 1 Dec 2017 16:28:32 +0200 Subject: [PATCH 5/5] Fix flake8 error --- .../juju/layers/kubernetes-master/reactive/kubernetes_master.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py index a56737e16d7..942171f7f3e 100644 --- a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py +++ b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py @@ -114,7 +114,6 @@ def check_for_upgrade_needed(): set_upgrade_needed(forced=True) - def snap_resources_changed(): ''' Check if the snapped resources have changed. The first time this method is