Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch to ubuntu:bionic for base image #256

Merged
merged 6 commits into from
Mar 26, 2019
Merged

switch to ubuntu:bionic for base image #256

merged 6 commits into from
Mar 26, 2019

Conversation

chenbh
Copy link
Contributor

@chenbh chenbh commented Feb 11, 2019

Part of concourse/concourse#3266

  • resolve compatibility issue between busybox vs coreutil timeout
  • alpine packages docker nicely, ubuntu requires adding docker's own repository
  • Image size is now 305mb -> 448mb

* resolve compatibility issue between busybox vs coreutil `timeout`

Signed-off-by: Bohan Chen <bochen@pivotal.io>
The ubuntu package `docker` is outdated and we should be using
`docker-ce` instead

Signed-off-by: Bohan Chen <bochen@pivotal.io>
@chenbh chenbh changed the title switch to ubuntu:bionic for base image wip: switch to ubuntu:bionic for base image Feb 11, 2019
Signed-off-by: Bishoy Youssef <byoussef@pivotal.io>
Co-authored-by: Bohan Chen <bochen@pivotal.io>
@chenbh chenbh changed the title wip: switch to ubuntu:bionic for base image switch to ubuntu:bionic for base image Feb 22, 2019
Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing updates to the README to reflect the ability to now build and test with both alpine and ubuntu as base images.

It'd also be good to make sure that the removal of the -t flag is not a problem.

assets/common.sh Show resolved Hide resolved
pivotal-bin-ju and others added 2 commits March 18, 2019 17:33
- busybox (alpine) `timeout` uses `-t` for duration
- coreutils (ubuntu) `timeout` duration is a required argument
- add logic to detect which linux distro and use the appropriate flags

Signed-off-by: Bin Ju <bju@pivotal.io>
Co-authored-by: Bohan Chen <bochen@pivotal.io>
Signed-off-by: Bin Ju <bju@pivotal.io>
Co-authored-by: Bohan Chen <bochen@pivotal.io>
@chenbh chenbh requested a review from cirocosta March 18, 2019 21:37
Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(whopsy)

Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems good to me, but the tests indicate a failure in the way we're using that flag:

docker build -t a:alpine -f ./dockerfiles/alpine/Dockerfile .

leads to


• Failure [0.425 seconds]
Out
/go/src/github.com/concourse/docker-image-resource/tests/out_test.go:18
  when cache_from images are specified
  /go/src/github.com/concourse/docker-image-resource/tests/out_test.go:575
    passes the arguments correctly to the docker build command [It]
    /go/src/github.com/concourse/docker-image-resource/tests/out_test.go:631

    Got stuck at:
        timeout: unrecognized option: t
        BusyBox v1.30.1 (2019-02-16 10:01:46 UTC) multi-call binary.

        Usage: timeout [-s SIG] SECS PROG ARGS

        Runs PROG. Sends SIG to it if it is not gone in SECS seconds.
        Default SIG: TERM.
        Docker failed to start within 120 seconds.
        cat: can't open '/tmp/docker.pid': No such file or directory

    Waiting for:
        DOCKER ARG: --cache-from

@chenbh
Copy link
Contributor Author

chenbh commented Mar 19, 2019

pulled new version of alpine:edge and ...

$ docker run -it alpine:edge
/ # timeout --help
BusyBox v1.30.1 (2019-02-16 10:01:46 UTC) multi-call binary.

Usage: timeout [-s SIG] SECS PROG ARGS

Runs PROG. Sends SIG to it if it is not gone in SECS seconds.
Default SIG: TERM.

Before:

BusyBox v1.29.3 (2019-01-24 07:45:07 UTC) multi-call binary.

Usage: timeout [-t SECS] [-s SIG] PROG ARGS

Runs PROG. Sends SIG to it if it is not gone in SECS seconds.
Defaults: SECS: 10, SIG: TERM.

Good old busybox version bumps

- busybox 1.30.x `timeout` uses same arg pattern as coreutils `timeout`

Signed-off-by: Bin Ju <bju@pivotal.io>
Co-authored-by: Bohan Chen <bochen@pivotal.io>
@chenbh chenbh requested a review from cirocosta March 19, 2019 14:34
@cirocosta
Copy link
Member

cirocosta commented Mar 19, 2019

Hey, I still have problems here from a clean environment (no images & from a fresh clone):

• Failure [0.490 seconds]
Out
/go/src/github.com/concourse/docker-image-resource/tests/out_test.go:18
  when build arguments contain envvars
  /go/src/github.com/concourse/docker-image-resource/tests/out_test.go:200
    expands envvars and passes the arguments correctly to the docker daemon [It]
    /go/src/github.com/concourse/docker-image-resource/tests/out_test.go:201

    Got stuck at:
        timeout: unrecognized option: t
        BusyBox v1.30.1 (2019-02-16 10:01:46 UTC) multi-call binary.

        Usage: timeout [-s SIG] SECS PROG ARGS

        Runs PROG. Sends SIG to it if it is not gone in SECS seconds.
        Default SIG: TERM.
        Docker failed to start within 120 seconds.
        cat: can't open '/tmp/docker.pid': No such file or directory

    Waiting for:
        DOCKER ARG: --build-arg

Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests still showing some problems related to timeouts

@chenbh
Copy link
Contributor Author

chenbh commented Mar 20, 2019

Worked with @cirocosta and it seemed to be a flake

@chenbh chenbh requested a review from cirocosta March 20, 2019 17:50
Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@cirocosta cirocosta merged commit 9bb78ff into master Mar 26, 2019
@cirocosta cirocosta deleted the bionic-base-image branch March 26, 2019 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants