containers / build

another build tool for container images (archived, see https://github.com/rkt/rkt/issues/4024)
Apache License 2.0
342 stars 80 forks source link

No failure to add dep with non-existent tag #283

Closed sanmai-NL closed 7 years ago

sanmai-NL commented 7 years ago

Context

acbuild.sh:

#!/bin/sh
set -e
tear_down() {
    set +e
    status=$?
    sudo acbuild end
    set -e
    return $status
}
trap tear_down INT TERM EXIT
set -x
sudo -v
acbuild begin
acbuild dep add 'quay.io/baselibrary/ubuntu:16.04'
sudo acbuild run -- "true"
acbuild set-name 'test/test'
acbuild write 'test.aci'
sudo acbuild end

https://quay.io/repository/baselibrary/ubuntu?tab=tags does not list image tagged 16.04.

acbuild version 0.4.0
appc version 0.8.5

rkt Version: 1.22.0
appc Version: 0.8.9
Go Version: go1.7.4
Go OS/Arch: linux/amd64
Features: -TPM +SDJOURNAL

Linux #1 SMP PREEMPT Fri Dec 9 07:24:34 CET 2016 x86_64 unknown unknown GNU/Linux

Action

As user who is member of rkt group:

sh acbuild.sh

Expected

acbuild dep fails.

Actual

+ sudo -v
+ acbuild begin
+ acbuild dep add quay.io/baselibrary/ubuntu:16.04
+ sudo acbuild run -- true
run: bad HTTP status code: 404
+ tear_down
+ set +e
+ status=0
+ sudo acbuild end
+ set -e
+ return 0

Alternative case

The previous script with the image name and tag ubuntu:14.04 instead succeeds.

cgonyeo commented 7 years ago

Dependencies are added lazily. If no acbuild run command is performed there's no reason why acbuild should spend the time downloading an image, so all dependency downloading isn't performed until a run command.

For UX reasons, maybe acbuild could make a HEAD request for the dependency being added during the dep add command and alert the user if the dependency doesn't appear to exist. I'm hesitant to do more than a warning on this though, as it would be a valid use case to add a dependency for an image that may only be available in a separate environment (like an on-prem container registry only accessible from production, or something).

Think doing this check and just printing a warning would be sufficient?

sanmai-NL commented 7 years ago

I think the larger issue for me was how unclear the error message was. I would say don't bother to spend time on writing code to check whether a dependency is valid then, but please do document clearly that these deps are lazily evaluated, and in case they are evaluated during a run, then report a clear error message of what request returned 404 and in what context this request was made (downloading a dep). As I think many of your customers, I am used to Docker Engine and Compose, where various other API calls are made over HTTP as well so you never know.

cgonyeo commented 7 years ago

Gotcha. I've never spent much time in the docker world, so to me it's clear that there's only one thing here that would be happening over an HTTP request, but I suppose that wouldn't be the case for everyone else. I'll see if I can make that error make more sense.