On Wed, Nov 15, 2023 at 1:53 PM Dumitru Ceara <[email protected]> wrote:

> On 11/15/23 13:40, Ales Musil wrote:
> > On Wed, Nov 15, 2023 at 12:58 PM Dumitru Ceara <[email protected]>
> wrote:
> >
> >> That allows us to use different distro bases for different stable
> >> branches.
> >>
> >> Fixes: 60a53abaa38a ("ci: Add automation for building the containers")
> >> Signed-off-by: Dumitru Ceara <[email protected]>
> >>
> >
> > Hi Dumitru,
> >
> > thank you for the series.
> >
> >
>
> Thanks for the review!
>
> >> ---
> >>  .ci/ci.sh                        |    9 ++++++++-
> >>  .cirrus.yml                      |    2 +-
> >>  .github/workflows/containers.yml |   13 ++++++++++---
> >>  .github/workflows/test.yml       |    7 ++++++-
> >>  utilities/containers/Makefile    |    4 ++--
> >>  5 files changed, 27 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/.ci/ci.sh b/.ci/ci.sh
> >> index 3f1b41eadc..ccec572c3a 100755
> >> --- a/.ci/ci.sh
> >> +++ b/.ci/ci.sh
> >> @@ -20,7 +20,8 @@ DPDK_PATH=${DPDK_PATH:-$OVN_PATH/dpdk-dir}
> >>  CONTAINER_CMD=${CONTAINER_CMD:-podman}
> >>  CONTAINER_WORKSPACE="/workspace"
> >>  CONTAINER_WORKDIR="/workspace/ovn-tmp"
> >> -IMAGE_NAME=${IMAGE_NAME:-"ovn-org/ovn-tests"}
> >> +DEFAULT_IMAGE_NAME="ovn-org/ovn-tests:main-fedora"
> >> +IMAGE_NAME=${IMAGE_NAME:-${DEFAULT_IMAGE_NAME}}
> >>
> >>  # Test variables
> >>  ARCH=${ARCH:-$(uname -m)}
> >> @@ -167,6 +168,12 @@ if [ -z "$DPDK" ]; then
> >>     mkdir -p "$DPDK_PATH"
> >>  fi
> >>
> >> +# Check if the IMAGE_NAME is a real image we can pull, otherwise fall
> >> +# back to DEFAULT_IMAGE_NAME.
> >> +if ! $CONTAINER_CMD pull $IMAGE_NAME; then
> >> +    IMAGE_NAME=$DEFAULT_IMAGE_NAME
> >> +fi
> >> +
> >>
> >
> > We shouldn't silently fall back to something different if the image
> doesn't
> > exist. This should be solved on a workflow level most likely. We can even
> > avoid specifying the IMAGE_NAME if we tag the downloaded image to
> > "ovn-org/ovn-tests", but that's just nit.
> >
>
> Makes sense, it's better to be explicit, I'll do it like that.
>
> >
> >>  CONTAINER_ID="$($CONTAINER_CMD run --privileged -d \
> >>      --pids-limit=-1 \
> >>      --env ASAN_OPTIONS=$ASAN_OPTIONS \
> >> diff --git a/.cirrus.yml b/.cirrus.yml
> >> index bd4cd08aaf..23e720cc47 100644
> >> --- a/.cirrus.yml
> >> +++ b/.cirrus.yml
> >> @@ -1,7 +1,7 @@
> >>  arm_unit_tests_task:
> >>
> >>    arm_container:
> >> -    image: ghcr.io/ovn-org/ovn-tests:fedora
> >> +    image: ghcr.io/ovn-org/ovn-tests:main-fedora
> >>      memory: 4G
> >>      cpu: 2
> >>
> >> diff --git a/.github/workflows/containers.yml
> >> b/.github/workflows/containers.yml
> >> index 57e815ed86..40a44a7c41 100644
> >> --- a/.github/workflows/containers.yml
> >> +++ b/.github/workflows/containers.yml
> >> @@ -7,8 +7,6 @@ on:
> >>      - cron:  '0 0 * * 1'
> >>
> >>  env:
> >> -  IMAGE_REGISTRY: ghcr.io
> >
> >
> > nit: Unrelated change, it is the same as before.
> >
>
> Actually, it's on purpose.  I wanted to define all image related
> variables in a single place.  If you want I can move IMAGE_NAME lower too.
>
> I'll wait for your input before sending v2.
>

