Properly fail if a conflicting package is installed and do not install conflicting packages #3050

TeddyAndrieux · 2021-01-18T14:54:38Z

Component:

'salt', 'check', 'deployment'

Context:

#2992

Summary:

Add an execution module to retrieve value from map.jinja
Check for conflicting package installed during bootstrap and expansion
Exclude conflicting packages when installing package in salt (e.g: we do not install containerd.io instead of containerd and runc) (NOTE that before this fix when running containerd SLS if containerd.io is available we install this one instead of runc and containerd)

Acceptance criteria:

Bootstrap output when docker-ce installed on the host

# /srv/scality/metalk8s-2.8.0-dev/bootstrap.sh 
> Determine the OS... done [0s]
> Checking that BootstrapConfiguration is present... done [0s]
> Pre-minion system tests... done [0s]
> Configure internal repositories... done [1s]
> Check mandatory packages presence... done [4s]
> Disabling Salt minion service... done [0s]
> Stopping Salt minion service... done [0s]
> Installing mandatory packages... done [11s]
> Configuring Salt minion to run in local mode... done [6s]
> Ensure archive is available... done [2s]
> Checking local node... fail [2s]

Failure while running step 'Checking local node'

Command: check_local_node

Output:

<< BEGIN >>
Error running 'metalk8s_checks.node': Node bootstrap: Package containerd.io-1.4.3 conflicts with MetalK8s installation, please remove it.
Package docker-ce-20.10.2 conflicts with MetalK8s installation, please remove it.
<< END >>

This script will now exit

Expansion output when docker-ce installed on this new node

# crictl exec -it 8cf89df3f31ef salt-run state.sls metalk8s.orchestrate.deploy_node saltenv=metalk8s-2.8.0-dev pillar="{'orchestrate': {'node_name': 'node-1'}}"
[...]
bootstrap_master:
[...]
----------
          ID: Check node
    Function: metalk8s.saltutil_cmd
        Name: metalk8s_checks.node
      Result: False
     Comment: Running function metalk8s_checks.node failed on minions: node-1
     Started: 14:45:26.883648
    Duration: 3345.072 ms
     Changes:   
              ----------
              node-1:
                  ----------
                  retcode:
                      1
                  stderr:
                      Error running 'metalk8s_checks.node': Node node-1: Package containerd.io-1.4.3 conflicts with MetalK8s installation, please remove it.
                      Package docker-ce-20.10.2 conflicts with MetalK8s installation, please remove it.
                  stdout:

Summary for bootstrap_master
------------
Succeeded: 1 (changed=2)
Failed:    1
------------
Total states run:     2
Total run time:   5.698 s

Fixes: #2992

bert-e · 2021-01-18T14:54:40Z

Hello teddyandrieux,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2021-01-18T14:54:44Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

gdemonet

Just one typo in the user-facing message, the rest is more minor/internal. Otherwise LGTM 👌 !

salt/_modules/metalk8s.py

salt/_modules/metalk8s_checks.py

bert-e · 2021-01-18T17:10:25Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

NicolasT · 2021-01-18T19:33:16Z

salt/metalk8s/defaults.yaml

+    docker: null
+    docker-ce: null
+    containerd.io: null
+


Could you take a look at the list provided by @Baitanik in #2989 (reply in thread) and check whether this list should be extended accordingly?

Yes I could try with those packages, but for pip packages, it's a bit hard to check IMHO.
E.g.: Checking all dependencies of Salt installed by pip and not by system package... It's doable but not sure it's worth and honestly deserve another PR since it's really a different check to me

In salt we have a function `jinja.load_map` that retrieve a specific value from a map.jinja (same as what we do in salt SLS) but this function does not support saltenv Add a new function to retrieve information from `map.jinja` in MetalK8s context, so hardcoded map.jinja path and retrieving saltenv from version stored in the pillar Sees: saltstack/salt#59300

If some package are installed on the host where we want to deploy MetalK8s the installation does not work (e.g: containerd.io) Add a new function to check that those package are not installed on the host before deploying all the MetalK8s components. NOTE: We do not automatically uninstall the package from the host since those packages may have been installed for good reason, so just ask the user to remove those packages Fixes: #2992

