[PATCH 1/2] Add CI configuration for Kubernetes

[PATCH 1/2] Add CI configuration for Kubernetes

[Camilla Conte]

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
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
   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: ""
   before_script:
     - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY
+    - until docker info; do sleep 1; done
   script:
     - docker pull $IMAGE_TAG || true
     - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA
diff --git a/.gitlab-ci.d/qemu-project.yml b/.gitlab-ci.d/qemu-project.yml
index a7ed447fe4..57b175f5c2 100644
--- a/.gitlab-ci.d/qemu-project.yml
+++ b/.gitlab-ci.d/qemu-project.yml
@@ -1,7 +1,24 @@
 # This file contains the set of jobs run by the QEMU project:
 # https://gitlab.com/qemu-project/qemu/-/pipelines
 
+variables:
+  RUNNER_TAG: ""
+
+workflow:
+  rules:
+    # Set additional variables when running on Kubernetes.
+    # https://wiki.qemu.org/Testing/CI/KubernetesRunners
+    - if: $RUNNER_TAG == "k8s"
+      variables:
+        DOCKER_HOST: tcp://docker:2376
+        DOCKER_TLS_CERTDIR: "/certs"
+        DOCKER_TLS_VERIFY: 1
+        DOCKER_CERT_PATH: "$DOCKER_TLS_CERTDIR/client"
+    # Run the pipeline in other cases.
+    - when: always
+
 include:
+  - local: '/.gitlab-ci.d/default.yml'
   - local: '/.gitlab-ci.d/base.yml'
   - local: '/.gitlab-ci.d/stages.yml'
   - local: '/.gitlab-ci.d/opensbi.yml'
-- 
2.39.2

[PATCH 2/2] Raise crash-test-debian timeout to 90 minutes

[Camilla Conte]

When running on the Kubernetes runner, this CI job is timing out.
Raise the limit to give the job enough time to run.

Signed-off-by: Camilla Conte <cconte@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index ba6f551752..893f8c69b4 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -98,6 +98,7 @@ crash-test-debian:
   needs:
     - job: build-system-debian
       artifacts: true
+  timeout: 90m
   variables:
     IMAGE: debian-amd64
   script:
-- 
2.39.2

Re: [PATCH 2/2] Raise crash-test-debian timeout to 90 minutes

[Thomas Huth]

On 07/04/2023 16.52, Camilla Conte wrote:
> When running on the Kubernetes runner, this CI job is timing out.
> Raise the limit to give the job enough time to run.
> 
> Signed-off-by: Camilla Conte <cconte@redhat.com>
> ---
>   .gitlab-ci.d/buildtest.yml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index ba6f551752..893f8c69b4 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -98,6 +98,7 @@ crash-test-debian:
>     needs:
>       - job: build-system-debian
>         artifacts: true
> +  timeout: 90m
>     variables:
>       IMAGE: debian-amd64
>     script:

Eldon recently discovered that the crash-test-debian jobs takes longer on 
his runner since it has KVM enabled and the job tests the devices twice - 
once with TCG and once with KVM. I assume this is the case for the 
Kubernetes runner, too? ... we should likely add a switch to the 
device-crash-test script to provide a way to enforce running with one 
accelerator only, so we can keep the run time in a sane limit.

Anyway, for the time being, this patch is of course a fair work-around, thus:

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Daniel P. Berrangé]

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.

> 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.

>    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.

>    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'.

In container-template.yml we in fact do the reverse

    - docker info
    - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD"

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.

>    script:
>      - docker pull $IMAGE_TAG || true
>      - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA
> diff --git a/.gitlab-ci.d/qemu-project.yml b/.gitlab-ci.d/qemu-project.yml
> index a7ed447fe4..57b175f5c2 100644
> --- a/.gitlab-ci.d/qemu-project.yml
> +++ b/.gitlab-ci.d/qemu-project.yml
> @@ -1,7 +1,24 @@
>  # This file contains the set of jobs run by the QEMU project:
>  # https://gitlab.com/qemu-project/qemu/-/pipelines
>  
> +variables:
> +  RUNNER_TAG: ""
> +
> +workflow:
> +  rules:
> +    # Set additional variables when running on Kubernetes.
> +    # https://wiki.qemu.org/Testing/CI/KubernetesRunners
> +    - if: $RUNNER_TAG == "k8s"
> +      variables:
> +        DOCKER_HOST: tcp://docker:2376
> +        DOCKER_TLS_CERTDIR: "/certs"
> +        DOCKER_TLS_VERIFY: 1
> +        DOCKER_CERT_PATH: "$DOCKER_TLS_CERTDIR/client"

Is there anyway we can get the runner itself to set these
correctly by default ?