Alright, let's move IMAGE_NAME too then.


>
> >
> >>
> >> -  IMAGE_NAMESPACE: ovn-org
> >>    IMAGE_NAME: ovn-tests
> >>    CONTAINERS_PATH: ./utilities/containers
> >>    DEPENDENCIES: podman
> >> @@ -31,13 +29,22 @@ jobs:
> >>        - name: Set up QEMU
> >>          uses: docker/setup-qemu-action@v2
> >>
> >> +      - name: Build image name
> >> +        run: |
> >> +          echo
> >> "IMAGE_BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}" \
> >> +            >> $GITHUB_ENV
> >> +          echo "IMAGE_REGISTRY=ghcr.io" \
> >> +            >> $GITHUB_ENV
> >> +          echo "IMAGE_NAMESPACE=$GITHUB_REPOSITORY_OWNER" \
> >> +            >> $GITHUB_ENV
> >> +
> >>        - name: Build container images
> >>          id: build-image
> >>          uses: redhat-actions/buildah-build@v2
> >>          with:
> >>            image: ${{ env.IMAGE_NAME }}
> >>            archs: amd64, arm64
> >> -          tags: ${{ matrix.distro }}
> >> +          tags: ${{ env.IMAGE_BRANCH }}-${{ matrix.distro }}
> >>            build-args: CONTAINERS_PATH=${{ env.CONTAINERS_PATH }}
> >>            dockerfiles: ${{ env.CONTAINERS_PATH }}/${{ matrix.distro
> >> }}/Dockerfile
> >>
> >> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> >> index 5c5ce6ed10..84b358ee46 100644
> >> --- a/.github/workflows/test.yml
> >> +++ b/.github/workflows/test.yml
> >> @@ -83,7 +83,6 @@ jobs:
> >>    build-linux:
> >>      needs: build-dpdk
> >>      env:
> >> -      IMAGE_NAME:  ghcr.io/ovn-org/ovn-tests:ubuntu
> >>        ARCH:        ${{ matrix.cfg.arch }}
> >>        CC:          ${{ matrix.cfg.compiler }}
> >>        DPDK:        ${{ matrix.cfg.dpdk }}
> >> @@ -128,6 +127,12 @@ jobs:
> >>        with:
> >>          submodules: recursive
> >>
> >> +    - name: Build image name
> >> +      run: |
> >> +        IMAGE_BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}
> >> +        echo "IMAGE_NAME=
> >> ghcr.io/$GITHUB_REPOSITORY_OWNER/ovn-tests:${IMAGE_BRANCH}-ubuntu
> <http://ghcr.io/$GITHUB_REPOSITORY_OWNER/ovn-tests:$%7BIMAGE_BRANCH%7D-ubuntu>
> >> <
> http://ghcr.io/$GITHUB_REPOSITORY_OWNER/ovn-tests:$%7BIMAGE_BRANCH%7D-ubuntu
> >"
> >> \
> >> +          >> $GITHUB_ENV
> >> +
> >>      # For weekly runs, don't update submodules
> >>      - name: checkout without submodule
> >>        if: github.event_name == 'schedule'
> >> diff --git a/utilities/containers/Makefile
> b/utilities/containers/Makefile
> >> index d79e4ad5ee..e2e8f95064 100644
> >> --- a/utilities/containers/Makefile
> >> +++ b/utilities/containers/Makefile
> >> @@ -1,5 +1,5 @@
> >>  CONTAINER_CMD ?= podman
> >> -IMAGE_NAME ?= "ovn-org/ovn-tests"
> >> +IMAGE_NAME ?= "ovn-org/ovn-tests:main"
> >>  CONTAINERS_PATH ?= "."
> >>
> >>  distros := fedora ubuntu
> >> @@ -7,5 +7,5 @@ distros := fedora ubuntu
> >>  .PHONY: $(distros)
> >>
> >>  $(distros):
> >> -       $(CONTAINER_CMD) build --no-cache --rm -t $(IMAGE_NAME):$@ \
> >> +       $(CONTAINER_CMD) build --no-cache --rm -t $(IMAGE_NAME)-$@ \
> >>         -f $@/Dockerfile .
> --build-arg=CONTAINERS_PATH=$(CONTAINERS_PATH)
> >>
> >>
> >
> > Thanks,
> > Ales
> >
> >
>
> Thanks,
> Dumitru
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to