| Message ID | 20211026145509.1029274-2-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gitlab-ci: Only push docker images to mainstream registry from /master | expand |
On Tue, Oct 26, 2021 at 11:55 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Users expect images pulled from registry.gitlab.com/qemu-project/qemu/ > to be stable. QEMU repository workflow pushes merge candidates to > the /staging branch, and on success the same commit is pushed as > /master. If /staging fails, we do not want to push the built images > to the registry. Therefore limit the 'docker push' command to the > /master branch on the mainstream CI. The fork behavior is unchanged. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > .gitlab-ci.d/container-template.yml | 11 ++++++++++- > .gitlab-ci.d/edk2.yml | 11 ++++++++++- > .gitlab-ci.d/opensbi.yml | 11 ++++++++++- > 3 files changed, 30 insertions(+), 3 deletions(-) > Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On 26/10/2021 16.55, Philippe Mathieu-Daudé wrote: > Users expect images pulled from registry.gitlab.com/qemu-project/qemu/ > to be stable. QEMU repository workflow pushes merge candidates to > the /staging branch, and on success the same commit is pushed as > /master. If /staging fails, we do not want to push the built images > to the registry. Therefore limit the 'docker push' command to the > /master branch on the mainstream CI. The fork behavior is unchanged. Hmmm, what if I have a patch series that updates one of the containers and then also contains a new test that depends on the updated container? Won't that fail in the staging branch now and make me look bad? Thomas
On Wed, Oct 27, 2021 at 08:01:07AM +0200, Thomas Huth wrote: > On 26/10/2021 16.55, Philippe Mathieu-Daudé wrote: > > Users expect images pulled from registry.gitlab.com/qemu-project/qemu/ > > to be stable. QEMU repository workflow pushes merge candidates to > > the /staging branch, and on success the same commit is pushed as > > /master. If /staging fails, we do not want to push the built images > > to the registry. Therefore limit the 'docker push' command to the > > /master branch on the mainstream CI. The fork behavior is unchanged. > > Hmmm, what if I have a patch series that updates one of the containers and > then also contains a new test that depends on the updated container? Won't > that fail in the staging branch now and make me look bad? Yep, if a patch series contains a dockerfile change we *must* run the container build no matter what, so tis patch doesn't fly. This scenario a tricky problem, and I'm not seeing an easy answer to it so far. Regards, Daniel
Hi Thomas, On 10/27/21 08:01, Thomas Huth wrote: > On 26/10/2021 16.55, Philippe Mathieu-Daudé wrote: >> Users expect images pulled from registry.gitlab.com/qemu-project/qemu/ >> to be stable. QEMU repository workflow pushes merge candidates to >> the /staging branch, and on success the same commit is pushed as >> /master. If /staging fails, we do not want to push the built images >> to the registry. Therefore limit the 'docker push' command to the >> /master branch on the mainstream CI. The fork behavior is unchanged. > > Hmmm, what if I have a patch series that updates one of the containers > and then also contains a new test that depends on the updated container? > Won't that fail in the staging branch now and make me look bad? Good point. My understanding is: - All tests based on Docker containers pull from DOCKER_DEFAULT_REGISTRY (see tests/docker/Makefile.include). These tests can be run on CI but are principally run locally, using the ':latest' tag, which is the reason we don't want to push invalid / oudated images. - CI tests also use the Docker containers. We might want to test freshly built images. Here we should explicit the use of local tag (without using the registry prefix) in this case. Otherwise default to the latest from registry (stable). So the problem you mentioned is the second case, and should be reworked in the YAML. I will revisit the overall CI YAML for your case, but the first case seems equally important. Regards, Phil.
Hi, > This scenario a tricky problem, and I'm not seeing an easy answer to it > so far. Only exclude stable branches? take care, Gerd
On Wed, Oct 27, 2021 at 01:35:25PM +0200, Gerd Hoffmann wrote: > Hi, > > > This scenario a tricky problem, and I'm not seeing an easy answer to it > > so far. > > Only exclude stable branches? The stable branches issue is just one of the bugs - the bigger bug is that we're publishing containers in the qemu-project registry from the 'staging' branch pipelines, even if the pipeline ultimately fails. Regards, Daniel
diff --git a/.gitlab-ci.d/container-template.yml b/.gitlab-ci.d/container-template.yml index 1baecd94606..0a736644b22 100644 --- a/.gitlab-ci.d/container-template.yml +++ b/.gitlab-ci.d/container-template.yml @@ -16,6 +16,15 @@ -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker" -r $CI_REGISTRY/qemu-project/qemu - docker tag "qemu/$NAME" "$TAG" - - docker push "$TAG" + # On mainstream CI, we only want to push images on the master branch, + # so skip the other cases (tag or non-master branch). + - if test "$CI_PROJECT_NAMESPACE" = "qemu-project" && + test -n "$CI_COMMIT_TAG" + -o "$CI_COMMIT_BRANCH" != "$CI_DEFAULT_BRANCH"; + then + :; + else + docker push "$TAG"; + fi after_script: - docker logout diff --git a/.gitlab-ci.d/edk2.yml b/.gitlab-ci.d/edk2.yml index 13d0f8b019f..e15f80f4874 100644 --- a/.gitlab-ci.d/edk2.yml +++ b/.gitlab-ci.d/edk2.yml @@ -33,7 +33,16 @@ docker-edk2: - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA --tag $IMAGE_TAG .gitlab-ci.d/edk2 - docker push $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA - - docker push $IMAGE_TAG + # On mainstream CI, we only want to push images on the master branch, + # so skip the other cases (tag or non-master branch). + - if test "$CI_PROJECT_NAMESPACE" = "qemu-project" && + test -n "$CI_COMMIT_TAG" + -o "$CI_COMMIT_BRANCH" != "$CI_DEFAULT_BRANCH"; + then + :; + else + docker push "$IMAGE_TAG"; + fi build-edk2: extends: .edk2_job_rules diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml index 5e0a2477c5d..a38ccbe5baa 100644 --- a/.gitlab-ci.d/opensbi.yml +++ b/.gitlab-ci.d/opensbi.yml @@ -34,7 +34,16 @@ docker-opensbi: - docker build --cache-from $IMAGE_TAG --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA --tag $IMAGE_TAG .gitlab-ci.d/opensbi - docker push $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA - - docker push $IMAGE_TAG + # On mainstream CI, we only want to push images on the master branch, + # so skip the other cases (tag or non-master branch). + - if test "$CI_PROJECT_NAMESPACE" = "qemu-project" && + test -n "$CI_COMMIT_TAG" + -o "$CI_COMMIT_BRANCH" != "$CI_DEFAULT_BRANCH"; + then + :; + else + docker push "$IMAGE_TAG"; + fi build-opensbi: extends: .opensbi_job_rules
Users expect images pulled from registry.gitlab.com/qemu-project/qemu/ to be stable. QEMU repository workflow pushes merge candidates to the /staging branch, and on success the same commit is pushed as /master. If /staging fails, we do not want to push the built images to the registry. Therefore limit the 'docker push' command to the /master branch on the mainstream CI. The fork behavior is unchanged. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- .gitlab-ci.d/container-template.yml | 11 ++++++++++- .gitlab-ci.d/edk2.yml | 11 ++++++++++- .gitlab-ci.d/opensbi.yml | 11 ++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-)