Add basic e2e tests for UBI image #5764

xliuxu · 2023-12-01T03:40:55Z

Add basic e2e tests for UBI image
Run basic e2e tests on a Kind cluster for the UBI image to make sure the UBI image can be deployed as expected. Add a new input parameter distro to the conformance workflow to allow testing of the UBI image.

Fixes: Add a simple CI sanity check for antrea/antrea-ubi image #5729

.github/workflows/kind.yml

.github/workflows/conformance.yml

.github/workflows/build.yml

antoninbas · 2024-01-29T22:21:38Z

.github/workflows/build.yml

-      env:
-        DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }}
-        DOCKER_PASSWORD: ${{ secrets.DOCKER_PASSWORD }}
+    - name: Install Kind


I am OK with running the basic kind test as part of this workflow even though it is not ideal. However, we should probably add a comment explaining why we are doing this (avoid an extra UBI build in a separate workflow)

I also don't think this is elegant. Should we move the test part out as a separate job?

I think we can move it to kind_ubi.yml, even if that means that the UBI image is built twice (once in build.yml, once in kind_ubi.yml), which is the same as for the Ubuntu image
In the future, we may want to avoid building these images in build.yml for PRs, since there is no point in building them twice (we will still build it for Github push events, since we need to push the new images to the registry in that case). But this should probably be done in a separate PR, and not this one.

How about we make it daily/bidaily job to avoid adding extra time to very basic jobs? I'm more concerned more tests would be added like this if there is a precedent. We could potentially move more rarely failed but long running tests to daily/bidaily job.

In the original issue (#5729) I suggested adding a very basic job just to make sure that the UBI-based image is not broken in a pretty obvious way (like it was around 1.14). I was hoping impact on PR test time would be minimal.
One issue that we have today is that even if someone modifies the UBI Dockerfile, we do not check anything besides making sure that the image still builds.

I am not a big fan of the daily jobs, as they can be broken for a long time before someone actually starts looking into the failure.

I think if we go with my suggestion above: skip building the UBI image in build.yml for PRs, and build the image + sanity check it in kind_ubi.yml, the impact on test time will be minimal. The sanity check itself should only take a couple of minutes. Additionally, after I fix #5944, we should see build time reduction for most jobs.

I also think that we should add a trigger to conformance.yml so that we run the K8s conformance tests (at least for the UBI image) for every release PR.

antoninbas · 2024-01-29T22:27:17Z

.github/workflows/conformance.yml

          fi
      - name: Build Antrea image if required
        if: ${{ steps.check-release.outputs.released == 'false' }}
        run: |
-          ./hack/build-antrea-linux-all.sh --pull
+          if [ ${{ inputs.antrea-controller-image-repository }} == 'antrea/antrea-controller-ubi' ] || [ ${{ inputs.antrea-agent-image-repository }} == 'antrea/antrea-agent-ubi' ]; then


I think I gave you a bad suggestion earlier, that's my bad. The new version doesn't make sense:

it doesn't support testing older minor versions of Antrea (pre image split)

it doesn't make sense when we need to build the image

So I think your earlier idea of having distribution workflow parameter was definitely better.

Thanks for the insights. I will also revert this.

tnqn · 2024-02-01T10:29:53Z

.github/workflows/build.yml

-      env:
-        DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }}
-        DOCKER_PASSWORD: ${{ secrets.DOCKER_PASSWORD }}
+    - name: Install Kind


How about we make it daily/bidaily job to avoid adding extra time to very basic jobs? I'm more concerned more tests would be added like this if there is a precedent. We could potentially move more rarely failed but long running tests to daily/bidaily job.

tnqn · 2024-02-01T10:31:48Z

.github/workflows/conformance.yml

+        type: choice
+        options:
+          - ubuntu
+          - ubi


I didn't know this before, so ubi image can also run on regular clusters?

yes, it should not be a problem

Yes. The only difference I noticed is that UBI images only have nf tables available. So it should be OK to test UBI images if the cluster is bootstrapped in nft mode

That's a good point. Actually the iptables-wrapper repo has a note about this:

RHEL / CentOS / UBI

RHEL 7 ships iptables 1.4, which does not support nft mode. RHEL 8 ships a hacked version of iptables 1.8 that only supports nft mode. Therefore, neither can be used as a basis for a portable iptables-using container image.

