salt: fix unsafe approach for etcd `initial-cluster-state #2198

Ebaneck · 2020-01-21T08:57:53Z

Component:

'salt', 'bootstrap'

Context:

Summary:

This PR ensures that we set the etcd initial cluster state to existing by default and then override this value only during a Bootstrap orchestration.

We also ensure to raise an error when the etcd member list is empty because If all nodes are unavailable we should never set the cluster state to new

Acceptance criteria:

Etcd deployment should be resilient.

Closes: #2102

bert-e · 2020-01-21T08:57:57Z

Hello ebaneck,

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-01-21T08:58:04Z

Integration data created

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

this pull request will merge bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state into
development/2.4
w/2.5/bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state will be merged into development/2.5

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

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-01-21T08:58:06Z

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

Logic in etcd.installed is good, but I'm not sure about this pillar value... WDYT @TeddyAndrieux @NicolasT ?

salt/metalk8s/defaults.yaml

salt/metalk8s/kubernetes/etcd/installed.sls

bert-e · 2020-01-21T11:03:12Z

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-01-21T14:55:21Z

History mismatch

Merge commit #90c55ff8583592d1187dd0c7359113122d7cb7bf on the integration branch
w/2.5/bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state is merging a branch which is neither the current
branch bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state nor the development branch
development/2.5.

It is likely due to a rebase of the branch bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state 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.

salt/metalk8s/kubernetes/etcd/installed.sls

TeddyAndrieux

LGTM

gdemonet

All good!

TeddyAndrieux

Build fail

salt/metalk8s/kubernetes/etcd/installed.sls

Initially, we set etcd cluster state to new when the member list from the pillars is empty. This approach is not optimal since if for what ever reason we are not able to retrieve the list. Now, we raise an error when setting the cluster state to `existing` if we are unable to obtain a list of members. Also, we only set to `new` state only during a bootstrap.

Ebaneck · 2020-01-22T07:37:04Z

/approve

bert-e · 2020-01-22T07:37:09Z

History mismatch

Merge commit #90c55ff8583592d1187dd0c7359113122d7cb7bf on the integration branch
w/2.5/bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state is merging a branch which is neither the current
branch bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state nor the development branch
development/2.5.

It is likely due to a rebase of the branch bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state 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.

The following options are set: approve

Ebaneck · 2020-01-22T07:37:13Z

/reset

bert-e · 2020-01-22T07:37:21Z

Reset complete

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

The following options are set: approve

bert-e · 2020-01-22T07:37:29Z

Integration data created

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

this pull request will merge bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state into
development/2.4
w/2.5/bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state will be merged into development/2.5

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

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

The following options are set: approve

bert-e · 2020-01-22T07:37:31Z

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:

@TeddyAndrieux

The following options are set: approve

stale

bert-e · 2020-01-22T08:53:24Z

Build failed

The build for commit did not succeed in branch w/2.5/bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state.

The following options are set: approve

bert-e · 2020-01-22T14:43:03Z

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.4
✔️ development/2.5

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

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-01-23T08:46:38Z

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

✔️ development/2.4
✔️ development/2.5

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

Please check the status of the associated issue None.

Goodbye ebaneck.

Ebaneck requested a review from a team January 21, 2020 08:57

gdemonet suggested changes Jan 21, 2020

View reviewed changes

Ebaneck force-pushed the bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state branch from 90c55ff to ef8d1a1 Compare January 21, 2020 14:55

Ebaneck requested a review from gdemonet January 21, 2020 15:15

TeddyAndrieux reviewed Jan 21, 2020

View reviewed changes

salt/metalk8s/kubernetes/etcd/installed.sls Outdated Show resolved Hide resolved

Ebaneck force-pushed the bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state branch 2 times, most recently from 03d024a to bdbb742 Compare January 21, 2020 16:17

TeddyAndrieux reviewed Jan 21, 2020

View reviewed changes

salt/metalk8s/kubernetes/etcd/installed.sls Outdated Show resolved Hide resolved

Ebaneck force-pushed the bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state branch from bdbb742 to 9e0eb56 Compare January 21, 2020 17:05

TeddyAndrieux approved these changes Jan 21, 2020

View reviewed changes

gdemonet previously approved these changes Jan 21, 2020

View reviewed changes

TeddyAndrieux previously requested changes Jan 21, 2020

View reviewed changes

salt/metalk8s/kubernetes/etcd/installed.sls Outdated Show resolved Hide resolved

Ebaneck dismissed gdemonet’s stale review via ac5a990 January 21, 2020 18:26

Ebaneck force-pushed the bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state branch from 9e0eb56 to ac5a990 Compare January 21, 2020 18:26

gdemonet approved these changes Jan 21, 2020

View reviewed changes

bert-e merged commit ac5a990 into development/2.4 Jan 23, 2020

bert-e deleted the bugfix/2102-fix-unsafe-approach-for-etcd-initial-cluster-state branch January 23, 2020 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

salt: fix unsafe approach for etcd `initial-cluster-state #2198

salt: fix unsafe approach for etcd `initial-cluster-state #2198

Ebaneck commented Jan 21, 2020

bert-e commented Jan 21, 2020

bert-e commented Jan 21, 2020

bert-e commented Jan 21, 2020

gdemonet left a comment

bert-e commented Jan 21, 2020

bert-e commented Jan 21, 2020

TeddyAndrieux left a comment

gdemonet left a comment

TeddyAndrieux left a comment

Ebaneck commented Jan 22, 2020

bert-e commented Jan 22, 2020

Ebaneck commented Jan 22, 2020

bert-e commented Jan 22, 2020

bert-e commented Jan 22, 2020

bert-e commented Jan 22, 2020

bert-e commented Jan 22, 2020

bert-e commented Jan 22, 2020

bert-e commented Jan 23, 2020

salt: fix unsafe approach for etcd `initial-cluster-state #2198

salt: fix unsafe approach for etcd `initial-cluster-state #2198

Conversation

Ebaneck commented Jan 21, 2020

Etcd deployment should be resilient.

bert-e commented Jan 21, 2020

Hello ebaneck,

bert-e commented Jan 21, 2020

Integration data created

bert-e commented Jan 21, 2020

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Jan 21, 2020

Waiting for approval

bert-e commented Jan 21, 2020

History mismatch

TeddyAndrieux left a comment

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

TeddyAndrieux left a comment

Choose a reason for hiding this comment

Ebaneck commented Jan 22, 2020

bert-e commented Jan 22, 2020

History mismatch

Ebaneck commented Jan 22, 2020

bert-e commented Jan 22, 2020

Reset complete

bert-e commented Jan 22, 2020

Integration data created

bert-e commented Jan 22, 2020

Waiting for approval

bert-e commented Jan 22, 2020

Build failed

bert-e commented Jan 22, 2020

In the queue

bert-e commented Jan 23, 2020