Migration to Python3 #2839

TeddyAndrieux · 2020-10-09T14:58:00Z

Component:

'salt', 'build'

Context:

#2203

Summary:

Install salt-master and salt-minion Python3 versions
Make salt unit tests working in Python3
Bump salt to 3002.2
Add an "ugly" orchestrate to migrate Salt minion
Remove non-breaking space characters from Solutions manifests

Acceptance criteria:

Fixes: #2203

bert-e · 2020-10-09T14:58:03Z

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 · 2020-10-09T14:58:07Z

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:

slaperche-scality · 2020-10-09T15:26:44Z

tests/install/steps/test_expansion.py

@@ -37,16 +37,12 @@ def declare_node(

 @when(parsers.parse('we deploy the node "{node_name}"'))
 def deploy_node(host, ssh_config, version, node_name):
-    accept_ssh_key = [


Seems unrelated to this PR.

And if this is really useless, then the doc needs an update as well: https://metal-k8s.readthedocs.io/en/development-2.6/installation/expansion.html#id3

So maybe this should be done in a dedicated PR (and keep this one only about py3 migration)

Yes you right, for the doc I think it's still good to have it as a pre-check for expansion.

Edit: Oh it will not work anyway in the doc since we need python3 installed to have a working salt-ssh, ok I will update the doc

I updated the documentation but I think I will keep it in this PR since it's changes needed for Python3 migration even if it's not directly related

Maybe you could replace this command with the one suggested in docs (using --raw-shell)? I think we added it here to make sure the test would fail early if there was any issue with the key, and also because it's good practice to do in tests exactly what we have in docs.

For me this one was here just because it was needed at the beginning to accept the ssh public key of the new node.
Anyway right I can add this check here like in the doc

salt/_modules/metalk8s.py

bert-e · 2020-10-13T15:46:36Z

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 · 2020-10-13T15:46:59Z

/after_pull_request 2850

bert-e · 2020-10-30T07:53:30Z

Branches have diverged

This pull request's source branch improvement/migrate-python-3 has diverged from
development/2.7 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

Merge origin/development/2.7 into improvement/migrate-python-3
Rebase improvement/migrate-python-3 onto origin/development/2.7

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

bert-e · 2020-10-30T08:00:48Z

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:

docs/installation/expansion.rst

salt/metalk8s/orchestrate/deploy_node.sls

gdemonet · 2020-11-05T08:54:42Z

tests/install/steps/test_expansion.py

@@ -37,16 +37,12 @@ def declare_node(

 @when(parsers.parse('we deploy the node "{node_name}"'))
 def deploy_node(host, ssh_config, version, node_name):
-    accept_ssh_key = [


Maybe you could replace this command with the one suggested in docs (using --raw-shell)? I think we added it here to make sure the test would fail early if there was any issue with the key, and also because it's good practice to do in tests exactly what we have in docs.

gdemonet

Only really minor comments, otherwise LGTM! Will manually test as well

salt/metalk8s/salt/minion/dependencies.sls

salt/metalk8s/orchestrate/migrate_salt.sls

TeddyAndrieux · 2020-11-27T11:34:43Z

/approve

bert-e · 2020-11-27T11:34:48Z

Conflict with a changeset in the queue

The changeset in this pull request conflicts with another changeset
already in the queue. Please wait for the current queue to merge into
the development branch. The conflict will then appear in this pull
request and can be sorted on the feature branch directly.

This changeset has not been added to the queue.

The following options are set: approve

With Python3 we can no longer use `cmp` in `sort` and `sorted` function so use our salt module `metalk8s.cmp_sorted` in Downgrade orchestrate if we are running with Python3, so that it works well with Python2 and Pyton3 Refs: #2203

In the past a `salt-ssh` command was needed prior to start the expansions to accept the ssh key but it's not longer needed so remove it. Instead do a simple `salt-ssh --raw-shell` command to check that salt-ssh work properly

Right now `salt.function` salt state module do not handle `roster` for ssh and `raw_shell`, create a custom salt state module to run a `saltutil.cmd` with all kwargs input so that we support `roster` and `raw_shell` Sees: saltstack/salt#58662

Change salt-master image to use Python3 salt-master package and also install Python3 dependencies, embed Python3 salt-minion version instead of Python2. In order to install Python3 salt-minion we need: - to install python3 - to install python36-rpm as by default version comparaison for package installation is wrong - to install python3 on nodes for being able to use `salt-ssh` Sees: saltstack/salt#58039 Sees: saltstack/salt#57972 Fixes: #2203

- removes `salttesting` dependency - migrates away from deprecated `TestCase` methods - migrates `Mock` objects call args checks

We need to pass `\;` to the `isoinfo` binary for reading file contents from an ISO archive, but this escape sequence is not valid in Python, and Python 3 warns about it. We could have escaped the backslash, but instead rely on the raw string notation for Python 3 (`r'my \escaped sequence'`).

Since we migrate from Python2 to Python3 a "classic" salt states cannot handle it properly as the Python version change during the state execution. Add a dedicated orchestrate that handle the migration from Salt Python2 to Salt Python3 and also the migration from Salt Python3 to Salt Python2, call this new orchestrate during Upgrade and Downgrade if needed Refs: #2203

Since we migrate to Salt Python3 we have Jinja 2.11 so we can use `loop.previtem` instead of ugly hack using a jinja variable. NOTE: This hack is still needed in downgrade orchestrate when we downgrade to `2.6.x` as we are running with Salt master metalk8s-2.6.x that use Salt Python2 and Jinja 2.7 Refs: #2203

In `deploy_node` orchestrate, before this commit, during upgrade and downgrade the `Install etcd node` state is called even if the Salt upgrade/downgrade failed. This commit just add a require on this etcd state

Solutions YAML files contains some non-breaking space characters inside some Jinja formula `{{ }}` that make Salt Python3 rendering fail. NOTE: Those non-breaking space characters did not break Jinja2.7 used by Salt in Python2 that's why we didn't see any issue because of this before

Add an entry in changelog file about Salt migration to Python3 and bump to Salt version 3002.2 Refs: #2203

See: #2726 See: #2936

bert-e · 2020-11-27T13:39: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:

The following options are set: approve

bert-e · 2020-11-27T14:55: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.7

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

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 · 2020-11-27T14:55:12Z

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

✔️ development/2.7

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

Please check the status of the associated issue None.

Goodbye teddyandrieux.

TeddyAndrieux requested a review from a team October 9, 2020 14:58

slaperche-scality reviewed Oct 9, 2020

View reviewed changes

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

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch 4 times, most recently from 05ac2c6 to 3c443e0 Compare October 13, 2020 15:43

scality deleted a comment from bert-e Oct 13, 2020

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch from 3c443e0 to db25db6 Compare October 30, 2020 07:53

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch from db25db6 to 1338ae2 Compare October 30, 2020 08:00

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch 5 times, most recently from 928b1ea to e220bc6 Compare November 3, 2020 15:33

TeddyAndrieux marked this pull request as ready for review November 3, 2020 15:34

gdemonet reviewed Nov 5, 2020

View reviewed changes

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch 2 times, most recently from 1f7e4dc to ed2482f Compare November 5, 2020 11:11

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch 5 times, most recently from 280492d to 1ca977e Compare November 17, 2020 14:40

gdemonet previously approved these changes Nov 26, 2020

View reviewed changes

salt/metalk8s/salt/minion/dependencies.sls Outdated Show resolved Hide resolved

salt/metalk8s/orchestrate/migrate_salt.sls Outdated Show resolved Hide resolved

TeddyAndrieux dismissed gdemonet’s stale review via 21327b5 November 26, 2020 14:09

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch 2 times, most recently from 21327b5 to f08fd55 Compare November 27, 2020 09:29

gdemonet previously approved these changes Nov 27, 2020

View reviewed changes

TeddyAndrieux dismissed gdemonet’s stale review via e42a510 November 27, 2020 10:07

gdemonet previously approved these changes Nov 27, 2020

View reviewed changes

TeddyAndrieux and others added 13 commits November 27, 2020 14:37

salt: In downgrade orchestrate use metalk8s.cmp_sorted

7655d6f

With Python3 we can no longer use `cmp` in `sort` and `sorted` function so use our salt module `metalk8s.cmp_sorted` in Downgrade orchestrate if we are running with Python3, so that it works well with Python2 and Pyton3 Refs: #2203

salt/unit-test: Migrate to Python 3

a7b51ea

- removes `salttesting` dependency - migrates away from deprecated `TestCase` methods - migrates `Mock` objects call args checks

build,tests: Bump salt to 3002.2

cf5df11

salt: Do not call etcd state if Salt upgrade/downgrade failed

334a7f5

In `deploy_node` orchestrate, before this commit, during upgrade and downgrade the `Install etcd node` state is called even if the Salt upgrade/downgrade failed. This commit just add a require on this etcd state

changelog: Add entry for Python3 migration

ea22895

Add an entry in changelog file about Salt migration to Python3 and bump to Salt version 3002.2 Refs: #2203

salt: Migrate sparse volume cleanup to Python3

0d1cb84

See: #2726 See: #2936

TeddyAndrieux dismissed gdemonet’s stale review via 0d1cb84 November 27, 2020 13:39

TeddyAndrieux force-pushed the improvement/migrate-python-3 branch from e42a510 to 0d1cb84 Compare November 27, 2020 13:39

gdemonet approved these changes Nov 27, 2020

View reviewed changes

bert-e merged commit 0d1cb84 into development/2.7 Nov 27, 2020

bert-e deleted the improvement/migrate-python-3 branch November 27, 2020 14:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migration to Python3 #2839

Migration to Python3 #2839

TeddyAndrieux commented Oct 9, 2020 •

edited

Loading

bert-e commented Oct 9, 2020

bert-e commented Oct 9, 2020

slaperche-scality Oct 9, 2020

TeddyAndrieux Oct 9, 2020 •

edited

Loading

TeddyAndrieux Oct 12, 2020

gdemonet Nov 5, 2020

TeddyAndrieux Nov 5, 2020

bert-e commented Oct 13, 2020

TeddyAndrieux commented Oct 13, 2020

bert-e commented Oct 30, 2020

bert-e commented Oct 30, 2020

gdemonet Nov 5, 2020

gdemonet left a comment

TeddyAndrieux commented Nov 27, 2020

bert-e commented Nov 27, 2020

bert-e commented Nov 27, 2020

bert-e commented Nov 27, 2020

bert-e commented Nov 27, 2020

Migration to Python3 #2839

Migration to Python3 #2839

Conversation

TeddyAndrieux commented Oct 9, 2020 • edited Loading

bert-e commented Oct 9, 2020

Hello teddyandrieux,

bert-e commented Oct 9, 2020

Waiting for approval

slaperche-scality Oct 9, 2020

Choose a reason for hiding this comment

TeddyAndrieux Oct 9, 2020 • edited Loading

Choose a reason for hiding this comment

TeddyAndrieux Oct 12, 2020

Choose a reason for hiding this comment

gdemonet Nov 5, 2020

Choose a reason for hiding this comment

TeddyAndrieux Nov 5, 2020

Choose a reason for hiding this comment

bert-e commented Oct 13, 2020

Waiting for approval

TeddyAndrieux commented Oct 13, 2020

bert-e commented Oct 30, 2020

Branches have diverged

bert-e commented Oct 30, 2020

Waiting for approval

gdemonet Nov 5, 2020

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

TeddyAndrieux commented Nov 27, 2020

bert-e commented Nov 27, 2020

Conflict with a changeset in the queue

bert-e commented Nov 27, 2020

Waiting for approval

bert-e commented Nov 27, 2020

In the queue

bert-e commented Nov 27, 2020

TeddyAndrieux commented Oct 9, 2020 •

edited

Loading

TeddyAndrieux Oct 9, 2020 •

edited

Loading