Another commit already added the check for conflicting package installed on the host but we also need to exclude those packages especially for `containerd.io` because if you try to install `containerd` or `runc` it will, by default` install `containerd.io` since this one provide `containerd` and `runc` and obsoletes thoses ones. Today MetalK8s cannot work with `containerd.io` so we need to explicitly exclude this package in case one repository on the host is already configured and provide this `containerd.io` package

gdemonet

Approving for these packages, I guess we can easily add some more (as the ones mentioned in #2989 (reply in thread)) later.

gdemonet · 2021-01-19T15:10:39Z

salt/_modules/metalk8s_checks.py

+    Note: We have some logic in this function, so `conflicting_packages` could
+    be:
+    - a single string `<package_name>` which is the name of the conflicting
+      package (it means we conflict with all versions of this package)
+    - a list `[<package_name1>, <package_name2]` with all conflicting package
+      names (it means we conflict with all versions of those packages)
+    - a dict `{<package1_name>: <package1_versions>, <package2_name>: <package2_versions>}`
+      where `package_versions` could be
+      - `None` mean we conflicting with all versions of this package
+      - A string for a single conflicting version of this package
+      - A list of string for multiple conflicting versions of this package


bert-e · 2021-01-19T15:13:24Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

TeddyAndrieux · 2021-01-19T15:49:56Z

/approve

bert-e · 2021-01-19T15:50:06Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.8

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2021-01-19T15:50:13Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.8

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

Please check the status of the associated issue None.

Goodbye teddyandrieux.

TeddyAndrieux requested a review from a team January 18, 2021 14:54

TeddyAndrieux force-pushed the improvement/add-check-before-install branch from 6bdae4c to 6660693 Compare January 18, 2021 14:56

gdemonet suggested changes Jan 18, 2021

View reviewed changes

salt/_modules/metalk8s.py Show resolved Hide resolved

salt/_modules/metalk8s_checks.py Outdated Show resolved Hide resolved

salt/_modules/metalk8s_checks.py Show resolved Hide resolved

salt/_modules/metalk8s_checks.py Outdated Show resolved Hide resolved

NicolasT reviewed Jan 18, 2021

View reviewed changes

TeddyAndrieux added 3 commits January 19, 2021 09:27

TeddyAndrieux force-pushed the improvement/add-check-before-install branch from 6660693 to 1ea12d0 Compare January 19, 2021 08:27

gdemonet approved these changes Jan 19, 2021

View reviewed changes

bert-e merged commit 1ea12d0 into development/2.8 Jan 19, 2021

bert-e deleted the improvement/add-check-before-install branch January 19, 2021 15:50

gdemonet mentioned this pull request Feb 2, 2021

salt: Check Service CIDR has a route declared #3076

Merged

gdemonet mentioned this pull request Feb 26, 2021

Backport multiple pre-checks in 2.7 #3153

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Properly fail if a conflicting package is installed and do not install conflicting packages #3050

Properly fail if a conflicting package is installed and do not install conflicting packages #3050

TeddyAndrieux commented Jan 18, 2021 •

edited

Loading

bert-e commented Jan 18, 2021

bert-e commented Jan 18, 2021

gdemonet left a comment

bert-e commented Jan 18, 2021

NicolasT Jan 18, 2021

TeddyAndrieux Jan 18, 2021

gdemonet left a comment

gdemonet Jan 19, 2021

bert-e commented Jan 19, 2021

TeddyAndrieux commented Jan 19, 2021

bert-e commented Jan 19, 2021

bert-e commented Jan 19, 2021

Properly fail if a conflicting package is installed and do not install conflicting packages #3050

Properly fail if a conflicting package is installed and do not install conflicting packages #3050

Conversation

TeddyAndrieux commented Jan 18, 2021 • edited Loading

bert-e commented Jan 18, 2021

Hello teddyandrieux,

bert-e commented Jan 18, 2021

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Jan 18, 2021

Waiting for approval

NicolasT Jan 18, 2021

Choose a reason for hiding this comment

TeddyAndrieux Jan 18, 2021

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

gdemonet Jan 19, 2021

Choose a reason for hiding this comment

bert-e commented Jan 19, 2021

Waiting for approval

TeddyAndrieux commented Jan 19, 2021

bert-e commented Jan 19, 2021

In the queue

bert-e commented Jan 19, 2021

TeddyAndrieux commented Jan 18, 2021 •

edited

Loading