Repository navigation
Conversation
klihub
force-pushed
the
fixes/e2e/wait-for-network-online
branch
from
November 2, 2026 18:51
750532a to
e553bfc
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Critical provisioning defects prevent fresh VMs from loading Ansible variables, allow reuse of incompletely reprovisioned hardware hosts, and leave NRI disabled on supported containerd 1.7 installations.
5 open issues
What changed in this PR
This PR hardens end-to-end test provisioning against network startup races and incomplete VM setup. Stacked on #883, it also includes support for running tests on existing hosts.
Changes:
- Wait for VM networking and explicitly refresh Fedora repository metadata before installing packages.
- Track incomplete VM provisioning to prevent reuse of partially configured machines.
- Add external-host setup, installed-runtime handling, shared Ansible variables, and usage documentation.
| File | Description |
|---|---|
test/e2e/run.sh |
Handles external-host naming, caching, and installed-runtime resolution. |
test/e2e/run_tests.sh |
Resolves external-host identity before running test suites. |
test/e2e/README.md |
Documents provisioning recovery and external-host usage. |
test/e2e/playbook/provision.yaml |
Adds network readiness checks and adapts provisioning for existing hosts. |
test/e2e/playbook/custom-kernel.yaml |
Uses a login-relative destination for cached kernel packages. |
test/e2e/lib/vm.bash |
Adds host provisioning helpers, shared variables, and provisioning-state tracking. |
test/e2e/files/Vagrantfile.in |
Reads provisioning variables from the generated JSON file. |
test/e2e/files/crio-nri-enable |
Removes the explicit CRI-O NRI-enablement helper. |
test/e2e/files/containerd-nri-enable |
Removes the explicit containerd NRI-enablement helper. |
Review effort: Balanced
| nri_resource_policy_src: NRI_RESOURCE_POLICY_SRC, | ||
| outdir: OUTPUT_DIR, | ||
| } | ||
| ansible.extra_vars = "@#{OUTPUT_DIR}/ansible-vars.json" |
Comment on lines
+853
to
+855
| if [ -n "$first" ] || [ -n "${provision:-}" ]; then | ||
| vm-hw-check-provisionable "$vagrantdir" | ||
| vm-provision-hw "$vagrantdir" |
| ansible.builtin.command: | ||
| cmd: /usr/local/bin/crio-nri-enable | ||
| when: is_crio | ||
| when: is_containerd and (manage_runtime | default(true) | bool) |
Comment on lines
+876
to
+879
| local s="${1//\\/\\\\}" | ||
|
|
||
| s="${s//\"/\\\"}" | ||
| printf '"%s"' "$s" |
There was a problem hiding this comment.
🟡 Changes recommended
Critical provisioning defects make Vagrant reference a nonexistent vars file and allow failed host reprovisioning to leave stale success records, causing later runs to reuse a partially rebuilt host.
6 open issues
Review effort: Balanced
klihub
force-pushed
the
fixes/e2e/wait-for-network-online
branch
from
November 2, 2026 19:22
e553bfc to
669dc31
Compare
The Vagrantfile built them in ruby, so provisioning a host which is not a vagrant VM would have needed a second copy of the same twenty-four values. Write them to a json file instead, which the vagrant provisioner and a direct ansible-playbook run both take. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The kubeconfig tasks hardcoded /home/vagrant and chowned to vagrant, so provisioning a host whose login user is anyone else died on the chown. This play runs with become, so every home fact describes root and the home has to be looked up instead. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Mode detection, option validation and the ssh config of a host which is not a VM we create. The config is the shape vagrant ssh-config writes, alias included, so nothing above it has to know which kind of host it is talking to. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The name of a VM doubles as the ssh alias and as the Kubernetes node name. On a host we did not create those come apart: the node is named after the host, while the output directory still has to tell a containerd run from a crio one against the same box. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
It hardcoded the vagrant user and looked for a key under .vagrant, and it is what deploys the plugin for every test, so a run against a host of its own would have failed at the first test rather than while provisioning. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
A host which already runs a cluster and has no recorded versions is refused rather than provisioned over: a second kubeadm init will not run, and taking a cluster down on a machine the user may share is not something to do unasked. provision=all is the way through. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
What tells a first run from a later one was the presence of a Vagrantfile, which a host of its own never has, so its record decides instead. The way out of a mismatch differs too: deleting an output directory does not change what is installed on a host. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Set test_hw_addr and test_hw_key and the framework writes the ssh config itself, provisions over ssh with the same playbook and the same variables, and skips everything about making a VM. Asking for the box cache as well is refused: there is nothing to package. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The playbook is now told it is on a host somebody owns, as is_hardware. Such a host does not get its root filesystem grown, and SELinux is not switched off: it has to be permissive or disabled already, since the task would also set it disabled for a next boot this run never performs. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Left to itself kubeadm names the node after the host's own hostname, which on a real host is the fqdn, while the framework looks the node up by the short name it was given. So kubectl wait never found the node. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Since Kubernetes 1.30 kubelet can run on a host which has swap enabled, so configure it to tolerate swap without using it rather than switching off somebody's swap. That needs a KubeletConfiguration on every run, so the kubeadm configuration is written unconditionally now and is the only way the cluster is initialized. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The unit files went to /lib/systemd/system, a symlink to /usr/lib on Fedora, so each overwrote the package-owned unit. /etc/systemd/system takes precedence instead, leaving the distro's file alone. On top of that, containerd_release=installed and crio_release=installed keep the host's runtime untouched, reading its version off the host. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Both runtimes enable NRI themselves now. The containerd script undid nothing: the playbook writes the config with "containerd config default", which already has disable = false. With the python one went the toml libraries it was the only user of. Nothing needed /etc/nri/conf.d or /opt/nri/plugins either, as the plugins are DaemonSets. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
What a run against such a host does differently, what it refuses, what the host has to provide when its own runtime is kept, and a walkthrough which anyone can follow against 127.0.0.1 without separate hardware. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
kubeadm reset was given the socket the run asked for, which is the wrong one when the runtime is being switched: it then removes none of the control plane containers, and they go on holding the ports kubeadm init needs. The endpoint kubeadm recorded in kubelet's arguments is used instead, and the requested socket stays the fallback. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Notice if vagrant provisioning fails for a VM. Bail out with an error for the related tests instead of trying to run them and failing with quasi-random errors. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Vagrant itself only waits for sshd which might start serving while the guest is still bringing up networking. Nothing in the box pulls in network-online.target. sshd is ordered after network.target alone, so any network access once sshd is up races with the guest network being brough online. One such network access attempt is the first DNS invocation, which then might simply fail. Add an explicit wait for the network to get online before we try to access it. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The dnf5 ansible module reports a failure to download repository metadata as a python traceback. It does not give a clear reason for failure neither it names the failed repository. Ask DNF for the metadata once the repositories are configured, so a failure reports both. Note that DNF expires its metadata per repository, so this only downloads what is missing or stale, never already cached data. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
klihub
force-pushed
the
fixes/e2e/wait-for-network-online
branch
from
November 3, 2026 07:13
669dc31 to
2dc1275
Compare
This branch has not been deployed
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.
Note: since it is touching the same parts of the provisioning playbook, this PR is stacked on #883.
This PR fixes an initial race which can and sometimes does cause a failed provisioning, which then goes unnoticed and causes a cascading set of often misleading errors we try to run the tests against a partially provisioned VM. In particular these commits