azure/acr-builder

Azure Container Registry Build Runner

acr-builder's Issues

build failed with --docker-registry but without --docker-image

Is this a BUG REPORT or FEATURE REQUEST?:
BUG REPORT

What happened?:
run acr-builder with --docker-registry but without --docker-image

What did you expect to happen?:
Build succeeded

How do you reproduce it (as minimally and precisely as possible)?:
Build failed as level=error msg="invalid reference format"

Failed to get image tag

Is this a BUG REPORT or FEATURE REQUEST?:
BUG
What happened?:
Unable to use acr-builder container to build docker image. It fails at the end when trying to get image tag.
What did you expect to happen?:
Build succeeded.
How do you reproduce it (as minimally and precisely as possible)?:

1. Run docker build --rm -t acr-builder . to create the acr-builder container image
2. Run ./scripts/run-build.sh (The script basically uses acr-builder container to build acr-builder source code.)
3. Error
   time="2018-01-31T07:14:25Z" level=info msg="Running command docker image ls --digests --format "{{.Digest}}" acr-builder"
   docker: 'image' is not a docker command.
   See 'docker --help'.
   Anything else we need to know?:

Environment:
Ubuntu 16.4 + Docker 17.12.0-ce

@shhsu

Remove --docker-image requirement when building against dockerfile

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

What happened?:
Build with --docker-file but without --docker-image, the command failed complaining "Image name not specified for docker file "

What did you expect to happen?:

If the users just want to test the dockerfile build without push, it's not necessary to provide the --docker-image parameter.

Output digests of build dependencies

Current output format:

[{"image": "<image>", "build-dependencies": ["<dep1>:<tag1>", "<dep2>:<tag2>"], "runtime-dependency": "<base1>:<tag3>"}, ...]

Proposed change