I wonder if UBI 9 is different

9.0_release_notes indicates that iptables-nft has been marked as deprecated. But I think it is safe to use them in UBI 9.

tnqn · 2024-02-01T10:33:09Z

.github/workflows/conformance.yml

      - name: Install Kind
        run: |
-          KIND_VERSION=$(head -n1 ./ci/kind/version)
+          KIND_VERSION=$(head -n1 ./ci/kind/version || echo v0.20.0)


thanks for fixing it, I had to switch to a tag when testing previous releases.

luolanzone · 2024-02-04T06:27:07Z

.github/workflows/build.yml

+        curl -Lo ./kind https://github.com/kubernetes-sigs/kind/releases/download/${KIND_VERSION}/kind-$(uname)-amd64
+        chmod +x ./kind
+        sudo mv kind /usr/local/bin
+    - name: Run basic e2e tests


I would agree Antonin to put this kind of e2e test to a file kind_ubi.yml which should be similar like kind.yml.

Created #6031 to track the follow-up issue.

.github/workflows/conformance.yml

antoninbas · 2024-02-28T17:35:12Z

.github/workflows/build.yml

@@ -91,6 +99,29 @@ jobs:
        docker push antrea/antrea-ubi:latest
        docker push antrea/antrea-agent-ubi:latest
        docker push antrea/antrea-controller-ubi:latest
+    - name: Install Kind


It's a bit strange to test the image after pushing it to the registry, but as long as we address #6031 promptly that works for me

Thanks. I can follow up with #6031 after this PR.

antoninbas · 2024-02-28T17:46:00Z

ci/kind/test-e2e-kind.sh

-    COMMON_IMAGES_LIST+=("antrea/antrea-agent-ubuntu-coverage:latest" \
-                         "antrea/antrea-controller-ubuntu-coverage:latest")
+    COMMON_IMAGES_LIST+=("${antrea_controller_image}-coverage:${antrea_image_tag}" \
+                         "${antrea_agent_image}-coverage:${antrea_image_tag}")


it's a bit weird to append the -coverage suffix automatically when a user provides an image names.
I don't have a great solution, but I think we should fail the script if the --coverage flag is used with --antrea-controller-image or --antrea-agent-image.

Added simple checks to make sure image names cannot be specified with coverage enabled.

antoninbas · 2024-03-01T20:16:04Z

@xliuxu Something weird is going on and it seems the new job keeps getting stuck in Github Actions, even after a second try. Could you take a look?

xliuxu · 2024-03-04T08:26:51Z

@xliuxu Something weird is going on and it seems the new job keeps getting stuck in Github Actions, even after a second try. Could you take a look?

I ran more tests at #6053 and suspect that the issue is related to insufficient disk space. The job can succeed if we clean up the build caches after building the image.
https://github.com/antrea-io/antrea/actions/runs/8137117739/job/22234796248
I think splitting the job into different stages and running them on multiple runners can also resolve this issue.

antoninbas · 2024-03-04T18:17:35Z

@xliuxu Something weird is going on and it seems the new job keeps getting stuck in Github Actions, even after a second try. Could you take a look?

I ran more tests at #6053 and suspect that the issue is related to insufficient disk space. The job can succeed if we clean up the build caches after building the image. https://github.com/antrea-io/antrea/actions/runs/8137117739/job/22234796248 I think splitting the job into different stages and running them on multiple runners can also resolve this issue.

I think pruning the build cache after building the image is sufficient for now, since we are planning to improve things in #6031 anyway

Run basic e2e tests on a Kind cluster for the UBI image to make sure the UBI image can be deployed as expected. Add a new input parameter `distro` to the conformance workflow to allow testing of the UBI image. Fixes: antrea-io#5729 Signed-off-by: Xu Liu <xliu2@vmware.com>

antoninbas · 2024-03-08T03:52:26Z

/skip-all

Run basic e2e tests on a Kind cluster for the UBI image to make sure the UBI image can be deployed as expected. Add a new input parameter `distro` to the conformance workflow to allow testing of the UBI image. Fixes: antrea-io#5729 Signed-off-by: Xu Liu <xliu2@vmware.com>

xliuxu force-pushed the xliu2-sanity-check-ubi branch 3 times, most recently from dfd3cb9 to b846ece Compare December 1, 2023 08:15

