Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gather pillar instead of refresh in check pillar salt module #2445

Merged

Conversation

TeddyAndrieux
Copy link
Collaborator

@TeddyAndrieux TeddyAndrieux commented Apr 22, 2020

Component:

'salt'

Context:

Flaky Rendering SLS missing key

Summary:

Check 2.4 - 2.6 branches for a cleaner/simpler diff

Backport some commit to improve stability in 2.0-2.3 branches:

Gather pillar information instead of just refresh and wait in check_pillar_keys salt module.

Revert f70905c as we do not need to manually refresh the pillar, a new pillar is gathered by the salt minion every time you run a salt state.

Add check pillar in etcd orchestrate

Acceptance criteria:

No more Rendering SLS flakies


Fixes: #2444

@TeddyAndrieux TeddyAndrieux requested a review from a team April 22, 2020 09:37
@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

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
Copy link
Contributor

bert-e commented Apr 22, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh with contents from w/2.2/bugfix/GH-2444-gather-pillar-instead-of-refresh
and development/2.3.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh origin/development/2.3
 $ git merge origin/w/2.2/bugfix/GH-2444-gather-pillar-instead-of-refresh
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh

TeddyAndrieux and others added 2 commits April 22, 2020 12:13
When we run `etcd` state we want to check `etcd` health by default but
when we deploy a new `etcd` the new member is not yet registered in the
`etcd` cluster so we need to skip the healthcheck of the `etcd` when
deploying this new `etcd` node, so adding a pillar value to skip
`etcd` healthcheck in the salt state

(cherry picked from commit fffbeb9)
This commit splits node deployment procedure into
a sequence of stages.

Initially, expanding the etcd cluster from 1 to 2 nodes puts
the cluster in a blocked state. This happens when a new member is declared
and not yet started.
If this time to start is too long, liveness probes on the initial etcd Pod will
fail, then it will be restarted and then enter a back-off loop.
Even after the new etcd Pod has been created, the new one won't be able to start,
since the initial is in back-off and will enter a back-off as well.

To prevent the above situation, we pre-pull the etcd image on the target Node,
which ensures the shortest time possible between the new member being registered
and its corresponding Pod being created. Note that this reordering is inspired by
kubespray

(cherry picked from commit 5f717c6)

Remove `skip_apiserver-proxy` as do not exists in this branch
@TeddyAndrieux TeddyAndrieux force-pushed the bugfix/GH-2444-gather-pillar-instead-of-refresh branch from fac7979 to 7550a5f Compare April 22, 2020 10:13
@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

History mismatch

Merge commit #c137acfe2f0502343d4c92bd645b6b703b96b084 on the integration branch
w/2.1/bugfix/GH-2444-gather-pillar-instead-of-refresh is merging a branch which is neither the current
branch bugfix/GH-2444-gather-pillar-instead-of-refresh nor the development branch
development/2.1.

It is likely due to a rebase of the branch bugfix/GH-2444-gather-pillar-instead-of-refresh 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.

@TeddyAndrieux
Copy link
Collaborator Author

/reset

@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

Reset complete

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

@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh with contents from w/2.2/bugfix/GH-2444-gather-pillar-instead-of-refresh
and development/2.3.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh origin/development/2.3
 $ git merge origin/w/2.2/bugfix/GH-2444-gather-pillar-instead-of-refresh
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh

During deploy node we restart some mandatory pod (salt-master,
repositories) at several place so we need to check pillar value
availability several time

(cherry picked from commit f528911)
In `check_pillar_keys` gather pillar information using internal salt
function instead of using `saltutil.refresh_pillar` as `refresh_pillar`
do not allow synchronous refresh in salt 2018.3

Fixes: #2444
…rate"

This reverts commit f70905c.

Refresh_pillar is not needed as a pillar is gathered by the salt-minion
each time a state is run

Sees: #2444
During etcd orchestrate etcd to deploy etcd we need to have information
about repository endpoint, so check this pillar value before running the
salt state on the minion
@TeddyAndrieux TeddyAndrieux force-pushed the bugfix/GH-2444-gather-pillar-instead-of-refresh branch from 7550a5f to a900a46 Compare April 22, 2020 11:12
@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

History mismatch

Merge commit #78b5b5083710099f65b7a95e04670efaa2958143 on the integration branch
w/2.1/bugfix/GH-2444-gather-pillar-instead-of-refresh is merging a branch which is neither the current
branch bugfix/GH-2444-gather-pillar-instead-of-refresh nor the development branch
development/2.1.

It is likely due to a rebase of the branch bugfix/GH-2444-gather-pillar-instead-of-refresh 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.

@TeddyAndrieux
Copy link
Collaborator Author

/reset

@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

Reset complete

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

@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh with contents from w/2.2/bugfix/GH-2444-gather-pillar-instead-of-refresh
and development/2.3.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh origin/development/2.3
 $ git merge origin/w/2.2/bugfix/GH-2444-gather-pillar-instead-of-refresh
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh

@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

Conflict

A conflict has been raised during the creation of
integration branch w/2.4/bugfix/GH-2444-gather-pillar-instead-of-refresh with contents from w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh
and development/2.4.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.4/bugfix/GH-2444-gather-pillar-instead-of-refresh origin/development/2.4
 $ git merge origin/w/2.3/bugfix/GH-2444-gather-pillar-instead-of-refresh
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.4/bugfix/GH-2444-gather-pillar-instead-of-refresh

@bert-e
Copy link
Contributor

bert-e commented Apr 22, 2020

Integration data created

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

The following branches will NOT be impacted:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.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
Copy link
Contributor

bert-e commented Apr 22, 2020

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 TeddyAndrieux marked this pull request as ready for review April 22, 2020 17:30
@TeddyAndrieux
Copy link
Collaborator Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Apr 23, 2020

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

Copy link
Contributor

@gdemonet gdemonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for taking care of this rather obscure issue :)

@bert-e
Copy link
Contributor

bert-e commented Apr 23, 2020

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.0

  • ✔️ development/2.1

  • ✔️ development/2.2

  • ✔️ development/2.3

  • ✔️ development/2.4

  • ✔️ development/2.5

  • ✔️ development/2.6

The following branches will NOT be impacted:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.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
Copy link
Contributor

bert-e commented Apr 23, 2020

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

  • ✔️ development/2.0

  • ✔️ development/2.1

  • ✔️ development/2.2

  • ✔️ development/2.3

  • ✔️ development/2.4

  • ✔️ development/2.5

  • ✔️ development/2.6

The following branches have NOT changed:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3

Please check the status of the associated issue GH-2444.

Goodbye teddyandrieux.

@bert-e bert-e merged commit 3d8d17e into development/2.0 Apr 23, 2020
@bert-e bert-e deleted the bugfix/GH-2444-gather-pillar-instead-of-refresh branch April 23, 2020 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants