assorted kubeadm configmap compatibility issues

There is an upstream issue in Kubeadm (affecting at least up till
1.24.4) where if the "certSANs" field of the kubeadm configmap contains
unquoted IPv6 addresses starting with colons in "flow style" it will
choke while parsing.

The problematic formatting looks like this:

        ClusterConfiguration: |
            apiServer:
                certSANs: [::1, 192.168.206.1, 127.0.0.1, 10.20.7.3]

While this is fine:

          ClusterConfiguration: |
            apiServer:
                certSANs:
                - ::1
                - 192.168.206.1
                - 127.0.0.1
                - 10.20.7.3

It also works to wrap each IPv6 address in quotes.

It's not clear what causes the certSANs field to be formatted in flow
style, but it was seen in testing after a platform upgrade followed
by a k8s upgrade.

The workaround is to modify the "upgrade first control plane" code
to update the configmap 'certSANs' field to block style if it's in
flow style and contains IPv6 addresses.

I've opened an upstream issue:
kubernetes/kubeadm#2858

We'll hit the same error in _get_kubernetes_join_cmd(), but since that
code is run more frequently rather than reformatting the configmap
we modify the code to explicitly set the certificate key rather than
passing in the whole kubeadm config file.  This is arguably how it
should have been done originally.

In StarlingX 7 by default we set the "HugePageStorageMediumSize=true"
feature gate in the kube-apiserver section of the kubeadm configmap.
In k8s 1.24 it's no longer supported.  In StarlingX 8 we remove it
from various locations (kubelet config, service parameters, etc.)
but we also need to remove it from the kubeadm configmap.

Test Plan:
PASS: platform upgrade from Starlingx 7 to 8, then K8s upgrade to 1.24
PASS: add "::1" address to certSANS in configmap then upgrade k8s
PASS: set HugePageStorageMediumSize in cm then upgrade k8s to 1.24

Change-Id: I45e9e22585a5b2912a339ad5905d011e3adc29ab
Closes-Bug: 2016041
Signed-off-by: Chris Friesen <[email protected]>

Author: cbf123

sysinv/sysinv/sysinv/sysinv/common/kubernetes.py

@@ -11,9 +11,13 @@
 
 from __future__ import absolute_import
 from distutils.version import LooseVersion
+from ipaddress import ip_address
+from ipaddress import IPv4Address
 import json
 import os
 import re
+import ruamel.yaml as yaml
+from ruamel.yaml.compat import StringIO
 import time
 import tsconfig.tsconfig as tsc
 
@@ -1293,3 +1297,58 @@ def get_cert_secret(self, name, namespace, max_retries=60):
                 return secret
             time.sleep(1)
         return None
+
+    def kubeadm_configmap_reformat(self, target_version):
+        """
+        There is an upstream issue in Kubeadm (affecting at least up till 1.24.4)
+        where if the "certSANs" field of the kubeadm configmap contains unquoted
+        IPv6 addresses in "flow style" it will choke while parsing.  The problematic
+        formatting looks like this:
+
+        ClusterConfiguration: |
+            apiServer:
+                certSANs: [::1, 192.168.206.1, 127.0.0.1, 10.20.7.3]
+
+        While this is fine:
+
+          ClusterConfiguration: |
+            apiServer:
+                certSANs:
+                - ::1
+                - 192.168.206.1
+                - 127.0.0.1
+                - 10.20.7.3
+        """
+        try:
+            configmap_name = 'kubeadm-config'
+            configmap = self.kube_read_config_map(configmap_name, 'kube-system')
+            newyaml = yaml.YAML()
+            stream = StringIO(configmap.data['ClusterConfiguration'])
+            info = newyaml.load(stream)
+            flow_style = info['apiServer']['certSANs'].fa.flow_style()
+            if flow_style:
+                # It's using flow syle, so we need to check if any addresses are IPv6.
+                need_reformat = False
+                try:
+                    for addr in info['apiServer']['certSANs']:
+                        if type(ip_address(addr)) is not IPv4Address:
+                            need_reformat = True
+                            break
+                except ValueError:
+                    # Shouldn't happen if addresses are well-formed.
+                    # If it does then reformat to be safe.
+                    need_reformat = True
+
+                if need_reformat:
+                    LOG.info('Converting kubeadm configmap certSANs to block style.')
+                    info['apiServer']['certSANs'].fa.set_block_style()
+                    outstream = StringIO()
+                    newyaml.dump(info, outstream)
+                    configmap = {'data': {'ClusterConfiguration': outstream.getvalue()}}
+                    self.kube_patch_config_map(configmap_name, 'kube-system', configmap)
+                    LOG.info('Successfully reformatted kubeadm configmap.')
+        except Exception as e:
+            LOG.exception("Unable to patch kubeadm config_map: %s" % e)
+            return 1
+
+        return 0

sysinv/sysinv/sysinv/sysinv/conductor/manager.py

@@ -14958,6 +14958,11 @@ def kube_upgrade_control_plane(self, context, host_uuid):
                 LOG.error("Problem sanitizing kubelet configmap feature gates.")
                 rc = 1
 
+            # Work around upstream kubeadm configmap parsing issue.
+            if self._kube.kubeadm_configmap_reformat(target_version) == 1:
+                LOG.error("Problem reformatting kubelet configmap.")
+                rc = 1
+
             if rc == 1:
                 kube_upgrade_obj.state = fail_state
                 kube_upgrade_obj.save()
@@ -16592,7 +16597,10 @@ def sanitize_feature_gates_kubeadm_configmap(self, target_version):
                 try:
                     feature_gates = sanitize_feature_gates(feature_gates,
                                 'RemoveSelfLink=false')
-                    if target_version == 'v1.25.3':
+                    if target_version == 'v1.24.4':
+                        feature_gates = sanitize_feature_gates(feature_gates,
+                                    'HugePageStorageMediumSize=true')
+                    elif target_version == 'v1.25.3':
                         feature_gates = sanitize_feature_gates(feature_gates,
                                     'TTLAfterFinished=true')
                     if not feature_gates:

sysinv/sysinv/sysinv/sysinv/puppet/kubernetes.py

@@ -9,10 +9,8 @@
 import json
 import keyring
 import netaddr
-import os
 import random
 import re
-import tempfile
 
 from oslo_log import log as logging
 from sysinv.common import constants
@@ -244,41 +242,15 @@ def _get_kubernetes_join_cmd(self, host):
         try:
             join_cmd_additions = ''
             if host.personality == constants.CONTROLLER:
-                # Upload the certificates used during kubeadm join
-                # The cert key will be printed in the last line of the output
-
-                # We will create a temp file with the kubeadm config
-                # We need this because the kubeadm config could have changed
-                # since bootstrap. Reading the kubeadm config each time
-                # it is needed ensures we are not using stale data
-
-                fd, temp_kubeadm_config_view = tempfile.mkstemp(
-                    dir='/tmp', suffix='.yaml')
-                with os.fdopen(fd, 'w') as f:
-                    cmd = ['kubectl', 'get', 'cm', '-n', 'kube-system',
-                           'kubeadm-config', '-o=jsonpath={.data.ClusterConfiguration}',
-                           KUBECONFIG]
-                    subprocess.check_call(cmd, stdout=f)  # pylint: disable=not-callable
-
-                # We will use a custom key to encrypt kubeadm certificates
-                # to make sure all hosts decrypt using the same key
-
+                # Upload the certificates used during kubeadm join.
                 key = str(keyring.get_password(CERTIFICATE_KEY_SERVICE,
-                        CERTIFICATE_KEY_USER))
-
-                with open(temp_kubeadm_config_view, "a") as f:
-                    f.write("---\r\napiVersion: kubeadm.k8s.io/v1beta2\r\n"
-                            "kind: InitConfiguration\r\ncertificateKey: "
-                            "{}".format(key))
-
+                                               CERTIFICATE_KEY_USER))
                 cmd = ['kubeadm', 'init', 'phase', 'upload-certs',
-                       '--upload-certs', '--config',
-                       temp_kubeadm_config_view]
-
+                       '--upload-certs', '--certificate-key', key]
                 subprocess.check_call(cmd)  # pylint: disable=not-callable
-                join_cmd_additions = \
-                    " --control-plane --certificate-key %s" % key
-                os.unlink(temp_kubeadm_config_view)
+
+                # Now add the key to the join command.
+                join_cmd_additions = " --control-plane --certificate-key %s" % key
 
                 # Configure the IP address of the API Server for the controller host.
                 # If not set the default network interface will be used, which does not

sysinv/sysinv/sysinv/sysinv/tests/common/test_kubernetes.py

@@ -1249,6 +1249,79 @@ def test_kube_list_custom_resource_pretty(self, mock_list_cluster_custom_object)
             watch=False
         )
 
