Skip to content
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

e2e, arm64: make the e2e tests works of Arm64 platform #3577

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

zhlhahaha
Copy link
Contributor

What this PR does / why we need it:
Enable e2e test works on Arm64 platform.

  1. Make cluster-sync work on kind provider
  2. Disable VDDK deployment on Arm64
  3. Label VDDK e2e tests, thus we can skip them
  4. Fix several e2e tests

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #3466

Release note:

None

For the Kind provider, we need to configure hostname
resolution for the local image registry in the CoreDNS
service. This ensures that local container images can
be successfully pulled into Kubernetes pods during
certain e2e tests.

Signed-off-by: howard zhang <[email protected]>
@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jan 2, 2025
@coveralls
Copy link

coveralls commented Jan 2, 2025

Coverage Status

coverage: 59.426% (+0.01%) from 59.412%
when pulling 9478ffd on zhlhahaha:3466
into f4cbf29 on kubevirt:main.

@zhlhahaha
Copy link
Contributor Author

zhlhahaha commented Jan 2, 2025

@zhlhahaha
Copy link
Contributor Author

cc: @awels @cfilleke @xpivarc @dhiller

@zhlhahaha
Copy link
Contributor Author

The e2e tests have fully passed in a docker in docker environment by running

export TARGET=kind-1.28
export RANDOM_CR=true
export KUBEVIRT_STORAGE=hpp
export HPP_CLASSIC=true
export CDI_E2E_SKIP="Destructive|\[VDDK\]"
./automation/test.sh

image

@zhlhahaha
Copy link
Contributor Author

If this PR get merged, we can add a periodic e2e tests for CDI on Arm64 cluster.

@zhlhahaha zhlhahaha mentioned this pull request Jan 2, 2025
9 tasks
Copy link
Collaborator

@akalenyu akalenyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this great effort! questions attached

tests/datavolume_test.go Outdated Show resolved Hide resolved
tests/import_test.go Outdated Show resolved Hide resolved
manifests/templates/registry-host.yaml.in Outdated Show resolved Hide resolved
cluster-sync/ephemeral_provider.sh Outdated Show resolved Hide resolved
cluster-sync/ephemeral_provider.sh Show resolved Hide resolved
tests/framework/pvc.go Outdated Show resolved Hide resolved
@zhlhahaha
Copy link
Contributor Author

@akalenyu Thanks for your quick comment! I do not have time you reply all your comment today and I will reply you tomorrow.

@zhlhahaha
Copy link
Contributor Author

@akalenyu Some background, I verify the cdi e2e tests in a prow pods on Arm64 server which is similar to the kubeVirt Community CI/CD environment.
The host system is ubuntu.

@akalenyu
Copy link
Collaborator

akalenyu commented Jan 2, 2025

@akalenyu Thanks for your quick comment! I do not have time you reply all your comment today and I will reply you tomorrow.

@akalenyu Some background, I verify the cdi e2e tests in a prow pods on Arm64 server which is similar to the kubeVirt Community CI/CD environment. The host system is ubuntu.

Totally understandable, I am out tomorrow but I can pick up on it next week. Feel free to ping on the k8s slack if you need any help.

@akalenyu
Copy link
Collaborator

akalenyu commented Jan 2, 2025

/test pull-cdi-goveralls
/test pull-cdi-unit-test
not related

@akalenyu
Copy link
Collaborator

akalenyu commented Jan 2, 2025

/test pull-cdi-unit-test

@akalenyu
Copy link
Collaborator

akalenyu commented Jan 2, 2025

/test pull-containerized-data-importer-e2e-istio

@zhlhahaha
Copy link
Contributor Author

/retest

1 similar comment
@zhlhahaha
Copy link
Contributor Author

/retest

cluster-sync/sync.sh Outdated Show resolved Hide resolved
@akalenyu
Copy link
Collaborator

akalenyu commented Jan 5, 2025

