salt: Add a check about conflicting services for MetalK8s

If some service are started on the host where we want to deploy MetalK8s
the installation may not work properly (e.g.: firewalld)
Add a new function to check that those service are not started on the
host before deploying all the MetalK8s components.
NOTE: We do not automatically stop the service from the host since those
services may have been started for good reason, so just ask the user to
remove those packages

Fixes: #3067
Cherry-picked from: 0acf814

CHANGELOG.md

@@ -9,6 +9,12 @@
   (PR [#XXXX](https://github.com/scality/metalk8s/pull/XXXX), backport of
   PR [#3050](https://github.com/scality/metalk8s/pull/3050))
 
+- [#3067](https://github.com/scality/metalk8s/issues/3067) - Check for conflicting
+  services (`firewalld`) already started or enabled on target machines before
+  installation (bootstrap or expansion)
+  (PR [#XXXX](https://github.com/scality/metalk8s/pull/XXXX), backport of
+  PR [#3069](https://github.com/scality/metalk8s/pull/3069))
+
 ### Bug fixes
 - Do not install `containerd.io` instead of `containerd` and `runc` when this
   package is available in one configured repository

salt/_modules/metalk8s_checks.py

@@ -26,6 +26,11 @@ def node(raises=True, **kwargs):
     if pkg_ret is not True:
         errors.append(pkg_ret)
 
+    # Run `services` check
+    svc_ret = __salt__['metalk8s_checks.services'](raises=False, **kwargs)
+    if svc_ret is not True:
+        errors.append(svc_ret)
+
     # Compute return of the function
     if errors:
         error_msg = 'Node {}: {}'.format(__grains__['id'], '\n'.join(errors))
@@ -99,6 +104,51 @@ def packages(conflicting_packages=None, raises=True, **kwargs):
     return error_msg or True
 
 
+def services(conflicting_services=None, raises=True, **kwargs):
+    """Check if some conflicting service are started on the machine,
+    return a string (or raise if `raises` is set to `True`) with the list of
+    conflicting services.
+
+    Arguments:
+        conflicting_services (list): override the list of service that conflict with
+            MetalK8s installation.
+        raises (bool): the method will raise if there is any conflicting service
+            started.
+
+    Note: We have some logic in this function, so `conflicting_services` could be:
+    - a single string `<service_name>` which is the name of the conflicting service
+    - a list `[<service_name1>, <service_name2>]` with all conflicting service name
+    """
+    if conflicting_services is None:
+        conflicting_services = __salt__['metalk8s.get_from_map'](
+            'defaults', saltenv=kwargs.get('saltenv')
+        )['conflicting_services']
+
+    if isinstance(conflicting_services, str):
+        conflicting_services = [conflicting_services]
+    errors = []
+
+    for service_name in conflicting_services:
+        # `service.status`:
+        #   True = service started
+        #   False = service not available or stopped
+        # `service.disabled`:
+        #   True = service disabled or not available
+        #   False = service not disabled
+        if __salt__['service.status'](service_name) or \
+                not __salt__['service.disabled'](service_name):
+            errors.append(
+                "Service {} conflicts with MetalK8s installation, "
+                "please stop and disable it.".format(service_name)
+            )
+
+    error_msg = '\n'.join(errors)
+    if error_msg and raises:
+        raise CheckError(error_msg)
+
+    return error_msg or True
+
+
 def sysctl(params, raises=True):
     """Check if the given sysctl key-values match the ones in memory and
     return a string (or raise if `raises` is set to `True`) with the list

salt/metalk8s/defaults.yaml

@@ -36,6 +36,10 @@ kubeadm_preflight:
       - iproute               # provides tc
       - coreutils             # provides touch
 
+# List of services that conflict with MetalK8s installation
+conflicting_services:
+  - firewalld
+
 repo:
   conflicting_packages:
     # List of package that conflict with MetalK8s installation

salt/metalk8s/orchestrate/deploy_node.sls

@@ -1,3 +1,4 @@
+{%- from "metalk8s/map.jinja" import defaults with context %}
 {%- from "metalk8s/map.jinja" import repo with context %}
 
 {%- set node_name = pillar.orchestrate.node_name %}
@@ -27,12 +28,14 @@ Check node:
     - ssh: true
     - roster: kubernetes
     - kwarg:
-        # NOTE: We need to use the `conflicting_packages` from the salt
-        # master since in salt-ssh when running an execution module we cannot
-        # embbed additional files (especially `map.jinja` in this case)
+        # NOTE: We need to use the `conflicting_packages` and `conflicting_services`
+        # from the salt master since in salt-ssh when running an execution module
+        # we cannot embbed additional files (especially `map.jinja` in this case)
         # Sees: https://github.com/saltstack/salt/issues/59314
         conflicting_packages: >-
           {{ repo.conflicting_packages | tojson }}
+        conflicting_services: >-
+          {{ defaults.conflicting_services | tojson }}
     - failhard: true
     - require:
       - metalk8s: Install python36

salt/tests/unit/modules/files/test_metalk8s_checks.yaml

@@ -1,13 +1,34 @@
 node:
+  # 1. Everything fine
   - packages_ret: True
+    services_ret: True
     result: True
+
+  # 2. Error with packages
   - packages_ret: "Package abcd got an error because of banana"
+    services_ret: True
     expect_raise: True
     result: "Node my_node_1: Package abcd got an error because of banana"
+  # 2. bis (no raises)
   - packages_ret: "Package toto got an error :)"
+    services_ret: True
     raises: False
     result: "Node my_node_1: Package toto got an error :)"
 
+  # 3. Error with services
+  - packages_ret: True
+    services_ret: "Service abcd got an error because of penguin"
+    expect_raise: True
+    result: "Node my_node_1: Service abcd got an error because of penguin"
+
+  # 4. Error with services and packages
+  - packages_ret: "Package abcd got an error because of banana"
+    services_ret: "Service abcd got an error because of penguin"
+    expect_raise: True
+    result: |-
+      Node my_node_1: Package abcd got an error because of banana
+      Service abcd got an error because of penguin
+
 packages:
   # 1. Success: No conflicting packages to check
   - conflicting_packages: {}
@@ -155,6 +176,82 @@ packages:
     result: |-
       Package my-installed-package-4.5.6 conflicts with MetalK8s installation, please remove it.
 
+services:
+  # 1. Success: No conflicting services to check
+  - conflicting_services: []
+    result: True
+
+  # 2. Success: Conflicting services not installed
+  - conflicting_services:
+      - my-not-started-service
+      - my-not-started-service2
+    service_status_ret:
+      my-not-started-service: False
+      my-not-started-service2: False
+    service_disabled_ret:
+      my-not-started-service: True
+      my-not-started-service2: True
+    result: True
+  # 2. bis
+  - conflicting_services: my-not-started-service
+    service_status_ret:
+      my-not-started-service: False
+    service_disabled_ret:
+      my-not-started-service: True
+    result: True
+
+  # 3. Success: Conflicting service (from map) not started
+  - get_map_ret:
+      conflicting_services:
+        - my-not-started-service
+    service_status_ret:
+      my-not-started-service: False
+    service_disabled_ret:
+      my-not-started-service: True
+    result: True
+
+  # 4. Error: Conflicting service started
+  - conflicting_services:
+      - my-started-service
+      - my-not-started-service
+    service_status_ret:
+      my-started-service: True
+      my-not-started-service: False
+    service_disabled_ret:
+      my-started-service: False
+      my-not-started-service: True
+    expect_raise: True
+    result: |-
+      Service my-started-service conflicts with MetalK8s installation, please stop and disable it.
+  # 4. bis
+  - conflicting_services: my-started-service
+    service_status_ret:
+      my-started-service: True
+    service_disabled_ret:
+      my-started-service: True
+    expect_raise: True
+    result: |-
+      Service my-started-service conflicts with MetalK8s installation, please stop and disable it.
+  # 4. ter (no raise)
+  - conflicting_services: my-started-service
+    raises: False
+    service_status_ret:
+      my-started-service: True
+    service_disabled_ret:
+      my-started-service: True
+    result: |-
+      Service my-started-service conflicts with MetalK8s installation, please stop and disable it.
+
+  # 5. Service stopped but still enabled
+  - conflicting_services: my-enabled-service
+    service_status_ret:
+      my-enabled-service: False
+    service_disabled_ret:
+      my-enabled-service: False
+    expect_raise: True
+    result: |-
+      Service my-enabled-service conflicts with MetalK8s installation, please stop and disable it.
+
 sysctl:
   - params:
       net.ipv4.ip_forward: 1

salt/tests/unit/modules/test_metalk8s_checks.py

@@ -33,14 +33,17 @@ def test_virtual(self):
         self.assertEqual(metalk8s_checks.__virtual__(), 'metalk8s_checks')
 
     @utils.parameterized_from_cases(YAML_TESTS_CASES["node"])
-    def test_node(self, packages_ret, result, expect_raise=False, **kwargs):
+    def test_node(self, packages_ret, services_ret, result,
+                  expect_raise=False, **kwargs):
         """
         Tests the return of `node` function
         """
         packages_mock = MagicMock(return_value=packages_ret)
+        services_mock = MagicMock(return_value=services_ret)
 
         salt_dict = {
-            'metalk8s_checks.packages': packages_mock
+            'metalk8s_checks.packages': packages_mock,
+            'metalk8s_checks.services': services_mock
         }
 
         with patch.dict(metalk8s_checks.__grains__, {'id': 'my_node_1'}), \
@@ -86,6 +89,36 @@ def test_packages(self, result, get_map_ret=None, list_pkgs_ret=None,
                     result
                 )
 
+    @utils.parameterized_from_cases(YAML_TESTS_CASES["services"])
+    def test_services(self, result, get_map_ret=None, service_status_ret=None,
+                      service_disabled_ret=None, expect_raise=False, **kwargs):
+        """
+        Tests the return of `services` function
+        """
+        get_map_mock = MagicMock(return_value=get_map_ret)
+        service_status_mock = MagicMock(side_effect=(service_status_ret or {}).get)
+        service_disabled_mock = MagicMock(side_effect=(service_disabled_ret or {}).get)
+
+        salt_dict = {
+            'metalk8s.get_from_map': get_map_mock,
+            'service.status': service_status_mock,
+            'service.disabled': service_disabled_mock
+        }
+
+        with patch.dict(metalk8s_checks.__salt__, salt_dict):
+            if expect_raise:
+                self.assertRaisesRegex(
+                    CheckError,
+                    result,
+                    metalk8s_checks.services,
+                    **kwargs
+                )
+            else:
+                self.assertEqual(
+                    metalk8s_checks.services(**kwargs),
+                    result
+                )
+
     @utils.parameterized_from_cases(YAML_TESTS_CASES["sysctl"])
     def test_sysctl(self, params, data, result, raises=False):
         """