+    def test_kubeadm_configmap_reformat(self):
+        mock_kube_patch_config_map = mock.MagicMock()
+        p2 = mock.patch(
+            'sysinv.common.kubernetes.KubeOperator.kube_patch_config_map',
+            mock_kube_patch_config_map)
+        p2.start().return_value = None
+        self.addCleanup(p2.stop)
+
+        # Test IPv4 only in block style
+        self.read_namespaced_config_map_result = kubernetes.client.V1ConfigMap(
+            api_version="v1",
+            data={"ClusterConfiguration":
+                  "apiServer:\n"
+                  "  certSANs:\n"
+                  "  - 127.0.0.1\n"
+                  "  - 192.168.206.2\n"
+                  },
+            metadata=kubernetes.client.V1ObjectMeta(
+                name="kubeadm-config",
+                namespace="kube-system"),
+        )
+        self.kube_operator.kubeadm_configmap_reformat('dummy')
+        mock_kube_patch_config_map.assert_not_called()
+
+        # Test IPv4 only in flow style
+        self.read_namespaced_config_map_result = kubernetes.client.V1ConfigMap(
+            api_version="v1",
+            data={"ClusterConfiguration":
+                  "apiServer:\n"
+                  "  certSANs: [127.0.0.1, 192.168.206.2]\n"
+                  },
+            metadata=kubernetes.client.V1ObjectMeta(
+                name="kubeadm-config",
+                namespace="kube-system"),
+        )
+        self.kube_operator.kubeadm_configmap_reformat('dummy')
+        mock_kube_patch_config_map.assert_not_called()
+
+        # Test IPv6 and IPv4 in block style
+        self.read_namespaced_config_map_result = kubernetes.client.V1ConfigMap(
+            api_version="v1",
+            data={"ClusterConfiguration":
+                  "apiServer:\n"
+                  "  certSANs:\n"
+                  "  - ::1\n"
+                  "  - 192.168.206.2\n"
+                  },
+            metadata=kubernetes.client.V1ObjectMeta(
+                name="kubeadm-config",
+                namespace="kube-system"),
+        )
+        self.kube_operator.kubeadm_configmap_reformat('dummy')
+        mock_kube_patch_config_map.assert_not_called()
+
+        # Test IPv6 and IPv4 in flow style
+        self.read_namespaced_config_map_result = kubernetes.client.V1ConfigMap(
+            api_version="v1",
+            data={"ClusterConfiguration":
+                  "apiServer:\n"
+                  "  certSANs: [::1, 127.0.0.1]\n"
+                  },
+            metadata=kubernetes.client.V1ObjectMeta(
+                name="kubeadm-config",
+                namespace="kube-system"),
+        )
+        self.kube_operator.kubeadm_configmap_reformat('dummy')
+        patch_config_map_arg = {
+            'data': {
+                'ClusterConfiguration':
+                    'apiServer:\n  certSANs:\n  - ::1\n  - 127.0.0.1\n'}}
+        mock_kube_patch_config_map.assert_called_with(
+                'kubeadm-config', 'kube-system', patch_config_map_arg)
+
 
 class TestKubernetesUtilities(base.TestCase):
     def test_is_kube_version_supported(self):

sysinv/sysinv/sysinv/sysinv/tests/conductor/test_manager.py

@@ -73,6 +73,11 @@ def __init__(self, db_api):
         self.update_secure_system_config = mock.MagicMock()
 
 
+class FakeKubeOperator(object):
+    def __init__(self):
+        self.kubeadm_configmap_reformat = mock.MagicMock()
+
+
 class FakePopen(object):
 
     def __init__(self, **kwargs):
@@ -221,6 +226,43 @@ def setUp(self):
         ttl_patch += 'v1.42.1\nscheduler: {}\n'
         self.kubeadm_config_map_patch_ttlafterfinished = {'data': {'ClusterConfiguration': ttl_patch}}
 
