-
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
Test bootstrap restore in CI #2061
Conversation
Hello alexandre-allard-scality,My role is to assist you with the merge of this Status report is not available. |
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:
|
2bec115
to
94965b3
Compare
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
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:
|
94965b3
to
04b2bb2
Compare
History mismatchMerge commit #68424c5580f49f92de6154607f5b4a30979a6e92 on the integration branch It is likely due to a rebase of the branch Please use the |
e101352
to
3cada92
Compare
Branches have divergedThis pull request's source branch To avoid any integration risks, please re-synchronize them using one of the
Note: If you choose to rebase, you may have to ask me to rebuild |
3cada92
to
2316a62
Compare
History mismatchMerge commit #2498e576cf484803cfbd102f5fae9752f54302e8 on the integration branch It is likely due to a rebase of the branch Please use the |
Great effort, seems like you encountered much more problems than anticipated. However, I think we don't want to keep adding things to Eve specifics, but instead extend the work that was done in |
09ffe18
to
c0fd193
Compare
c0fd193
to
8e18864
Compare
8e18864
to
ccb5142
Compare
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.
Small comment but LGTM
/reset |
/reset |
Reset completeI have successfully deleted this pull request's integration branches. |
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/improvement/1687-test-restore-in-ci origin/development/2.5
$ git merge origin/improvement/1687-test-restore-in-ci
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.5/improvement/1687-test-restore-in-ci |
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.6/improvement/1687-test-restore-in-ci origin/development/2.6
$ git merge origin/w/2.5/improvement/1687-test-restore-in-ci
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.6/improvement/1687-test-restore-in-ci |
ConflictA conflict has been raised during the update of Please resolve the conflict on the integration branch ( Here are the steps to resolve this conflict: $ git fetch
$ git checkout w/2.5/improvement/1687-test-restore-in-ci
$ git pull # or "git reset --hard origin/w/2.5/improvement/1687-test-restore-in-ci"
$ git merge origin/development/2.5
$ # <intense conflict resolution>
$ git commit
$ git merge origin/improvement/1687-test-restore-in-ci
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.5/improvement/1687-test-restore-in-ci |
Apiserver proxy redirect to an old bootstrap node that may not longer exists, so lets reconfigure apiserver-proxy on every nodes to not longer talk to the old bootstrap node
Deploy two node instead of only one during expansion tests to have HA etcd and being able to test the bootstrap restoration Refs: #1687
(cherry picked from commit 1e110a3)
This `then` will be useful in restoration tests also so we move it to a common place Refs: #1687
Adding `EXTRA_OPTS` environment variable in `wait_pods_status` helper to allow to add extra arguments like "--namespace" if needed
We now create AlertManager & Prometheus volumes on node1 instead of the bootstrap node on multi node environment to properly test that storage volume does not need to sit on bootstrap node
We need to be able to connect to all the nodes to do the expansion from the bastion host, create an helper in eve to do it
we don't want to check presence of kube-dns pods on the bootstrap node, because pods can move to other node, especially during restore tests Refs: #1687
This scenario run the restore script and ensure that everything is working as expected (pods, ...) after the restoration. Refs: #1687
Launch bootstrap restore tests in CI during post-merge stages, this CI step first need a full MetalK8s cluster with at least 3-node etcd cluster Refs: #1687
bd08570
to
0293ddd
Compare
History mismatchMerge commit #c6a3bee8fde5180bfefdf29af4869211091ac97d on the integration branch It is likely due to a rebase of the branch Please use the |
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.6/improvement/1687-test-restore-in-ci origin/development/2.6
$ git merge origin/w/2.5/improvement/1687-test-restore-in-ci
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.6/improvement/1687-test-restore-in-ci |
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:
|
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.
Nothing preventing the merge, but Terraform, our tests, and Eve's YAML need to be cleaned-up and refactored. For the future :)
@@ -1597,7 +1597,7 @@ stages: | |||
name: Run installation scenarii on the bastion | |||
env: | |||
<<: *_env_bastion_tests | |||
PYTEST_FILTERS: "install and ci and multinodes" | |||
PYTEST_FILTERS: "install and ci and multinodes and not node2" |
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.
Beurk. Let's keep it, but really not what BDD tags should be used for, IMHO
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.
Yes but we do not have better solution here for expansion right now I think
- ShellCommand: ©_bastion_pub_key_ssh | ||
name: Send bastion public key to nodes | ||
command: > | ||
for host in $HOSTS_LIST; do | ||
ssh -F ssh_config bastion "cat .ssh/bastion.pub" | | ||
ssh -F ssh_config $host "cat >> .ssh/authorized_keys" | ||
done | ||
env: | ||
HOSTS_LIST: bootstrap | ||
workdir: build/eve/workers/openstack-multiple-nodes/terraform/ | ||
|
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.
Why not do this in Terraform?
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.
It was not done before but yes maybe this could go in terraform
- SetPropertyFromCommand: | ||
name: Set node1 etcd container id | ||
property: node1_etcd_container_id | ||
command: > | ||
ssh -F ssh_config node1 | ||
sudo crictl ps -q --label io.kubernetes.pod.namespace=kube-system | ||
--label io.kubernetes.container.name=etcd --state Running | ||
workdir: build/eve/workers/openstack-multiple-nodes/terraform/ | ||
haltOnFailure: true | ||
- SetPropertyFromCommand: | ||
name: Set boostrap node etcd member id | ||
property: bootstrap_etcd_member_id | ||
command: > | ||
ssh -F ssh_config node1 | ||
sudo crictl exec -i "%(prop:node1_etcd_container_id)s" sh -c \" | ||
ETCDCTL_API=3 etcdctl --endpoints https://127.0.0.1:2379 | ||
--cert /etc/kubernetes/pki/etcd/server.crt | ||
--key /etc/kubernetes/pki/etcd/server.key | ||
--cacert /etc/kubernetes/pki/etcd/ca.crt | ||
member list\" | awk -F ', ' '$3 ~ "bootstrap" { print $1 }' | ||
workdir: build/eve/workers/openstack-multiple-nodes/terraform/ | ||
haltOnFailure: true | ||
- ShellCommand: | ||
name: Remove bootstrap node from etcd members | ||
command: > | ||
ssh -F ssh_config node1 | ||
sudo crictl exec -i "%(prop:node1_etcd_container_id)s" sh -c \" | ||
ETCDCTL_API=3 etcdctl --endpoints https://127.0.0.1:2379 | ||
--cert /etc/kubernetes/pki/etcd/server.crt | ||
--key /etc/kubernetes/pki/etcd/server.key | ||
--cacert /etc/kubernetes/pki/etcd/ca.crt | ||
member remove %(prop:bootstrap_etcd_member_id)s\" | ||
workdir: build/eve/workers/openstack-multiple-nodes/terraform/ | ||
haltOnFailure: true | ||
- ShellCommand: | ||
name: Remove bootstrap node object | ||
command: > | ||
ssh -F ssh_config node1 | ||
sudo kubectl --kubeconfig=/etc/kubernetes/admin.conf delete | ||
node --selector="node-role.kubernetes.io/bootstrap" | ||
workdir: build/eve/workers/openstack-multiple-nodes/terraform/ | ||
haltOnFailure: true |
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.
Why isn't all this orchestrated in the test? Looks like it's not in "the right place"...
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.
Yes could be part of the pytest test directly but then we need to do it after spawning the new bootstrap node, anyway yes it's doable
/approve |
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 |
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 alexandre-allard-scality. |
Component: ci, tests
Context:
Summary:
Acceptance criteria: Green build
Closes: #1687