Prevent downgrade when not possible #2858

alexandre-allard · 2020-10-14T13:14:02Z

Component: salt, scripts, downgrade

Context: See #2852 :)

Summary:

Acceptance criteria:

Closes: #2852

bert-e · 2020-10-14T13:14:04Z

Hello alexandre-allard-scality,

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-14T13:14:12Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/2852-prevent-downgrade-when-not-possible into
development/2.6
w/2.7/improvement/2852-prevent-downgrade-when-not-possible will be merged into 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

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2020-10-14T13:14:14Z

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:

alexandre-allard · 2020-10-14T13:15:22Z

salt/metalk8s/orchestrate/downgrade/precheck.sls

@@ -16,6 +18,13 @@ Correct saltenv "{{ saltenv }}" for downgrade to "{{ dest_version }}":

 {%- endif %}

+{%- if not defaults.downgrade_supported and not pillar.force_downgrade | default(False) %}


Maybe we want this pillar entry below the metalk8s one ? pillar.metalk8s.force_downgrade or even something like pillar.metalk8s.downgrade.force ?
Same for defaults, may be downgrade.supported, but I'm not sure the downgrade dict would be useful for anything else in the future.

I'd put everything under metalk8s.downgrade, either using both .supported and .force, or with a single key, maybe naming it .enabled?