I am not sure why all lanes fail with CDI pods not being ready

timed out waiting for the condition on pods/cdi-docker-registry-host-7989bdb8c8-8djb2
timed out waiting for the condition on pods/cdi-file-host-5c87dc856f-6rpqg
timed out waiting for the condition on pods/cdi-operator-59c7548cfb-g76fj
timed out waiting for the condition on pods/cdi-uploadproxy-6548888c56-9vwtr

/test pull-containerized-data-importer-e2e-nfs

EDIT: I think the non-registry pods didn't really fail the check, their state just didn't get evaluated since docker-registry host was broken or something like that

@zhlhahaha
Copy link
Contributor Author

@akalenyu
We also need Multiarch fakeovirt-image.
#3519

I have write a script to build multiarch fakeovirt-image(#3519 (comment)), but need someone run it and publish it to the quay.io/kubevirt/fakeovirt

@akalenyu
Copy link
Collaborator

akalenyu commented Jan 6, 2025

@akalenyu We also need Multiarch fakeovirt-image. #3519

I have write a script to build multiarch fakeovirt-image(#3519 (comment)), but need someone run it and publish it to the quay.io/kubevirt/fakeovirt

I see that @awels is on board so maybe @dhiller can help push this

zhlhahaha added a commit to zhlhahaha/containerized-data-importer that referenced this pull request Jan 8, 2025
This label means the e2e tests are not suitable to test in
containerized k8s env (kind provider).

In this commit, we label two tests because the large size images
would lead to OOM of cdi-uploader. For more detail, please
refer to:
kubevirt#3577 (comment)

Signed-off-by: howard zhang <[email protected]>
@kubevirt-bot kubevirt-bot added size/M and removed size/L labels Jan 8, 2025
cluster-sync/sync.sh Outdated Show resolved Hide resolved
zhlhahaha added a commit to zhlhahaha/containerized-data-importer that referenced this pull request Jan 8, 2025
This label means the e2e tests are not suitable to test in
containerized k8s env (kind provider).

In this commit, we label two tests because the large size images
would lead to OOM of cdi-uploader. For more detail, please
refer to:
kubevirt#3577 (comment)

Signed-off-by: howard zhang <[email protected]>
Add SYS_ADMIN and SYS_CHROOT capability to make buildah work
in kind cluster

Signed-off-by: howard zhang <[email protected]>
Signed-off-by: howard zhang <[email protected]>
This label means the e2e tests are not suitable to test in
containerized k8s env (kind provider).

In this commit, we label two tests because the large size images
would lead to OOM of cdi-uploader. For more detail, please
refer to:
kubevirt#3577 (comment)

Signed-off-by: howard zhang <[email protected]>
The output of ls -ln for a file with 660 permissions can be either
-rw-rw----. or -rw-rw----

Signed-off-by: howard zhang <[email protected]>
@akalenyu
Copy link
Collaborator

akalenyu commented Jan 9, 2025

/approve
anything else needed? can this merge without the image push?

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akalenyu

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 9, 2025
@kubevirt-bot
Copy link
Contributor

@zhlhahaha: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-containerized-data-importer-e2e-ceph-wffc 4881c70 link true /test pull-containerized-data-importer-e2e-ceph-wffc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@zhlhahaha
Copy link
Contributor Author

anything else needed? can this merge without the image push?

Yes, we need a multi-architecture quay.io/kubevirt/fakeovirt image to enable us to run periodic e2e tests on Arm.

Currently, I’m working on two tasks:

  1. Writing a multi-architecture manifest build script for CDI, similar to the approach used in kubevirt/kubevirt. Once this is completed, we’ll be able to publish multi-architecture CDI images.
  2. Coordinating with my company to contribute an Arm64 server for the CDI CI/CD pipeline. This will allow us to integrate Arm64-related e2e and unit tests into the PR submission process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CDI on ARM
4 participants