Merge pull request #59803 from wwwtyro/rye/robust-label-change

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Avoid hook errors when effecting label changes on kubernetes-worker charm.

**What this PR does / why we need it**: Avoid hook errors when effecting label changes on kubernetes-worker charm.

**Release note**:
```release-note
Avoid hook errors when effecting label changes on kubernetes-worker charm.
```

cluster/juju/layers/kubernetes-worker/reactive/kubernetes_worker.py

@@ -365,7 +365,7 @@ def start_worker(kube_api, kube_control, auth_control, cni):
     set_state('kubernetes-worker.config.created')
     restart_unit_services()
     update_kubelet_status()
-    apply_node_labels()
+    set_state('kubernetes-worker.label-config-required')
     remove_state('kubernetes-worker.restart-needed')
 
 
@@ -412,37 +412,54 @@ def render_and_launch_ingress():
         hookenv.close_port(443)
 
 
-@when('config.changed.labels', 'kubernetes-worker.config.created')
+@when('config.changed.labels')
+def handle_labels_changed():
+    set_state('kubernetes-worker.label-config-required')
+
+
+@when('kubernetes-worker.label-config-required',
+      'kubernetes-worker.config.created')
 def apply_node_labels():
-    ''' Parse the labels configuration option and apply the labels to the node.
+    ''' Parse the labels configuration option and apply the labels to the
-    '''
+        node. '''
-    # scrub and try to format an array from the configuration option
+    # Get the user's configured labels.
     config = hookenv.config()
-    user_labels = _parse_labels(config.get('labels'))
+    user_labels = {}
-
+    for item in config.get('labels').split(' '):
-    # For diffing sake, iterate the previous label set
+        if '=' in item:
-    if config.previous('labels'):
+            key, val = item.split('=')
-        previous_labels = _parse_labels(config.previous('labels'))
+            user_labels[key] = val
-        hookenv.log('previous labels: {}'.format(previous_labels))
+        else:
-    else:
+            hookenv.log('Skipping malformed option: {}.'.format(item))
-        # this handles first time run if there is no previous labels config
+    # Collect the current label state.
-        previous_labels = _parse_labels("")
+    current_labels = db.get('current_labels') or {}
-
+    # Remove any labels that the user has removed from the config.
-    # Calculate label removal
+    for key in list(current_labels.keys()):
-    for label in previous_labels:
+        if key not in user_labels:
-        if label not in user_labels:
+            try:
-            hookenv.log('Deleting node label {}'.format(label))
+                remove_label(key)
-            _apply_node_label(label, delete=True)
+                del current_labels[key]
-        # if the label is in user labels we do nothing here, it will get set
+                db.set('current_labels', current_labels)
-        # during the atomic update below.
+            except ApplyNodeLabelFailed as e:
-
+                hookenv.log(str(e))
-    # Atomically set a label
+                return
-    for label in user_labels:
+    # Add any new labels.
-        _apply_node_label(label, overwrite=True)
+    for key, val in user_labels.items():
-
+        try:
-    # Set label for application name
+            set_label(key, val)
-    _apply_node_label('juju-application={}'.format(hookenv.service_name()),
+            current_labels[key] = val
-                      overwrite=True)
+            db.set('current_labels', current_labels)
+        except ApplyNodeLabelFailed as e:
+            hookenv.log(str(e))
+            return
+    # Set the juju-application label.
+    try:
+        set_label('juju-application', hookenv.service_name())
+    except ApplyNodeLabelFailed as e:
+        hookenv.log(str(e))
+        return
+    # Label configuration complete.
+    remove_state('kubernetes-worker.label-config-required')
 
 
 @when_any('config.changed.kubelet-extra-args',
@@ -881,8 +898,8 @@ def enable_gpu():
         return
 
     # Apply node labels
-    _apply_node_label('gpu=true', overwrite=True)
+    set_label('gpu', 'true')
-    _apply_node_label('cuda=true', overwrite=True)
+    set_label('cuda', 'true')
 
     set_state('kubernetes-worker.gpu.enabled')
     set_state('kubernetes-worker.restart-needed')
@@ -902,8 +919,8 @@ def disable_gpu():
     hookenv.log('Disabling gpu mode')
 
     # Remove node labels
-    _apply_node_label('gpu', delete=True)
+    remove_label('gpu')
-    _apply_node_label('cuda', delete=True)
+    remove_label('cuda')
 
     remove_state('kubernetes-worker.gpu.enabled')
     set_state('kubernetes-worker.restart-needed')
@@ -1012,43 +1029,33 @@ class ApplyNodeLabelFailed(Exception):
     pass
 
 
-def _apply_node_label(label, delete=False, overwrite=False):
+def persistent_call(cmd, retry_message):
-    ''' Invoke kubectl to apply node label changes '''
-    nodename = get_node_name()
-
-    # TODO: Make this part of the kubectl calls instead of a special string
-    cmd_base = 'kubectl --kubeconfig={0} label node {1} {2}'
-
-    if delete is True:
-        label_key = label.split('=')[0]
-        cmd = cmd_base.format(kubeconfig_path, nodename, label_key)
-        cmd = cmd + '-'
-    else:
-        cmd = cmd_base.format(kubeconfig_path, nodename, label)
-        if overwrite:
-            cmd = '{} --overwrite'.format(cmd)
-    cmd = cmd.split()
-
     deadline = time.time() + 180
     while time.time() < deadline:
         code = subprocess.call(cmd)
         if code == 0:
-            break
+            return True
-        hookenv.log('Failed to apply label %s, exit code %d. Will retry.' % (
+        hookenv.log(retry_message)
-            label, code))
         time.sleep(1)
     else:
-        msg = 'Failed to apply label %s' % label
+        return False
-        raise ApplyNodeLabelFailed(msg)
 
 
-def _parse_labels(labels):
+def set_label(label, value):
-    ''' Parse labels from a key=value string separated by space.'''
+    nodename = get_node_name()
-    label_array = labels.split(' ')
+    cmd = 'kubectl --kubeconfig={0} label node {1} {2}={3} --overwrite'
-    sanitized_labels = []
+    cmd = cmd.format(kubeconfig_path, nodename, label, value)
-    for item in label_array:
+    cmd = cmd.split()
-        if '=' in item:
+    retry = 'Failed to apply label %s=%s. Will retry.' % (label, value)
-            sanitized_labels.append(item)
+    if not persistent_call(cmd, retry):
-        else:
+        raise ApplyNodeLabelFailed(retry)
-            hookenv.log('Skipping malformed option: {}'.format(item))
+
-    return sanitized_labels
+
+def remove_label(label):
+    nodename = get_node_name()
+    cmd = 'kubectl --kubeconfig={0} label node {1} {2}-'
+    cmd = cmd.format(kubeconfig_path, nodename, label)
+    cmd = cmd.split()
+    retry = 'Failed to remove label {0}. Will retry.'.format(label)
+    if not persistent_call(cmd, retry):
+        raise ApplyNodeLabelFailed(retry)