image : 
    repo , [tag], digest
    -> deps [
        { repo [tag] digest , type },
        {  repo [tag] digest , type }

support --debug flag which will output verbose details of the build

Add support for source provided as tar archive

Discover dependencies by transitively walking docker-compose

Output all dependencies by transitively walking down all the docker files in the docker compose.

Remove warning for docker login

Is this a BUG REPORT or FEATURE REQUEST?:
Feature request
What happened?:
When running the builder there is a warning to pass in the password through stdin
What did you expect to happen?:
Warning should not appear
How do you reproduce it (as minimally and precisely as possible)?:
Run the builder with credentials for the registry
Anything else we need to know?:

Environment:

Simpify inputs

Docker build command is considerably easier to use than acr-builder
https://docs.docker.com/engine/reference/commandline/build/#git-repositories
They have enhanced their input to allow github url or tarball
We should make our input at least as simple as theirs

Registry name for image from docker hub is wrong

Is this a BUG REPORT or FEATURE REQUEST?:
BUG REPORT

What happened?:

1. Run ./scripts/run-build.sh
2. In the dependencies output, the registry for runtime image docker is empty. It should be registry.hub.docker.com.

Run docker build in Travis CI

Is this a BUG REPORT or FEATURE REQUEST?:
Feature Request.

Support --docker-secret-build-arg

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST
What happened?:
Allow passing --docker-secret-build-arg and hide the value from the output or log.

Collect platform-specific digest for base image

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

What happened?:
For multi-arch docker images (eg, Ubuntu), the builder can only collect the master digest for all platforms as `the current limitation as ``docker inspectand docker image``` commands. Ideally, we want to provide platform-specific digest for dependency tracking.

Add Travis CI badge

Is this a BUG REPORT or FEATURE REQUEST?:
Feature request.

Add the Travis CI badge so we can see whether or not the build is actively passing/failing.

Enable base image references

Use milliseconds for build duration

Is this a BUG REPORT or FEATURE REQUEST?:
Feature request

What happened?:

Build duration is currently being printed out using %s in main.go which causes formats like 3d5h6s... for longer times. This makes it harder to process, so instead just always use milliseconds.

How do you reproduce it (as minimally and precisely as possible)?:

Run the builder with a large image.

Discover dependencies in Docker file

Parse the docker file and discover dependencies.

Introduce a common dependency management tool

For identifying dependencies, performing analysis, picking compatible libraries, managing the vendor folder, etc.

Failed to build if the from command use arg

Is this a BUG REPORT or FEATURE REQUEST?:
BUG REPORT

What happened?:
run acr-build using the following dockerfile with --docker-build-arg tag=latest

ARG tag
FROM ubuntu:${tag}

Build failed:
level=error msg="invalid reference format"

The dependency scan needs to expand arguments in from command.

Default entry point does not display help and instead shows an error

docker run --rm -it acr-builder
INFO[0000] Chdir to /
ERRO[0000] Failed building build task, error: Failed to find docker compose file in source directory

Add --pull in docker build command

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

--pull option always attempts to pull a newer version of the image to make sure the local cache is up to date.
https://docs.docker.com/engine/reference/commandline/build/#options

Base image dependencies should be output before building the image.

Detect file format and align with docker build

Simplify local build to enable commands similar to docker build

build -f Dockerfile/Compose.yml -t myregistry.azurecr.io\app:v1

We can assume that the docker sock is passed in for these scenarios.
Also we can default the file to pick up Dockerfile by default followed by docker-compose.yml second.

Show progress bar while downloading tar file

Improve logging if failing to parse image digests

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

Log warning message if getRepoDigest function fails to parse image digests.

cc @shhsu

Typo in the README

sername should be username

Output time taken for build in a consumable form

Provide useful error message when invoking builder without Image name for docker build

Is this a BUG REPORT or FEATURE REQUEST?:
Bug Report
What happened?:
Acr Builder error is not helpful when docker image not passed in with docker-file
ERRO[0032] invalid reference format

What did you expect to happen?:
Should specify the the image was not passed in.

How do you reproduce it (as minimally and precisely as possible)?:
Run builder with docker file and no image.

Anything else we need to know?:

Environment:

Failure to parse build references

2018/01/19 16:57:24 server.go:73: Using API v1
2018/01/19 16:57:24 debugger.go:98: launching process with args:
[/home/eric/.gvm/pkgsets/go1.9/global/src/github.com/acr-builder/debug
--push --docker-user ehotinger --docker-password <password> --docker-image
hello-multistage:docker --docker-registry <registry>]
API server listening at: 127.0.0.1:2345
2018/01/19 16:57:25 debugger.go:497: continuing
time="2018-01-19T16:57:25-05:00" level=info msg="Running command
docker login -u ehotinger -p <password> <registry>"
Login Succeeded
time="2018-01-19T16:57:26-05:00" level=info msg="Running command
docker build -t <registry>/hello-multistage:docker ."
Sending build context to Docker daemon 3.072 kB
Step 1/7 : FROM golang:alpine as Build
Error parsing reference: "golang:alpine as Build" is not a valid
repository/tag: invalid reference format

Image digest result is <none>

Is this a BUG REPORT or FEATURE REQUEST?:
BUG

What happened?:
Unable to get image digests after build succeeds. All values are <none>.

What did you expect to happen?:
Valid digest value for each image.

How do you reproduce it (as minimally and precisely as possible)?:

1. Run docker build --rm -t acr-builder . to create the acr-builder container image
2. Run ./scripts/run-build.sh (The script basically uses acr-builder container to build acr-builder source code.)
3. Error: Running command docker image ls --digests --format "{{.Digest}}" acr-builder
   "<none>"

Anything else we need to know?:
The issue seems related to docker/cli#728.
A potential fix is to move image digest evaluation task after push.

Environment:
Ubuntu 16.4 + Docker 17.12.0-ce

@shhsu

Testcase TestObtainFromKnownLocation intermittently failed as file_server was not ready

It seems like a timing issue. The code started the file server in a background goroutine. The server might not be ready when the code tried to connect.

func testArchiveSource(t *testing.T, tc obtainTestCase) {
cleanup(tc.targetDir)
defer cleanup(tc.targetDir)
server := testCommon.StartStaticFileServer(t)
defer testCommon.ReportOnError(t, func() error { return server.Shutdown(context.TODO()) })
source := NewArchiveSource(tc.url, tc.targetDir)
runner := test.NewMockRunner()
runner.PrepareCommandExpectation(tc.expectedCommands)
fs := runner.GetFileSystem().(*test.MockFileSystem)
fs.PrepareChdir(tc.expectedChdir)
if tc.getWdErr != nil {
fs.On("Getwd").Return("home", *tc.getWdErr).Once()
}
err := source.Obtain(runner)

Add a dockerignore file

Is this a BUG REPORT or FEATURE REQUEST?:

Feature request. Some unnecessary files can be filtered out by the docker daemon.

Unit test is unstable

Is this a BUG REPORT or FEATURE REQUEST?:
BUG
What happened?:
Run ./scripts/run-build.sh to build acr-builder. Unit test sometimes failed as the following error.

--- FAIL: TestCompileHappy (0.07s)
assertions.go:247:
Error Trace: build_test.go:314
build_test.go:294
build_test.go:299
build.go:20
build.go:278
workflow.go:70
workflow.go:49
build_test.go:284
build_test.go:234
Error: Max difference between 2018-02-01 09:14:03.003017782 +0000 UTC m=+0.0111730
13 and 2018-02-01 09:14:02 +0000 UTC allowed is 1s, but difference was 1.003017782s

What did you expect to happen?:
unit test passed

How do you reproduce it (as minimally and precisely as possible)?:

Anything else we need to know?:

Environment:

Remove requirement for needing either HeadRev or Branch for git

Support local build

Dockerfile scenario: allow build/push to multiple tags

A single docker build command can build into multiple tags, we should be able to build the image, tag them all and push all the tags

Utilize docker build context to support tarball, git://, git@, github.com/, https, http

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

What happened?:
Currently, the acr-build download tarball or git source to local first and then pass the local context to docker build. Two issues.

1. The source is copied twice (remote->local->docker daemon).
2. It doesn't support some git path like pull request, eg, myrepo.git#pull/42/head.

What did you expect to happen?:
Pass the remote url to tarball or git source directly to docker build as [build context]
(https://docs.docker.com/engine/reference/commandline/build/#extended-description)

Remove requirement of registry argument

Postfix / to the argument if registry is supplied

Add contributing guide

Don't use os.Exit(-1)

Is this a BUG REPORT or FEATURE REQUEST?:

Bug report.

What happened?:

If the builder fails to run, it will have its exit code be 255 (exit status out of range) if you run docker inspect on it. Values should be 0..255 inclusive, where 1 is a general failure. http://tldp.org/LDP/abs/html/exitcodes.html

What did you expect to happen?:

The container should produce standard exit codes from docker inspect based on different types of failures.

How do you reproduce it (as minimally and precisely as possible)?:

Crash the builder by passing in garbage arguments. Run docker ps -a and do docker inspect

Fix
This is fixed by simply changing os.Exit(-1) in main.go to be 1, but we should define standard exit codes so we can determine which piece failed.

Assume push to ACR_BUILD_DOCKER_REGISTRY even if it is not defined in docker-compose image name

Talk to @northtyphoon and @mnltejaswini

Currently we require user to explicitly refer to ACR_BUILD_DOCKER_REGISTRY in docker compose file as image name. i.e.

image: ${ACR_BUILD_DOCKER_REGISTRY}hello-builder-1.0-${ACR_BUILD_BUILD_NUMBER}

This means when user first come onboard they would have to modify their docker-compose file. It is not desirable.

We should assume the push goes to ACR_BUILD_DOCKER_REGISTRY if no registry name is specified in the image name.

The build dependency output should include git commit

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

What happened?:
Currently, the build dependency output only provides image digest. If we want to rebuild the image if base images change, we also need to know the source commit id.

Allow specifiying the target build stage to build

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

When building a multi-stage docker file, allow users to set the target build stage through --target option.

Reference: https://docs.docker.com/engine/reference/commandline/build/#specifying-target-build-stage-target

Add support for Travis CI

Is this a BUG REPORT or FEATURE REQUEST?:
Feature request.

Add support for Travis CI so we can have all the automation benefits.

Allow --no-cache option in docker build command

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

--no-cache allow not using cache when building the image. enforce clean build if needed.

https://docs.docker.com/engine/reference/commandline/build/#options

Drop docker-compose support

Is this a BUG REPORT or FEATURE REQUEST?:
FEATURE REQUEST

We have discussed it a few times. Since we will evaluate our build configuration story and may look for other options, I suggest we drop docker-compose support and focus on dockerfile at the current stage. It will also help further cleanup and refactor the code base and reduce the image size.

@sajayantony
@ehotinger

When scanning docker-compose file and an image name is not defined, use service name as it's image name

This is the default behavior of docker compose and we should do the same

Update README

The initial README needs to be tweaked a bit for consistency and grammar.

Remove vendor.conf

vendor.conf is unused after putting in godep for dependency management.

When git target directory is present but not on the default branch, branch parameter would be required

This issue follows #15 and we could decide this might be a non-issue.

We are talking about the scenario where

• git source is present on the local system
• currently a non-default branch is checked out

The build system would not clone the repo again because it is present. It would follow the procedure of clean->fetch->checkout->pull to ensure the latest source.

The issue arises when we are pulling but the local repo is on the default branch. If we omit the branch parameter, we get something like this

>git pull <remote>
You asked to pull from the remote '<remote>', but did not specify
a branch. Because this is not the default configured remote
for your current branch, you must specify a branch on the command line.

Note that this scenario should not affect our acr-build-runner because I think we can assume the branch parameter would always be passed in.

How should we treat this scenario?

1. Accept that the error is expected and always require user to specify branch parameter if this scenario is expected
2. Use git command to figure out what the current branch and call git pull <remote> <branch> instead

I am leaning towards 1 because we would just be opaquely following git's behavior.