+        self.kubeadm_config_read_HugePageStorageMediumSize = kubernetes.client.V1ConfigMap(
+            api_version='v1',
+            data={'ClusterConfiguration': 'apiServer:\n'
+                                            '  certSANs:\n'
+                                            '  - 192.168.206.1\n'
+                                            '  - 127.0.0.1\n'
+                                            '  - 10.10.6.3\n'
+                                            '  extraArgs:\n'
+                                            '    event-ttl: 24h\n'
+                                            '    feature-gates: HugePageStorageMediumSize=true,Foo=false\n'
+                                            '  extraVolumes:\n'
+                                            '  - hostPath: '
+                                            '/etc/kubernetes/encryption-provider.yaml\n'
+                                            'apiVersion: kubeadm.k8s.io/v1beta3\n'
+                                            'controllerManager:\n'
+                                            '  extraArgs:\n'
+                                            '    pod-eviction-timeout: 30s\n'
+                                            '    feature-gates: HugePageStorageMediumSize=true\n'
+                                            '  extraVolumes:\n'
+                                            'kind: ClusterConfiguration\n'
+                                            'kubernetesVersion: v1.23.1\n'
+                                            'scheduler: {}\n'},
+
+            metadata=kubernetes.client.V1ObjectMeta(
+                        name='kubeadm-config',
+                        namespace='kube-system'),
+        )
+        ttl_patch = 'apiServer:\n  certSANs: [192.168.206.1, 127.0.0.1, '
+        ttl_patch += '10.10.6.3]\n  extraArgs: {event-ttl: 24h, feature-gates: '
+        ttl_patch += 'Foo=false}\n  extraVolumes:\n  - '
+        ttl_patch += '{hostPath: /etc/kubernetes/encryption-provider.yaml}\n'
+        ttl_patch += 'apiVersion: kubeadm.k8s.io/v1beta3\ncontrollerManager:\n  '
+        ttl_patch += 'extraArgs: {pod-eviction-timeout: 30s}\n  extraVolumes: '
+        ttl_patch += 'null\nkind: ClusterConfiguration\nkubernetesVersion: '
+        ttl_patch += 'v1.23.1\nscheduler: {}\n'
+        self.kubeadm_config_map_patch_HugePageStorageMediumSize = {'data': {'ClusterConfiguration': ttl_patch}}
+
         self.kubeadm_config_map_read_image_repository = kubernetes.client.V1ConfigMap(
             api_version='v1',
             data={'ClusterConfiguration': 'apiServer:\n'
@@ -1272,6 +1314,8 @@ def test_kube_upgrade_control_plane_first_master(self):
         p2.start().return_value = 0
         self.addCleanup(p2.stop)
 
+        self.service._kube = FakeKubeOperator()
+
         # Speed up the test
         kubernetes.MANIFEST_APPLY_INTERVAL = 1
         kubernetes.POD_START_INTERVAL = 1
@@ -1349,6 +1393,8 @@ def test_kube_upgrade_control_plane_first_master_manifest_timeout(self):
         p2.start().return_value = 0
         self.addCleanup(p2.stop)
 
+        self.service._kube = FakeKubeOperator()
+
         # Speed up the test
         kubernetes.MANIFEST_APPLY_INTERVAL = 1
         kubernetes.MANIFEST_APPLY_TIMEOUT = 1
@@ -1425,6 +1471,8 @@ def test_kube_upgrade_control_plane_first_master_upgrade_fail(self):
         p2.start().return_value = 0
         self.addCleanup(p2.stop)
 
+        self.service._kube = FakeKubeOperator()
+
         # Speed up the test
         kubernetes.MANIFEST_APPLY_INTERVAL = 1
         kubernetes.POD_START_INTERVAL = 1
@@ -1788,6 +1836,33 @@ def test_sanitize_feature_gates_kubeadm_configmap_with_RemoveSelfLink(self):
         mock_kube_patch_config_map.assert_called_with(
                 'kubeadm-config', 'kube-system', self.kubeadm_config_map_patch_RemoveSelfLink)
 
+    def test_sanitize_feature_gates_kubeadm_configmap_with_HugePageStorageMediumSize(self):
+        """
+        This unit test covers the following use cases:
+        1. a component with an 'extraArgs' field containing 'feature-gates' with
+           only a "HugePageStorageMediumSize=true" entry
+        2. a component with an 'extraArgs' field containing 'feature-gates' with a
+           "HugePageStorageMediumSize=true" entry as well as others
+        """
+        mock_kube_read_config_map = mock.MagicMock()
+        p = mock.patch(
+            'sysinv.common.kubernetes.KubeOperator.kube_read_config_map',
+            mock_kube_read_config_map)
+        p.start().return_value = self.kubeadm_config_read_HugePageStorageMediumSize
+        self.addCleanup(p.stop)
+
+        mock_kube_patch_config_map = mock.MagicMock()
+        p2 = mock.patch(
+            'sysinv.common.kubernetes.KubeOperator.kube_patch_config_map',
+            mock_kube_patch_config_map)
+        p2.start().return_value = None
+        self.addCleanup(p2.stop)
+
+        self.service.start()
+        self.service.sanitize_feature_gates_kubeadm_configmap('v1.24.4')
+        mock_kube_patch_config_map.assert_called_with(
+                'kubeadm-config', 'kube-system', self.kubeadm_config_map_patch_HugePageStorageMediumSize)
+
     def test_sanitize_feature_gates_kubeadm_configmap_with_ttlafterfinished(self):
         mock_kube_read_config_map = mock.MagicMock()
         p = mock.patch(