popper icon indicating copy to clipboard operation
popper copied to clipboard

cli: engine-agnostic way of building images

Open ivotron opened this issue 4 years ago • 6 comments

Proposal to add an image attribute for steps that can be used instead of the uses one, with the following:

steps:
- image:
    name: myregistry.io/myorg/myrepo
    tags:
    - tag1
    - tag2
    dockerfile: path/to/dockerfile/Dockerfile.foo
    context: ./
    build_args:
      ARG1: v14.2.9
      ARG2: another arg
    push: true
    import_cache: docker.io/another/img:tag
    export_cache: docker.io/an-image/toexport:tag # or the keyword 'inline'
# other step attributes as usual
  runs: [cmd]
  args: [--as, --usual]

The above is inspired by docker-compose's syntax. The goal is to have an engine-independent way of specifying how to build images.

ivotron avatar Aug 07 '20 20:08 ivotron

@wtraylor any feedback on the above would be greatly appreciated 🙏

ivotron avatar Aug 07 '20 20:08 ivotron

Thank you, @ivotron, for asking for feedback. I have looked a bit at the Docker Compose syntax, but I have no practical experience with it.

The guiding question should be: What do users really need? What missing feature would force them to drop Popper and write everything manually? It the same time, you shouldn’t focus on those 80% of features that will rarely get used.

For my current project, I have no need for a more elaborate syntax for building images. The only issue I have is that the docker network bridge doesn’t work on my system, so I need to use --network host for the building step. That is an issue of my particular host system, though.

So, my 2 cents are to first create a sharp focus on the core features and get all of them really easy to use (with fixing bugs, adding help messages, providing examples, writing tutorials, etc.). Once there is a larger user base, the specific needs of the community will become more apparent.

wtraylor avatar Aug 11 '20 09:08 wtraylor

thanks a lot for the feedback @wtraylor! I agree 100% on the 80-20 rule. I should have specified that this image attribute would not be replacing the current uses one but rather they'd both exist.

The main rationale behind this is that in some cases there is the need to build and push images as part of the workflow, for example:

- id: build-rook-img
  uses: docker://docker:19.03.10
  args:
  - build 
  -   --build-arg=CEPH_RELEASE=v14.2.9
  -   --tag=uccross/skyhookdm-ceph:v14.2.9
  -   --file=docker/Dockerfile.release
  -   .

- id: push-img
  uses: docker://docker:19.03.3
  secrets: [DOCKER_USERNAME, DOCKER_PASSWORD, TRAVIS_PULL_REQUEST]
  runs: [sh, -ec]
  args:
  - |
    # only push from upstream and only for the master branch
    if [[ $TRAVIS_PULL_REQUEST == false ]] && [[ $GIT_REMOTE_ORIGIN_URL == "https://github.com/uccross/skyhookdm-ceph-cls" ]] && [[ $GIT_BRANCH == "master" ]]; then
      docker login -u "$DOCKER_USERNAME" -p "$DOCKER_PASSWORD"
      docker push uccross/skyhookdm-ceph:v14.2.9
    fi

the above doesn't run in singularity or podman, thus making the workflow not portable. The build attribute would help in this scenario since it will abstract building and pushing using the underlying engine.

ivotron avatar Aug 17 '20 13:08 ivotron

high-level outline of how this would be implemented:

  1. add changes to the YAML spec so it supports the syntax specified in the OP.

  2. write new tests for this in https://github.com/getpopper/popper/blob/master/src/test/test_parser.py

  3. add new workflows that use this feature, one for each of the supported engines in https://github.com/getpopper/popper/blob/master/src/test/test_runner_host.py

cc: @BrayanDurazo

ivotron avatar Aug 19 '20 20:08 ivotron

Hello @ivotron my group and I would like to work on this as our final issue for our UTCS class. So altogether we are thinking of working on issues #541 , #822 , #892 , and this one. Thanks!

jaimistry12 avatar Sep 21 '20 16:09 jaimistry12

@jaimistry12 sounds good. Let us know if y'all have any questions, we'll be happy to help.

ivotron avatar Sep 24 '20 22:09 ivotron