xliuxu force-pushed the xliu2-sanity-check-ubi branch 11 times, most recently from 3507aa0 to 56fff97 Compare January 25, 2024 05:56

xliuxu changed the title ~~[Not For Review] Add sanity checks for UBI image in github actions~~ Add basic e2e tests for UBI image Jan 25, 2024

xliuxu force-pushed the xliu2-sanity-check-ubi branch from 56fff97 to 5e1abf6 Compare January 25, 2024 05:58

xliuxu marked this pull request as ready for review January 25, 2024 05:59

xliuxu force-pushed the xliu2-sanity-check-ubi branch from 5e1abf6 to 5b3e126 Compare January 25, 2024 06:00

xliuxu requested review from antoninbas and luolanzone January 25, 2024 06:01

antoninbas reviewed Jan 25, 2024

View reviewed changes

.github/workflows/kind.yml Outdated Show resolved Hide resolved

.github/workflows/kind.yml Outdated Show resolved Hide resolved

.github/workflows/conformance.yml Outdated Show resolved Hide resolved

xliuxu force-pushed the xliu2-sanity-check-ubi branch 8 times, most recently from 930025c to 7ccb2d3 Compare January 29, 2024 06:47

antoninbas reviewed Jan 29, 2024

View reviewed changes

xliuxu force-pushed the xliu2-sanity-check-ubi branch 4 times, most recently from 2693fa5 to 300729a Compare February 1, 2024 08:25

tnqn reviewed Feb 1, 2024

View reviewed changes

luolanzone reviewed Feb 4, 2024

View reviewed changes

xliuxu force-pushed the xliu2-sanity-check-ubi branch from 300729a to aa8b0b4 Compare February 28, 2024 02:55

xliuxu mentioned this pull request Feb 28, 2024

Add separate CI job for UBI images #6031

Closed

antoninbas reviewed Feb 28, 2024

View reviewed changes

xliuxu force-pushed the xliu2-sanity-check-ubi branch from aa8b0b4 to 097f0e1 Compare February 29, 2024 12:54

antoninbas previously approved these changes Feb 29, 2024

View reviewed changes

xliuxu dismissed antoninbas’s stale review via 501637e March 5, 2024 02:25

xliuxu force-pushed the xliu2-sanity-check-ubi branch from 097f0e1 to 501637e Compare March 5, 2024 02:25

xliuxu requested review from antoninbas and luolanzone March 8, 2024 02:22

antoninbas approved these changes Mar 8, 2024

View reviewed changes

antoninbas merged commit ac8aa95 into antrea-io:main Mar 8, 2024
51 of 55 checks passed

tnqn mentioned this pull request Mar 11, 2024

build-ubi keeps failing on basic e2e tests #6095

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic e2e tests for UBI image #5764

Add basic e2e tests for UBI image #5764

xliuxu commented Dec 1, 2023 •

edited

Loading

antoninbas Jan 29, 2024

xliuxu Jan 31, 2024

antoninbas Jan 31, 2024 •

edited

Loading

tnqn Feb 1, 2024

antoninbas Feb 1, 2024

antoninbas Jan 29, 2024

xliuxu Jan 31, 2024

tnqn Feb 1, 2024

tnqn Feb 1, 2024

antoninbas Feb 1, 2024

xliuxu Feb 2, 2024

antoninbas Feb 2, 2024 •

edited

Loading

xliuxu Feb 2, 2024

tnqn Feb 1, 2024

luolanzone Feb 4, 2024

xliuxu Feb 28, 2024

antoninbas Feb 28, 2024

xliuxu Feb 29, 2024

antoninbas Feb 28, 2024

xliuxu Feb 29, 2024

antoninbas commented Mar 1, 2024

xliuxu commented Mar 4, 2024

antoninbas commented Mar 4, 2024

antoninbas commented Mar 8, 2024

Add basic e2e tests for UBI image #5764

Add basic e2e tests for UBI image #5764

Conversation

xliuxu commented Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas commented Mar 1, 2024

xliuxu commented Mar 4, 2024

antoninbas commented Mar 4, 2024

antoninbas commented Mar 8, 2024

xliuxu commented Dec 1, 2023 •

edited

Loading

antoninbas Jan 31, 2024 •

edited

Loading

antoninbas Feb 2, 2024 •

edited

Loading