diff --git a/CHANGELOG.md b/CHANGELOG.md index 80a1f136f2..230ac2129f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ configuration file (PR [#3065](https://github.com/scality/metalk8s/pull/3065)) +- [#3067](https://github.com/scality/metalk8s/issues/3067) - Check for conflicting + services already started on the machine before doing all the installation + (PR [#3069](https://github.com/scality/metalk8s/pull/3069)) + ### Bug fixes - [#3022](https://github.com/scality/metalk8s/issues/3022) - Ensure salt-master container can start at reboot even if local salt-minion is down diff --git a/salt/_modules/metalk8s_checks.py b/salt/_modules/metalk8s_checks.py index 85978095f4..bf18b65668 100644 --- a/salt/_modules/metalk8s_checks.py +++ b/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 `` which is the name of the conflicting service + - a list `[, ]` 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 diff --git a/salt/metalk8s/defaults.yaml b/salt/metalk8s/defaults.yaml index 6e42f157f4..1c2ff73fac 100644 --- a/salt/metalk8s/defaults.yaml +++ b/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 diff --git a/salt/metalk8s/orchestrate/deploy_node.sls b/salt/metalk8s/orchestrate/deploy_node.sls index 45024e511e..8d5d8763c1 100644 --- a/salt/metalk8s/orchestrate/deploy_node.sls +++ b/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 %} @@ -26,12 +27,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 diff --git a/salt/tests/unit/modules/files/test_metalk8s_checks.yaml b/salt/tests/unit/modules/files/test_metalk8s_checks.yaml index 9cfcb628fe..ae2ddb94a3 100644 --- a/salt/tests/unit/modules/files/test_metalk8s_checks.yaml +++ b/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 diff --git a/salt/tests/unit/modules/test_metalk8s_checks.py b/salt/tests/unit/modules/test_metalk8s_checks.py index 695510c611..b2b84fb3c7 100644 --- a/salt/tests/unit/modules/test_metalk8s_checks.py +++ b/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): """