e2e: make provisioning more robust. - #773
Draft
klihub wants to merge 3 commits into
Draft
Conversation
Nothing said whether provisioning ever ran to the end, so a run interrupted while provisioning looked, to the next one, like a run which had finished. That run then skipped provisioning and every test case failed on a VM with no cluster. Guessing cannot replace the fact: a Vagrantfile appears before the VM is created, and the provisioned flag of vagrant is cleared by the --no-provision of a VM which comes from a box. So leave a mark, in two places. The one in the output directory is what a run reads when it decides whether to provision. The one in the VM is written by the playbook as its last task, so it travels inside the disk image and a box packaged from the VM can be checked rather than trusted. Adding that task changes the provisioning recipe hash, which invalidates the cached boxes. That is what we want: they predate the mark. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The playbook ends in kubeadm init, which cannot run on a VM which already has a cluster: the ports are taken, the manifests are in place and etcd has data. So provisioning a VM which has been provisioned before failed, whether it was asked for with provision=1 or whether the framework was retrying provisioning that an earlier run had interrupted. Take the existing cluster down with kubeadm reset first. A VM which has not been created, or which never got as far as installing kubeadm, has nothing to reset. This needs the VM to be up: if it cannot be reached the run says so and provisions the VM as it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
A VM without the mark of the playbook cannot run tests, whether its provisioning was interrupted, it comes from an output directory or a box older than the mark, or it was never provisioned at all. Provisioning fixes every one of those, so do that rather than report the VM and give up. Make provision a tri-state so that a run keeps the last word: no leaves an existing VM alone and reports it, 1 provisions it in any case, and the default provisions it only if it turns out to need it. Note that provision=no used to mean the same as provision=1, being non-empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the e2e framework’s VM lifecycle handling by introducing durable “provisioning completed” stamps (both host-side and inside the VM) and by making reprovisioning safer (cluster reset) and more automatic when a VM is detected as not fully provisioned.
Changes:
- Add documentation describing re-provisioning behavior and the new
provisionmodes (auto/force/never). - Update the Ansible provisioning playbook to write an in-VM provisioning completion marker (
/etc/nri-e2e-provisioned) at the very end. - Extend
test/e2e/lib/vm.bashto track provisioning state via.provisioned, verify the in-VM stamp, and reset the cluster before reprovisioning.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/e2e/README.md | Documents the new provisioning markers and reprovisioning controls (provision=*). |
| test/e2e/playbook/provision.yaml | Records successful end-of-playbook completion inside the VM via /etc/nri-e2e-provisioned. |
| test/e2e/lib/vm.bash | Implements provisioning state tracking, auto-reprovisioning, and pre-provision kubeadm reset logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| command -v kubeadm > /dev/null || exit 0 | ||
| kubeadm reset --force || true | ||
| rm -rf /etc/cni/net.d /root/.kube /home/vagrant/.kube | ||
| rm -f $VM_PROVISIONED_STAMP'" ); then |
Comment on lines
+615
to
+630
| if [ "$provisioning_mode" == "force" ]; then | ||
| # Provisioning was asked for explicitly, so provision whatever is here. | ||
| # Until that succeeds this VM does not count as provisioned. | ||
| vm-unmark-provisioned "$vagrantdir" | ||
| elif vm-provisioned "$vagrantdir"; then | ||
| echo "VM $vmname is already provisioned, skipping provisioning..." | ||
| # Keep the provisioner out of the Vagrantfile too: --no-provision leaves | ||
| # the machine flagged as not provisioned, so the next vagrant up, whether | ||
| # it comes from the next test case or from make ssh, would run it. | ||
| no_provision="--no-provision" | ||
| e2e_no_provision=1 | ||
| # The cluster of a VM which came from a box starts up when the VM boots, | ||
| # so it may still be starting, see the wait at the end of this function. | ||
| if vm-provisioned-from-box "$vagrantdir"; then | ||
| use_cached_box=1 | ||
| fi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make provisioning/reprovisioning a bit more robust/convenient.