IMHO the ideal would be that the k8s runners are registerd with the
qemu project to run *any* jobs without requiring tags. That way the
runners will "just work" when share runners are unavailable/exhausted,
like we have with Eldon's runner

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 :|

Re: [PATCH 2/2] Raise crash-test-debian timeout to 90 minutes

[Daniel P. Berrangé]

On Tue, Apr 11, 2023 at 10:22:51AM +0200, Thomas Huth wrote:
> On 07/04/2023 16.52, Camilla Conte wrote:
> > When running on the Kubernetes runner, this CI job is timing out.
> > Raise the limit to give the job enough time to run.
> > 
> > Signed-off-by: Camilla Conte <cconte@redhat.com>
> > ---
> >   .gitlab-ci.d/buildtest.yml | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> > index ba6f551752..893f8c69b4 100644
> > --- a/.gitlab-ci.d/buildtest.yml
> > +++ b/.gitlab-ci.d/buildtest.yml
> > @@ -98,6 +98,7 @@ crash-test-debian:
> >     needs:
> >       - job: build-system-debian
> >         artifacts: true
> > +  timeout: 90m
> >     variables:
> >       IMAGE: debian-amd64
> >     script:
> 
> Eldon recently discovered that the crash-test-debian jobs takes longer on
> his runner since it has KVM enabled and the job tests the devices twice -
> once with TCG and once with KVM. I assume this is the case for the
> Kubernetes runner, too? ... we should likely add a switch to the
> device-crash-test script to provide a way to enforce running with one
> accelerator only, so we can keep the run time in a sane limit.
> 
> Anyway, for the time being, this patch is of course a fair work-around, thus:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

You added --tcg-only to the test a little while after your review here,
so hopefully this patch can be dropped.

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 :|

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

[Camilla Conte]

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.

> >    script:
> >      - docker pull $IMAGE_TAG || true
> >      - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA
> > diff --git a/.gitlab-ci.d/qemu-project.yml b/.gitlab-ci.d/qemu-project.yml
> > index a7ed447fe4..57b175f5c2 100644
> > --- a/.gitlab-ci.d/qemu-project.yml
> > +++ b/.gitlab-ci.d/qemu-project.yml
> > @@ -1,7 +1,24 @@
> >  # This file contains the set of jobs run by the QEMU project:
> >  # https://gitlab.com/qemu-project/qemu/-/pipelines
> >
> > +variables:
> > +  RUNNER_TAG: ""
> > +
> > +workflow:
> > +  rules:
> > +    # Set additional variables when running on Kubernetes.
> > +    # https://wiki.qemu.org/Testing/CI/KubernetesRunners
> > +    - if: $RUNNER_TAG == "k8s"
> > +      variables:
> > +        DOCKER_HOST: tcp://docker:2376
> > +        DOCKER_TLS_CERTDIR: "/certs"
> > +        DOCKER_TLS_VERIFY: 1
> > +        DOCKER_CERT_PATH: "$DOCKER_TLS_CERTDIR/client"
>
> Is there anyway we can get the runner itself to set these
> correctly by default ?

Yes, the runner can set environment variables for its jobs.

My concern here is that over time we lose visibility of these
customizations if we put them in the runner configuration.
This can be solved by having a repo in the qemu-project namespace to
host the runner configuration, something like I did here:
https://gitlab.com/spotlesstofu/qemu-ci-kubernetes.

> IMHO the ideal would be that the k8s runners are registerd with the
> qemu project to run *any* jobs without requiring tags. That way the
> runners will "just work" when share runners are unavailable/exhausted,
> like we have with Eldon's runner

The problem here is that the Kubernetes (k8s) runner can't run windows
jobs at the moment. If we wait for the shared runners to be exhausted,
those few windows jobs in the pipeline won't be able to run.


> 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 :|
>

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

[Daniel P. Berrangé]

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.

> > >    script:
> > >      - docker pull $IMAGE_TAG || true
> > >      - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA
> > > diff --git a/.gitlab-ci.d/qemu-project.yml b/.gitlab-ci.d/qemu-project.yml
> > > index a7ed447fe4..57b175f5c2 100644
> > > --- a/.gitlab-ci.d/qemu-project.yml
> > > +++ b/.gitlab-ci.d/qemu-project.yml
> > > @@ -1,7 +1,24 @@
> > >  # This file contains the set of jobs run by the QEMU project:
> > >  # https://gitlab.com/qemu-project/qemu/-/pipelines
> > >
> > > +variables:
> > > +  RUNNER_TAG: ""
> > > +
> > > +workflow:
> > > +  rules:
> > > +    # Set additional variables when running on Kubernetes.
> > > +    # https://wiki.qemu.org/Testing/CI/KubernetesRunners
> > > +    - if: $RUNNER_TAG == "k8s"
> > > +      variables:
> > > +        DOCKER_HOST: tcp://docker:2376
> > > +        DOCKER_TLS_CERTDIR: "/certs"
> > > +        DOCKER_TLS_VERIFY: 1
> > > +        DOCKER_CERT_PATH: "$DOCKER_TLS_CERTDIR/client"
> >
> > Is there anyway we can get the runner itself to set these
> > correctly by default ?
> 
> Yes, the runner can set environment variables for its jobs.
> 
> My concern here is that over time we lose visibility of these
> customizations if we put them in the runner configuration.
> This can be solved by having a repo in the qemu-project namespace to
> host the runner configuration, something like I did here:
> https://gitlab.com/spotlesstofu/qemu-ci-kubernetes.

Cleber put configs for the current QEMU private runners into
the main qemu.git at scripts/ci/setup/  We should have any
setup for the k8s runner somewhere nearby too. Or move all
of it out into a separate repository.

> > IMHO the ideal would be that the k8s runners are registerd with the
> > qemu project to run *any* jobs without requiring tags. That way the
> > runners will "just work" when share runners are unavailable/exhausted,
> > like we have with Eldon's runner
> 
> The problem here is that the Kubernetes (k8s) runner can't run windows
> jobs at the moment. If we wait for the shared runners to be exhausted,
> those few windows jobs in the pipeline won't be able to run.

Hmm, that's awkward. I'm not convinced we should be expecting the maintainer
doing staging builds to decide whether or not to the set tag when running a
pipeline. I guess maybe we can set 'RUNNER_TAG' in the web UI settings for
CI, to turn it on globally for qemu.git upstream.

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 :|

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

[Camilla Conte]

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?

> > > >    script:
> > > >      - docker pull $IMAGE_TAG || true
> > > >      - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA
> > > > diff --git a/.gitlab-ci.d/qemu-project.yml b/.gitlab-ci.d/qemu-project.yml
> > > > index a7ed447fe4..57b175f5c2 100644
> > > > --- a/.gitlab-ci.d/qemu-project.yml
> > > > +++ b/.gitlab-ci.d/qemu-project.yml
> > > > @@ -1,7 +1,24 @@
> > > >  # This file contains the set of jobs run by the QEMU project:
> > > >  # https://gitlab.com/qemu-project/qemu/-/pipelines
> > > >
> > > > +variables:
> > > > +  RUNNER_TAG: ""
> > > > +
> > > > +workflow:
> > > > +  rules:
> > > > +    # Set additional variables when running on Kubernetes.
> > > > +    # https://wiki.qemu.org/Testing/CI/KubernetesRunners
> > > > +    - if: $RUNNER_TAG == "k8s"
> > > > +      variables:
> > > > +        DOCKER_HOST: tcp://docker:2376
> > > > +        DOCKER_TLS_CERTDIR: "/certs"
> > > > +        DOCKER_TLS_VERIFY: 1
> > > > +        DOCKER_CERT_PATH: "$DOCKER_TLS_CERTDIR/client"
> > >
> > > Is there anyway we can get the runner itself to set these
> > > correctly by default ?
> >
> > Yes, the runner can set environment variables for its jobs.
> >
> > My concern here is that over time we lose visibility of these
> > customizations if we put them in the runner configuration.
> > This can be solved by having a repo in the qemu-project namespace to
> > host the runner configuration, something like I did here:
> > https://gitlab.com/spotlesstofu/qemu-ci-kubernetes.

> Cleber put configs for the current QEMU private runners into
> the main qemu.git at scripts/ci/setup/  We should have any
> setup for the k8s runner somewhere nearby too. Or move all
> of it out into a separate repository.

Okay, I'll add a patch to put the runner configuration near scripts/ci/setup/.

> > > IMHO the ideal would be that the k8s runners are registerd with the
> > > qemu project to run *any* jobs without requiring tags. That way the
> > > runners will "just work" when share runners are unavailable/exhausted,
> > > like we have with Eldon's runner
> >
> > The problem here is that the Kubernetes (k8s) runner can't run windows
> > jobs at the moment. If we wait for the shared runners to be exhausted,
> > those few windows jobs in the pipeline won't be able to run.

> Hmm, that's awkward. I'm not convinced we should be expecting the maintainer
> doing staging builds to decide whether or not to the set tag when running a
> pipeline. I guess maybe we can set 'RUNNER_TAG' in the web UI settings for
> CI, to turn it on globally for qemu.git upstream.

Setting the RUNNER_TAG variable in the web UI would work.

Someone would have to switch (remove) the RUNNER_TAG variable whenever
we want to use the shared runners instead of the Kubernetes runner.
This variable change could probably be automated to switch runners
when a certain amount of remaining shared runners minutes is reached.


> 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 :|
>

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 :|