(No strong opinion about the place for force_downgrade but maybe since it's only a pillar key that will be passed to orchestrate I would say to be consistent metalk8s.orchestrate.force_downgrade (or metalk8s.orchestrate.downgrade.force as you want))
Also here the check is wrong I think, because we can downgrade from 2.6.1 to 2.6.0 for example. IMO you should add a check about the destination version here

salt/metalk8s/defaults.yaml

gdemonet · 2020-10-14T14:36:50Z

salt/metalk8s/orchestrate/downgrade/precheck.sls

@@ -16,6 +18,13 @@ Correct saltenv "{{ saltenv }}" for downgrade to "{{ dest_version }}":

 {%- endif %}

+{%- if not defaults.downgrade_supported and not pillar.force_downgrade | default(False) %}


I'd put everything under metalk8s.downgrade, either using both .supported and .force, or with a single key, maybe naming it .enabled?

salt/metalk8s/orchestrate/downgrade/precheck.sls

TeddyAndrieux · 2020-10-14T14:45:50Z

salt/metalk8s/orchestrate/downgrade/precheck.sls

@@ -16,6 +18,13 @@ Correct saltenv "{{ saltenv }}" for downgrade to "{{ dest_version }}":

 {%- endif %}

+{%- if not defaults.downgrade_supported and not pillar.force_downgrade | default(False) %}


(No strong opinion about the place for force_downgrade but maybe since it's only a pillar key that will be passed to orchestrate I would say to be consistent metalk8s.orchestrate.force_downgrade (or metalk8s.orchestrate.downgrade.force as you want))
Also here the check is wrong I think, because we can downgrade from 2.6.1 to 2.6.0 for example. IMO you should add a check about the destination version here

TeddyAndrieux · 2020-10-14T14:48:40Z

salt/metalk8s/defaults.yaml

+# Define if the downgrade from this version is supported.
+# It should be set to false if manual operations are needed
+# (e.g. downgrade of etcd), prior to downgrading the cluster.
+# The downgrade can still be forced setting force_downgrade to True
+# in the pillar.
+downgrade_supported: false


Do we really want a pillar key to say downgrade is supported ? Because it's supported from 2.6.1 to 2.6.0 for example but will never be supported from 2.6.x to 2.5.x ... so to me we should just have a check about the version in the precheck

TeddyAndrieux · 2020-10-14T14:53:07Z

salt/metalk8s/orchestrate/downgrade/precheck.sls

+{%- if not defaults.downgrade_supported and not pillar.force_downgrade | default(False) %}
+
+Downgrade from current version is not supported:
+  test.fail_without_changes


To be clear I would add a clear comment here something like "etcd downgrade not supported blablabla" + Maybe a link to a github issue for this #1750 (maybe we want to do some cleaning in the ticket (and talk about 2.6 instead of non-released 2.3) but no related to this PR)

I'd rather link to 2.6.0 release notes, where we will explain this limitation, but agreed, additional help would be very useful for users.

bert-e · 2020-10-22T10:26:39Z

History mismatch

Merge commit #a3d20c5c5aa3cb729e2af0ae64ea6b3e32c5a06a on the integration branch
w/2.7/improvement/2852-prevent-downgrade-when-not-possible is merging a branch which is neither the current
branch improvement/2852-prevent-downgrade-when-not-possible nor the development branch
development/2.7.

It is likely due to a rebase of the branch improvement/2852-prevent-downgrade-when-not-possible and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

alexandre-allard · 2020-10-23T10:23:53Z

/reset

bert-e · 2020-10-23T10:24:01Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2020-10-23T10:24:10Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/2852-prevent-downgrade-when-not-possible into
development/2.6
w/2.7/improvement/2852-prevent-downgrade-when-not-possible will be merged into 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

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2020-10-23T10:24:11Z

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:

bert-e · 2020-10-23T11:47:32Z

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

LGTM, just a small comment for the message to display in case the downgrade isn't supported.

salt/metalk8s/orchestrate/downgrade/precheck.sls

bert-e · 2020-10-23T14:46:11Z

Branches have diverged

This pull request's source branch improvement/2852-prevent-downgrade-when-not-possible has diverged from
development/2.6 by more than 50 commits.

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

Merge origin/development/2.6 into improvement/2852-prevent-downgrade-when-not-possible
Rebase improvement/2852-prevent-downgrade-when-not-possible onto origin/development/2.6

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

Refs: #2852

We now set a boolean in `defaults.yaml` telling if we support downgrade from a version or not. It should be set to false if manual operations are needed (e.g. downgrade of etcd), prior to downgrading the cluster The cluster can still be force setting `metalk8s.downgrade.enabled` to `True` in the pillar. The check will fail early before anything got upgraded to avoid having an half upgraded cluster situation. Refs: #2852

This option is used to by-pass the precheck in downgrade orchestrate, preventing a user from downgrading for unsupported path. This is here in case one still want to downgrade its cluster, for example, if he already has done some manual operations to ensure the downgrade will work (e.g. manual downgrade of etcd). Refs: #2852

bert-e · 2020-10-26T10:41:57Z

History mismatch

Merge commit #d17744acf01c7b674bd0e966f6a25fea51e72a42 on the integration branch
w/2.7/improvement/2852-prevent-downgrade-when-not-possible is merging a branch which is neither the current
branch improvement/2852-prevent-downgrade-when-not-possible nor the development branch
development/2.7.

It is likely due to a rebase of the branch improvement/2852-prevent-downgrade-when-not-possible and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

alexandre-allard · 2020-10-26T10:42:36Z

/reset

bert-e · 2020-10-26T10:42:43Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2020-10-26T10:42:51Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge improvement/2852-prevent-downgrade-when-not-possible into
development/2.6
w/2.7/improvement/2852-prevent-downgrade-when-not-possible will be merged into 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

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2020-10-26T10:42:52Z

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

bert-e · 2020-10-26T11:58:11Z

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

bert-e · 2020-10-27T08:51:59Z

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:

alexandre-allard · 2020-10-27T09:45:17Z

/approve

bert-e · 2020-10-27T09:45:23Z

Build failed

The build for commit did not succeed in branch w/2.7/improvement/2852-prevent-downgrade-when-not-possible.

The following options are set: approve

TeddyAndrieux

Not tested but LGTM

bert-e · 2020-10-28T14:02:53Z

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.6
✔️ 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

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-10-28T16:02:51Z

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

✔️ development/2.6
✔️ 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

Please check the status of the associated issue None.

Goodbye alexandre-allard-scality.

alexandre-allard requested a review from a team as a code owner October 14, 2020 13:14

alexandre-allard changed the title ~~Improvement/2852 prevent downgrade when not possible~~ Prevent downgrade when not possible Oct 14, 2020

alexandre-allard commented Oct 14, 2020

View reviewed changes

gdemonet reviewed Oct 14, 2020

View reviewed changes

TeddyAndrieux reviewed Oct 14, 2020

View reviewed changes

alexandre-allard force-pushed the improvement/2852-prevent-downgrade-when-not-possible branch from be085bd to 4c32721 Compare October 22, 2020 10:26

alexandre-allard force-pushed the improvement/2852-prevent-downgrade-when-not-possible branch 2 times, most recently from 4a71f07 to 126332f Compare October 22, 2020 10:30

gdemonet suggested changes Oct 23, 2020

View reviewed changes

salt/metalk8s/orchestrate/downgrade/precheck.sls Outdated Show resolved Hide resolved

alexandre-allard added 3 commits October 26, 2020 11:41

salt: ensure no step is run in downgrade if prechecks fail

8c0b95e

Refs: #2852

alexandre-allard force-pushed the improvement/2852-prevent-downgrade-when-not-possible branch from 126332f to 292705b Compare October 26, 2020 10:41

alexandre-allard requested a review from gdemonet October 26, 2020 14:01

gdemonet approved these changes Oct 27, 2020

View reviewed changes

TeddyAndrieux approved these changes Oct 27, 2020

View reviewed changes

bert-e merged commit 211dd07 into development/2.6 Oct 28, 2020

bert-e deleted the improvement/2852-prevent-downgrade-when-not-possible branch October 28, 2020 16:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent downgrade when not possible #2858

Prevent downgrade when not possible #2858

alexandre-allard commented Oct 14, 2020

bert-e commented Oct 14, 2020

bert-e commented Oct 14, 2020

bert-e commented Oct 14, 2020

alexandre-allard Oct 14, 2020 •

edited

Loading

gdemonet Oct 14, 2020

TeddyAndrieux Oct 14, 2020

gdemonet Oct 14, 2020

TeddyAndrieux Oct 14, 2020

TeddyAndrieux Oct 14, 2020

gdemonet Oct 14, 2020

TeddyAndrieux Oct 14, 2020

gdemonet Oct 14, 2020

bert-e commented Oct 22, 2020

alexandre-allard commented Oct 23, 2020

bert-e commented Oct 23, 2020

bert-e commented Oct 23, 2020

bert-e commented Oct 23, 2020

bert-e commented Oct 23, 2020

gdemonet left a comment

bert-e commented Oct 23, 2020

bert-e commented Oct 26, 2020

alexandre-allard commented Oct 26, 2020

bert-e commented Oct 26, 2020

bert-e commented Oct 26, 2020

bert-e commented Oct 26, 2020

bert-e commented Oct 26, 2020

bert-e commented Oct 27, 2020

alexandre-allard commented Oct 27, 2020

bert-e commented Oct 27, 2020

TeddyAndrieux left a comment

bert-e commented Oct 28, 2020

bert-e commented Oct 28, 2020

		@@ -16,6 +18,13 @@ Correct saltenv "{{ saltenv }}" for downgrade to "{{ dest_version }}":

		{%- endif %}

		{%- if not defaults.downgrade_supported and not pillar.force_downgrade \| default(False) %}

Prevent downgrade when not possible #2858

Prevent downgrade when not possible #2858

Conversation

alexandre-allard commented Oct 14, 2020

bert-e commented Oct 14, 2020

Hello alexandre-allard-scality,

bert-e commented Oct 14, 2020

Integration data created

bert-e commented Oct 14, 2020

Waiting for approval

alexandre-allard Oct 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bert-e commented Oct 22, 2020

History mismatch

alexandre-allard commented Oct 23, 2020

bert-e commented Oct 23, 2020

Reset complete

bert-e commented Oct 23, 2020

Integration data created

bert-e commented Oct 23, 2020

Waiting for approval

bert-e commented Oct 23, 2020

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Oct 23, 2020

Branches have diverged

bert-e commented Oct 26, 2020

History mismatch

alexandre-allard commented Oct 26, 2020

bert-e commented Oct 26, 2020

Reset complete

bert-e commented Oct 26, 2020

Integration data created

bert-e commented Oct 26, 2020

Waiting for approval

bert-e commented Oct 26, 2020

Waiting for approval

bert-e commented Oct 27, 2020

Waiting for approval

alexandre-allard commented Oct 27, 2020

bert-e commented Oct 27, 2020

Build failed

TeddyAndrieux left a comment

Choose a reason for hiding this comment

bert-e commented Oct 28, 2020

In the queue

bert-e commented Oct 28, 2020

alexandre-allard Oct 14, 2020 •

edited

Loading