-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fix backup restore #2161
Fix backup restore #2161
Conversation
Hello teddyandrieux,My role is to assist you with the merge of this Status report is not available. |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/2.5/bugfix/fix-backup-restore origin/development/2.5
$ git merge origin/bugfix/fix-backup-restore
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.5/bugfix/fix-backup-restore |
ceb28f7
to
7e0e729
Compare
/help |
Help pageThe following options and commands are available at this time. Options
Commands
|
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/2.5/bugfix/fix-backup-restore origin/development/2.5
$ git merge origin/bugfix/fix-backup-restore
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.5/bugfix/fix-backup-restore |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
/after_pull_request=2147 |
Waiting for other pull request(s)The current pull request is locked by the after_pull_request option. In order for me to merge this pull request, run the following actions first: ➡️ Merge the Alternatively, delete all the after_pull_request comments from this pull request. The following options are set: after_pull_request |
/approve |
Waiting for other pull request(s)The current pull request is locked by the after_pull_request option. In order for me to merge this pull request, run the following actions first: ➡️ Merge the Alternatively, delete all the after_pull_request comments from this pull request. The following options are set: approve, after_pull_request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of it looks good to me, will test :)
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
The following options are set: approve, after_pull_request |
I don't know if it's a PEBKAC issue (or even a flaky), but I had to add the new etcd member by hand to complete the bootstrap node restoration.
Here is the output of the script:
In etcd container logs, I have:
Am I missing a step or something ? |
7e0e729
to
f68a05b
Compare
History mismatchMerge commit #bc1e12933d05b44283ebcf6fff18face66d09be6 on the integration branch It is likely due to a rebase of the branch Please use the The following options are set: approve, after_pull_request |
f68a05b
to
7bb37bc
Compare
Move kubernetes pki from `./pki` to `./kubernetes/pki` in backup archive to reflect machine layout Fixes: #2142
Add some metadata information in the backup archive and a `sha256sum` of all the file, and check integrity of the backup archive at the beginning of the restore script Fixes: #2143
For restore we may want to specify an external apiserver IP for apiserver proxy and admin kubeconfig
To properly configure the new bootstrap node we need to have mine informations from other nodes (like control plane and workload plane IPs), to be able to update the mine we need that all minions ready and having correct pillar
You cannot use the restore script if you do not have High Availability | ||
apiserver because some information required to reconfigure the others | ||
nodes are stored in the apiserver. | ||
|
||
.. warning:: | ||
|
||
In case of a 3-node etcd cluster (2 nodes + unreachable old bootstrap node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, maybe it should be an optional step in the clear, with the required command(s) and expected output. Too easy to go over it and have weird errors in the script's output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done but command are quite ugly we could do something smarter if we add a salt module to remove an etcd member but not part of this PR IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, would be nice to provide a user-friendly wrapper to etcdctl
, maybe as a Salt module or in a kubectl
plugin, not sure. For now, having the "ugly" command for copy-pasting is good enough!
7bb37bc
to
09939e4
Compare
/reset |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: approve, after_pull_request |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/2.5/bugfix/fix-backup-restore origin/development/2.5
$ git merge origin/bugfix/fix-backup-restore
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.5/bugfix/fix-backup-restore The following options are set: approve, after_pull_request |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
The following options are set: approve, after_pull_request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some docs changes, otherwise LGTM
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
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: The following options are set: approve, after_pull_request |
bb83bb2
to
22d4188
Compare
History mismatchMerge commit #09939e4df6c156c48413af0f463e514a8dac89ee on the integration branch It is likely due to a rebase of the branch Please use the The following options are set: approve, after_pull_request |
During restore we need to sync_auth to have a working salt-api, also need to reconfigure the control plane nginx ingress to set the new external IP used to access the UI
/reset |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: approve, after_pull_request |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/2.5/bugfix/fix-backup-restore origin/development/2.5
$ git merge origin/bugfix/fix-backup-restore
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.5/bugfix/fix-backup-restore The following options are set: approve, after_pull_request |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
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: The following options are set: approve, after_pull_request |
Build failedThe build for commit did not succeed in branch w/2.5/bugfix/fix-backup-restore. The following options are set: approve, after_pull_request |
SALT_MASTER_CALL=(crictl exec -i "$(get_salt_container)") | ||
|
||
"${SALT_MASTER_CALL[@]}" salt-run --state-output=mixed state.orchestrate \ | ||
metalk8s.addons.nginx-ingress-control-plane.deployed \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could simply run metalk8s.deployed
here? Being idempotent and only executed on the master, I'm thinking it's some kind of "K8s highstate"... If you think so, we could do with a debt ticket to fix in 2.5 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since for the moment we basically replace every object I think it's not a good idea because we will lose all tuning done by the user on k8s objects when restoring which is unexpected IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let's wait for #1996 then
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve, after_pull_request |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye teddyandrieux. |
Component:
'backup', 'restore'
Context:
Summary:
apiserver_ip
arg in restore scriptAcceptance criteria:
Working backup restore procedure
TODO:
Fixes: #2141
Fixes: #2142
Fixes: #2143
Fixes: #2157