[libvirt PATCH 5/5] ci: Improve CI_IMAGE_TAG handling

Andrea Bolognani posted 5 patches 5 years, 8 months ago
[libvirt PATCH 5/5] ci: Improve CI_IMAGE_TAG handling
Posted by Andrea Bolognani 5 years, 8 months ago
Since we're already building the full container image reference
dynamically at this point, we can finally get rid of the annoying
requirement to include ":" in CI_IMAGE_TAG.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 ci/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index e1a5faaba6..96e4d62611 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -55,9 +55,9 @@ CI_IMAGE_REGISTRY = registry.gitlab.com
 # image instead
 CI_IMAGE_PREFIX = libvirt/libvirt/ci-
 
-# The default tag is ':latest' but if the container
+# The default tag is 'latest' but if the container
 # repo above uses different conventions this can override it
-CI_IMAGE_TAG = :latest
+CI_IMAGE_TAG = latest
 
 # We delete the virtual root after completion, set
 # to 0 if you need to keep it around for debugging
@@ -220,7 +220,10 @@ ci-run-command@%: ci-prepare-tree
 	if test "$(CI_IMAGE_REGISTRY)"; then \
 		image="$${image}$(CI_IMAGE_REGISTRY)/"; \
 	fi; \
-	image="$${image}$(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG)"; \
+	image="$${image}$(CI_IMAGE_PREFIX)$*"; \
+	if test "$(CI_IMAGE_TAG)"; then \
+		image="$${image}:$(CI_IMAGE_TAG)"; \
+	fi; \
 	$(CI_ENGINE) run $(CI_ENGINE_ARGS) "$$image" \
 		/bin/bash -c ' \
 		$(CI_USER_HOME)/prepare || exit 1; \
-- 
2.25.4

Re: [libvirt PATCH 5/5] ci: Improve CI_IMAGE_TAG handling
Posted by Daniel P. Berrangé 5 years, 8 months ago
On Fri, May 29, 2020 at 03:00:44PM +0200, Andrea Bolognani wrote:
> Since we're already building the full container image reference
> dynamically at this point, we can finally get rid of the annoying
> requirement to include ":" in CI_IMAGE_TAG.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  ci/Makefile | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/ci/Makefile b/ci/Makefile
> index e1a5faaba6..96e4d62611 100644
> --- a/ci/Makefile
> +++ b/ci/Makefile
> @@ -55,9 +55,9 @@ CI_IMAGE_REGISTRY = registry.gitlab.com
>  # image instead
>  CI_IMAGE_PREFIX = libvirt/libvirt/ci-
>  
> -# The default tag is ':latest' but if the container
> +# The default tag is 'latest' but if the container
>  # repo above uses different conventions this can override it
> -CI_IMAGE_TAG = :latest
> +CI_IMAGE_TAG = latest
>  
>  # We delete the virtual root after completion, set
>  # to 0 if you need to keep it around for debugging
> @@ -220,7 +220,10 @@ ci-run-command@%: ci-prepare-tree
>  	if test "$(CI_IMAGE_REGISTRY)"; then \
>  		image="$${image}$(CI_IMAGE_REGISTRY)/"; \
>  	fi; \
> -	image="$${image}$(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG)"; \
> +	image="$${image}$(CI_IMAGE_PREFIX)$*"; \
> +	if test "$(CI_IMAGE_TAG)"; then \
> +		image="$${image}:$(CI_IMAGE_TAG)"; \
> +	fi; \

Again, I'm not seeing what this test is for

>  	$(CI_ENGINE) run $(CI_ENGINE_ARGS) "$$image" \
>  		/bin/bash -c ' \
>  		$(CI_USER_HOME)/prepare || exit 1; \
> -- 
> 2.25.4
> 

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: [libvirt PATCH 5/5] ci: Improve CI_IMAGE_TAG handling
Posted by Andrea Bolognani 5 years, 8 months ago
On Tue, 2020-06-02 at 11:37 +0100, Daniel P. Berrangé wrote:
> On Fri, May 29, 2020 at 03:00:44PM +0200, Andrea Bolognani wrote:
> > -# The default tag is ':latest' but if the container
> > +# The default tag is 'latest' but if the container
> >  # repo above uses different conventions this can override it
> > -CI_IMAGE_TAG = :latest
> > +CI_IMAGE_TAG = latest
> >  
> >  # We delete the virtual root after completion, set
> >  # to 0 if you need to keep it around for debugging
> > @@ -220,7 +220,10 @@ ci-run-command@%: ci-prepare-tree
> >  	if test "$(CI_IMAGE_REGISTRY)"; then \
> >  		image="$${image}$(CI_IMAGE_REGISTRY)/"; \
> >  	fi; \
> > -	image="$${image}$(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG)"; \
> > +	image="$${image}$(CI_IMAGE_PREFIX)$*"; \
> > +	if test "$(CI_IMAGE_TAG)"; then \
> > +		image="$${image}:$(CI_IMAGE_TAG)"; \
> > +	fi; \
> 
> Again, I'm not seeing what this test is for

It was intended to account for the possibility of the user passing
an empty CI_IMAGE_TAG, but now that I think about it a bit more
that would be quite a silly thing to do and erroring out in that
case is perfectly fine. I'll drop this commit.

-- 
Andrea Bolognani / Red Hat / Virtualization