Make baremetal targets more resilient to re-running #1004
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: slagle The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/757fa2e5999c44e9a3c2177d929abf44 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 37m 09s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/16ea423fe18a4050b9204a9afda9feed ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 43m 17s |
slagle
force-pushed
the
baremetal-cleanup
branch
from
4b68adf to
0f01f20
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4ec49ccf729540e99379e7b8bd274d09 ❌ openstack-k8s-operators-content-provider FAILURE in 12m 10s |
This refactors some of the baremetal targets and scripts so that the targets can be re-used without completely removing everything (resources and instances) that were created on previous runs. This enables: - creating different sets of edpm compute nodes, some preprovisioned and some baremetal, without the baremetal target completely deleting all edpm instances that existed previously. - adding additional baremetal nodes to create different NodeSets/Deployments without deleting all previous resources. - Setting specific baremetal node prefixes and suffixes to control the naming so that specific new nodes can be added without overwriting others. - Removes over eager rm's, such as for out/edpm, which completely wipes out anything edpm related already deployed. Signed-off-by: James Slagle <[email protected]>
slagle
force-pushed
the
baremetal-cleanup
branch
from
0f01f20 to
ecad344
Compare
slagle
changed the title
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/aa440636c1db4dabb5d478ff97b3f811 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 58m 04s |
| scripts/edpm-compute-baremetal.sh --cleanup | ||
| pushd .. && rm -Rf out/edpm || true && popd | ||
| make bmaas_sushy_emulator_cleanup || true | ||
| if [ "${NODE_COUNT}" = "1" ]; then \ |
There was a problem hiding this comment.
As we're changing the name of the vms and bmhs, cleanup won't work with things created with earlier version of the script. They have to be manually deleted.
There was a problem hiding this comment.
I think that would still be possible with using the env vars to set the names. However, I don't think we need to maintain backwards compatibility here anyway.
| edpm_baremetal_compute: export BMAAS_NODE_COUNT=${BM_NODE_COUNT} | ||
| edpm_baremetal_compute: export NODE_COUNT=${BM_NODE_COUNT} | ||
| edpm_baremetal_compute: export DEPLOY_DIR=../out/bmaas | ||
| edpm_baremetal_compute: ## Create virtual baremetal for the dataplane | ||
| $(eval $(call vars)) | ||
| make bmaas_virtual_bms | ||
| make bmaas_sushy_emulator_wait | ||
| bash scripts/edpm-compute-baremetal.sh --create | ||
| if [ "${NODE_COUNT}" = "1" ]; then \ |
There was a problem hiding this comment.
Though you can now re-run to create additional BMHs, as we cleanup the existing OpenstackDataPlaneNodeSet/OpenStackDataPlaneDeployments with edpm_deploy_cleanup[1] that would deprovision the all existing BMHs as well, before provisioning them again.
[1] https://github.com/openstack-k8s-operators/install_yamls/blob/main/Makefile#L922
There was a problem hiding this comment.
true. I would actually drop the cleanup deps from edpm_baremetal_deploy and edpm_deploy. I usually use the existing resources already written out, customize them as needed, and use them with oc directly. I guess it depends how far we want to take it in terms of making the targets work without cleanup or making them re-runnable.
| if [ ${NODE_COUNT} -eq 1 ]; then \ | ||
| scripts/bmaas/vbm-setup.sh --create ; \ | ||
| else \ | ||
| for INDEX in $(shell seq 0 $$((${NODE_COUNT} -1))) ; do \ |
There was a problem hiding this comment.
Calling these scripts as many times as the number of nodes is possibly little inefficient, though does not matter probably as no one would be deploying large number of nodes with install_yamls.
There was a problem hiding this comment.
Why do we need to iterate in the Makefile though? The vbm-setup.sh takes a NODE_COUNT env var and iterates within the script itself:
https://github.com/openstack-k8s-operators/install_yamls/blob/main/devsetup/scripts/bmaas/vbm-setup.sh#L166-L170
if [ ${NODE_COUNT} -eq 1 ]; then \
scripts/bmaas/vbm-setup.sh --create ; \
else
scripts/bmaas/vbm-setup.sh --create --num-nodes $NODE_COUNT
fi
Then we can just handle the NODE_NAME_SUFFIX in the script?
Just avoids re-running the script like Rabi is mentioning/
There was a problem hiding this comment.
I guess the whole check isn't really needed in that scenario actually, because NODE_COUNT would just be handled in the script anyway.
There was a problem hiding this comment.
This PR changes vbm-setup.sh to drop --num-nodes, so it manages just a single VM. That change is already included in this PR. Are you asking me to drop that change from the PR? The only inefficient part of calling the script twice would be setting up the libvirt logging, not really something I'd optimize in a dev repo, but we can go back to the way it was if that is wanted.
gen-edpm-node.sh manages 1 vm at a time. That is used for networker and bootc vms already, with the Makefile controlling the looping. So, I was changing the baremetal scripts to work in a similar way.
gen-edpm-node-bgp.sh is hardcoded to manage 3 at a time, so that's a different way as well.
Whichever way it's done, I'd like to see these things consistent.
This refactors some of the baremetal targets and scripts so that the
targets can be re-used without completely removing everything (resources
and instances) that were created on previous runs. This enables:
some baremetal, without the baremetal target completely deleting all
edpm instances that existed previously.
NodeSets/Deployments without deleting all previous resources.
naming so that specific new nodes can be added without overwriting
others.
out anything edpm related already deployed.
Signed-off-by: James Slagle [email protected]