Re: [PATCH 1/2] Add CI configuration for Kubernetes

[Daniel P . Berrangé]

On Fri, May 19, 2023 at 03:06:41PM +0100, Camilla Conte wrote:
> On Fri, May 19, 2023 at 1:51 PM Daniel P. Berrangé <berrange@redhat.com> 
> wrote:
> >
> > On Fri, May 19, 2023 at 01:33:50PM +0100, Camilla Conte wrote:
> > > On Fri, May 19, 2023 at 10:00 AM Daniel P. Berrangé <berrange@redhat.com> 
> > > wrote:
> > > >
> > > > On Fri, Apr 07, 2023 at 03:52:51PM +0100, Camilla Conte wrote:
> > > > > Configure Gitlab CI to run on Kubernetes
> > > > > according to the official documentation.
> > > > > https://docs.gitlab.com/ee/ci/docker/using_docker_build.html#docker-in-docker-with-tls-enabled-in-kubernetes
> > > > >
> > > > > These changes are needed because of the CI jobs
> > > > > using Docker-in-Docker (dind).
> > > > > As soon as Docker-in-Docker is replaced with Kaniko,
> > > > > these changes can be reverted.
> > > > >
> > > > > I documented what I did to set up the Kubernetes runner on the wiki:
> > > > > https://wiki.qemu.org/Testing/CI/KubernetesRunners
> > > > >
> > > > > Signed-off-by: Camilla Conte <cconte@redhat.com>
> > > > > ---
> > > > >  .gitlab-ci.d/container-template.yml |  6 +++---
> > > > >  .gitlab-ci.d/default.yml            |  3 +++
> > > > >  .gitlab-ci.d/opensbi.yml            |  8 +++-----
> > > > >  .gitlab-ci.d/qemu-project.yml       | 17 +++++++++++++++++
> > > > >  4 files changed, 26 insertions(+), 8 deletions(-)
> > > > >  create mode 100644 .gitlab-ci.d/default.yml
> > > > >
> > > > > diff --git a/.gitlab-ci.d/container-template.yml 
> > > > > b/.gitlab-ci.d/container-template.yml
> > > > > index 519b8a9482..f55a954741 100644
> > > > > --- a/.gitlab-ci.d/container-template.yml
> > > > > +++ b/.gitlab-ci.d/container-template.yml
> > > > > @@ -1,14 +1,14 @@
> > > > >  .container_job_template:
> > > > >    extends: .base_job_template
> > > > > -  image: docker:stable
> > > > > +  image: docker:20.10.16
> > > > >    stage: containers
> > > > >    services:
> > > > > -    - docker:dind
> > > > > +    - docker:20.10.16-dind
> > > > >    before_script:
> > > > >      - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
> > > > >      - export 
> > > > > COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
> > > > >      - apk add python3
> > > > > -    - docker info
> > > > > +    - until docker info; do sleep 1; done
> > > > >      - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p 
> > > > > "$CI_REGISTRY_PASSWORD"
> > > > >    script:
> > > > >      - echo "TAG:$TAG"
> > > > > diff --git a/.gitlab-ci.d/default.yml b/.gitlab-ci.d/default.yml
> > > > > new file mode 100644
> > > > > index 0000000000..292be8b91c
> > > > > --- /dev/null
> > > > > +++ b/.gitlab-ci.d/default.yml
> > > > > @@ -0,0 +1,3 @@
> > > > > +default:
> > > > > +  tags:
> > > > > +    - $RUNNER_TAG
> > > >
> > > > Can we just put this in base.yml instead of creating a new file.
> > >
> > > Sure.
> > >
> > > > > diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
> > > > > index 9a651465d8..5b0b47b57b 100644
> > > > > --- a/.gitlab-ci.d/opensbi.yml
> > > > > +++ b/.gitlab-ci.d/opensbi.yml
> > > > > @@ -42,17 +42,15 @@
> > > > >  docker-opensbi:
> > > > >    extends: .opensbi_job_rules
> > > > >    stage: containers
> > > > > -  image: docker:stable
> > > > > +  image: docker:20.10.16
> > > > >    services:
> > > > > -    - docker:stable-dind
> > > > > +    - docker:20.10.16-dind
> > > >
> > > > Can you elaborate on this ?  I know the docs about use that particular
> > > > version tag, but they don't appear to explain why. If this is not
> > > > actually a hard requirements, we should keep using the stable tag.
> > >
> > > Yes, we can keep using "stable".
> > > Then, we should be ready to address future issues that may arise from
> > > "stable" not being compatible with the runner.
> > >
> > > > >    variables:
> > > > >      GIT_DEPTH: 3
> > > > >      IMAGE_TAG: $CI_REGISTRY_IMAGE:opensbi-cross-build
> > > > > -    # We don't use TLS
> > > > > -    DOCKER_HOST: tcp://docker:2375
> > > > > -    DOCKER_TLS_CERTDIR: ""
> > > >
> > > > So IIUC, this was always redundant when using gitlab CI. We should just
> > > > remove these in a standalone commit.
> > >
> > > Okay, I'll put this in a separate commit.
> > >
> > > > >    before_script:
> > > > >      - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD 
> > > > > $CI_REGISTRY
> > > > > +    - until docker info; do sleep 1; done
> > > >
> > > > Was this really needed ?  The docs don't show that, and docker login is
> > > > synchronous, so I wouldn't expect us to them poll on 'docker info'.
> > >
> > > Unfortunately, yes. We need to wait until the "docker info" command is
> > > successful. This ensures that the Docker server has started and the
> > > subsequent docker commands won't fail.
> >
> > >
> > > > In container-template.yml we in fact do the reverse
> > > >
> > > >     - docker info
> > > >     - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p 
> > > > "$CI_REGISTRY_PASSWORD"
> > >
> > > About "docker login", as far as I understand it's a client-only
> > > command. It doesn't involve the Docker server at all. These two
> > > commands are not related to each other, it doesn't matter if "docker
> > > login" runs before or after "docker info".
> > >
> > > > imho best make this opensbi.yml file match contanier-template.yml, and
> > > > could be part of the same cleanup commit that removes thhose two docker
> > > > env vars.
> > >
> > > You mean to replace the "docker-opensbi" job in the "opensbi.yml" file
> > > with the same as the ".container_job_template" from the
> > > "container-template.yml" file?
> > > These two look too much different to me. I think we need to keep both.
> 
> > No, I didn't mean we have to merge them. Just that the 
> > container-template.yml
> > file merely does 'docker info' without any loop. So either that one is 
> > broken,
> > or using a loop in opensbi.yml is redundant.
> >
> > Assuming you've tested this series on k8s successfully, it would indicate
> > that the looping is not required, otherwise all the container jobs would
> > have failed.
> 
> Actually, I added the 'docker info' loop in the container-template.yml
> file too. Or am I missing your point?

No, I'm blind and missed that. So all is consistent already.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|