-
Notifications
You must be signed in to change notification settings - Fork 9
Fix integration test timeout with VM image pull optimization #178
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
base: main
Are you sure you want to change the base?
Fix integration test timeout with VM image pull optimization #178
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yalzhang The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
5ea1bb4 to
a616774
Compare
- Change VM imagePullPolicy to "IfNotPresent" for image caching - Increase test_attestation VM startup timeout from 300s to 900s - Update pre-pull-images.sh to use $APPROVED_IMAGE from Makefile - Delete the kubevirt images in the pre-pull-images.sh since the multi-arch load issues Fixes first integration test timeout while speeding up subsequent tests. Signed-off-by: Yalan Zhang <yalzhang@redhat.com>
a616774 to
653ef93
Compare
| "quay.io/kubevirt/virt-launcher:${KV_VERSION}" | ||
| "quay.io/kubevirt/virt-handler:${KV_VERSION}" | ||
| "quay.io/kubevirt/virt-api:${KV_VERSION}" | ||
| "quay.io/kubevirt/virt-controller:${KV_VERSION}" | ||
| "quay.io/kubevirt/virt-operator:${KV_VERSION}" |
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.
multi-arch load issues
which issues?
| echo "Error: Failed to pull $IMAGE" | ||
| fi | ||
| kind load docker-image $IMAGE | ||
| docker pull "$IMAGE" && kind load docker-image "$IMAGE" |
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.
+1 but maybe
| docker pull "$IMAGE" && kind load docker-image "$IMAGE" | |
| docker pull "$IMAGE" && kind load docker-image "$IMAGE" || echo "Error: Failed to pull $IMAGE" |
| container_disk: Some(VirtualMachineTemplateSpecVolumesContainerDisk { | ||
| image: image.to_string(), | ||
| image_pull_policy: Some("Always".to_string()), | ||
| image_pull_policy: Some("IfNotPresent".to_string()), |
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.
Does this increase waits a lot? As per the k8s docs, even for Always, if the digest matches, the image does not get downloaded again. My worry is that some new image that doesn't receive a new tag is not used.
|
|
||
| test_ctx.info(format!("Waiting for VM {} to reach Running state", vm_name)); | ||
| virt::wait_for_vm_running(client, namespace, vm_name, 300).await?; | ||
| virt::wait_for_vm_running(client, namespace, vm_name, 900).await?; |
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.
Not for this PR but maybe we should have configurable multipliers for these.
Fixes first integration test timeout while speeding up subsequent tests.