From 5c11f663bea48cdf8de8dddb9cc3f0b502222650 Mon Sep 17 00:00:00 2001 From: George Kraft Date: Mon, 25 Jun 2018 14:53:56 -0500 Subject: [PATCH] juju: Fix upgrade actions not working with resources --- .../reactive/kubernetes_master.py | 88 +++++++++++++------ .../reactive/kubernetes_worker.py | 59 +++++++++++-- 2 files changed, 112 insertions(+), 35 deletions(-) diff --git a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py index aee8438efb..c34739b5fe 100644 --- a/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py +++ b/cluster/juju/layers/kubernetes-master/reactive/kubernetes_master.py @@ -15,6 +15,7 @@ # limitations under the License. import base64 +import hashlib import os import re import random @@ -64,8 +65,11 @@ from charmhelpers.contrib.charmsupport import nrpe nrpe.Check.shortname_re = '[\.A-Za-z0-9-_]+$' gcp_creds_env_key = 'GOOGLE_APPLICATION_CREDENTIALS' +snap_resources = ['kubectl', 'kube-apiserver', 'kube-controller-manager', + 'kube-scheduler', 'cdk-addons'] os.environ['PATH'] += os.pathsep + os.path.join(os.sep, 'snap', 'bin') +db = unitdata.kv() def set_upgrade_needed(forced=False): @@ -86,7 +90,6 @@ def channel_changed(): def service_cidr(): ''' Return the charm's service-cidr config ''' - db = unitdata.kv() frozen_cidr = db.get('kubernetes-master.service-cidr') return frozen_cidr or hookenv.config('service-cidr') @@ -94,7 +97,6 @@ def service_cidr(): def freeze_service_cidr(): ''' Freeze the service CIDR. Once the apiserver has started, we can no longer safely change this value. ''' - db = unitdata.kv() db.set('kubernetes-master.service-cidr', service_cidr()) @@ -107,10 +109,8 @@ def check_for_upgrade_needed(): add_rbac_roles() set_state('reconfigure.authentication.setup') remove_state('authentication.setup') - changed = snap_resources_changed() - if changed == 'yes': - set_upgrade_needed() - elif changed == 'unknown': + + if not db.get('snap.resources.fingerprint.initialised'): # 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 @@ -118,6 +118,9 @@ def check_for_upgrade_needed(): # Forcibly means we do not prompt the user to call the upgrade action. set_upgrade_needed(forced=True) + migrate_resource_checksums() + check_resources_for_upgrade_needed() + # Set the auto storage backend to etcd2. auto_storage_backend = leader_get('auto_storage_backend') is_leader = is_state('leadership.is_leader') @@ -125,27 +128,56 @@ def check_for_upgrade_needed(): leader_set(auto_storage_backend='etcd2') -def snap_resources_changed(): - ''' - Check if the snapped resources have changed. The first time this method is - called will report "unknown". +def get_resource_checksum_db_key(resource): + ''' Convert a resource name to a resource checksum database key. ''' + return 'kubernetes-master.resource-checksums.' + resource - 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() - resources = ['kubectl', 'kube-apiserver', 'kube-controller-manager', - 'kube-scheduler', 'cdk-addons'] - paths = [hookenv.resource_get(resource) for resource in resources] - if db.get('snap.resources.fingerprint.initialised'): - result = 'yes' if any_file_changed(paths) else 'no' - return result - else: - db.set('snap.resources.fingerprint.initialised', True) - any_file_changed(paths) - return 'unknown' +def calculate_resource_checksum(resource): + ''' Calculate a checksum for a resource ''' + md5 = hashlib.md5() + path = hookenv.resource_get(resource) + if path: + with open(path, 'rb') as f: + data = f.read() + md5.update(data) + return md5.hexdigest() + + +def migrate_resource_checksums(): + ''' Migrate resource checksums from the old schema to the new one ''' + for resource in snap_resources: + new_key = get_resource_checksum_db_key(resource) + if not db.get(new_key): + path = hookenv.resource_get(resource) + if path: + # old key from charms.reactive.helpers.any_file_changed + old_key = 'reactive.files_changed.' + path + old_checksum = db.get(old_key) + db.set(new_key, old_checksum) + else: + # No resource is attached. Previously, this meant no checksum + # would be calculated and stored. But now we calculate it as if + # it is a 0-byte resource, so let's go ahead and do that. + zero_checksum = hashlib.md5().hexdigest() + db.set(new_key, zero_checksum) + + +def check_resources_for_upgrade_needed(): + hookenv.status_set('maintenance', 'Checking resources') + for resource in snap_resources: + key = get_resource_checksum_db_key(resource) + old_checksum = db.get(key) + new_checksum = calculate_resource_checksum(resource) + if new_checksum != old_checksum: + set_upgrade_needed() + + +def calculate_and_store_resource_checksums(): + for resource in snap_resources: + key = get_resource_checksum_db_key(resource) + checksum = calculate_resource_checksum(resource) + db.set(key, checksum) def add_rbac_roles(): @@ -253,7 +285,8 @@ 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() + calculate_and_store_resource_checksums() + db.set('snap.resources.fingerprint.initialised', True) set_state('kubernetes-master.snaps.installed') remove_state('kubernetes-master.components.started') @@ -1123,8 +1156,6 @@ def parse_extra_args(config_key): def configure_kubernetes_service(service, base_args, extra_args_key): - db = unitdata.kv() - prev_args_key = 'kubernetes-master.prev_args.' + service prev_args = db.get(prev_args_key) or {} @@ -1412,7 +1443,6 @@ def set_token(password, save_salt): param: password - the password to be stored param: save_salt - the key to store the value of the token.''' - db = unitdata.kv() db.set(save_salt, password) return db.get(save_salt) diff --git a/cluster/juju/layers/kubernetes-worker/reactive/kubernetes_worker.py b/cluster/juju/layers/kubernetes-worker/reactive/kubernetes_worker.py index 43b69a9e95..e8032197d3 100644 --- a/cluster/juju/layers/kubernetes-worker/reactive/kubernetes_worker.py +++ b/cluster/juju/layers/kubernetes-worker/reactive/kubernetes_worker.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import hashlib import json import os import random @@ -36,7 +37,7 @@ from charms.reactive import when, when_any, when_not, when_none from charms.kubernetes.common import get_version -from charms.reactive.helpers import data_changed, any_file_changed +from charms.reactive.helpers import data_changed from charms.templating.jinja2 import render from charmhelpers.core import hookenv, unitdata @@ -52,6 +53,7 @@ kubeconfig_path = '/root/cdk/kubeconfig' kubeproxyconfig_path = '/root/cdk/kubeproxyconfig' kubeclientconfig_path = '/root/.kube/config' gcp_creds_env_key = 'GOOGLE_APPLICATION_CREDENTIALS' +snap_resources = ['kubectl', 'kubelet', 'kube-proxy'] os.environ['PATH'] += os.pathsep + os.path.join(os.sep, 'snap', 'bin') db = unitdata.kv() @@ -64,6 +66,7 @@ def upgrade_charm(): hookenv.atexit(remove_state, 'config.changed.install_from_upstream') cleanup_pre_snap_services() + migrate_resource_checksums() check_resources_for_upgrade_needed() # Remove the RC for nginx ingress if it exists @@ -88,12 +91,56 @@ def upgrade_charm(): set_state('kubernetes-worker.restart-needed') +def get_resource_checksum_db_key(resource): + ''' Convert a resource name to a resource checksum database key. ''' + return 'kubernetes-worker.resource-checksums.' + resource + + +def calculate_resource_checksum(resource): + ''' Calculate a checksum for a resource ''' + md5 = hashlib.md5() + path = hookenv.resource_get(resource) + if path: + with open(path, 'rb') as f: + data = f.read() + md5.update(data) + return md5.hexdigest() + + +def migrate_resource_checksums(): + ''' Migrate resource checksums from the old schema to the new one ''' + for resource in snap_resources: + new_key = get_resource_checksum_db_key(resource) + if not db.get(new_key): + path = hookenv.resource_get(resource) + if path: + # old key from charms.reactive.helpers.any_file_changed + old_key = 'reactive.files_changed.' + path + old_checksum = db.get(old_key) + db.set(new_key, old_checksum) + else: + # No resource is attached. Previously, this meant no checksum + # would be calculated and stored. But now we calculate it as if + # it is a 0-byte resource, so let's go ahead and do that. + zero_checksum = hashlib.md5().hexdigest() + db.set(new_key, zero_checksum) + + def check_resources_for_upgrade_needed(): hookenv.status_set('maintenance', 'Checking resources') - resources = ['kubectl', 'kubelet', 'kube-proxy'] - paths = [hookenv.resource_get(resource) for resource in resources] - if any_file_changed(paths): - set_upgrade_needed() + for resource in snap_resources: + key = get_resource_checksum_db_key(resource) + old_checksum = db.get(key) + new_checksum = calculate_resource_checksum(resource) + if new_checksum != old_checksum: + set_upgrade_needed() + + +def calculate_and_store_resource_checksums(): + for resource in snap_resources: + key = get_resource_checksum_db_key(resource) + checksum = calculate_resource_checksum(resource) + db.set(key, checksum) def set_upgrade_needed(): @@ -151,7 +198,6 @@ def upgrade_needed_status(): @when('kubernetes-worker.snaps.upgrade-specified') def install_snaps(): - check_resources_for_upgrade_needed() channel = hookenv.config('channel') hookenv.status_set('maintenance', 'Installing kubectl snap') snap.install('kubectl', channel=channel, classic=True) @@ -159,6 +205,7 @@ def install_snaps(): snap.install('kubelet', channel=channel, classic=True) hookenv.status_set('maintenance', 'Installing kube-proxy snap') snap.install('kube-proxy', channel=channel, classic=True) + calculate_and_store_resource_checksums() set_state('kubernetes-worker.snaps.installed') set_state('kubernetes-worker.restart-needed') remove_state('kubernetes-worker.snaps.upgrade-needed')