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

Docker #340

Merged
merged 4 commits into from Mar 27, 2019

Conversation

Projects
None yet
2 participants
@sbrunk
Copy link
Member

commented Mar 25, 2019

This is an alternative approach to almond-sh/docker-images#1.

The main differences are:

  • We have a single generic Dockerfile in the main repo.
  • When a tag is pushed, we use travis to build docker images with kernels for 2.11, 2.12 and both, and push them to docker hub.

The Dockerfile makes it easy to create a docker image from a locally built kernel by setting LOCAL_IVY=yes. This is used in travis where the images are created from locally published jars, which is why we can run this job in parallel to uploading to sonatype.

Right now it does a sbt + publishLocal in the update-docker-images script, then copies $HOME/.ivy/local into $TRAVIS_BUILD_DIR/ivy-local so we can copy it into the docker image. I think this could be improved by doing an additional publish to $TRAVIS_BUILD_DIR/ivy-local during the release stage directly. I tried but my understanding of sbt seems to be too limited.

@alexarchambault
Copy link
Member

left a comment

Added a few comments. That looks neat!

.travis.yml Outdated
stages:
- name: test
- name: release
if: (branch = master AND type = push) OR (tag IS present)
- name: upload-launcher

This comment has been minimized.

Copy link
@alexarchambault

alexarchambault Mar 25, 2019

Member

Why remove the upload-launcher stage? Couldn't it be kept along the new upload-artifacts?

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 25, 2019

Author Member

Because, if my understanding of travis is correct, jobs run only in parallel if they're in the same stage.

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 26, 2019

Author Member

I've put the upload-launcher stage back in as it was before.

Dockerfile Outdated
# i.e SCALA_VERSIONS="2.11.12 2.12.8"
ARG SCALA_VERSIONS
USER $NB_UID
RUN [ -z "$SCALA_VERSIONS" ] && echo "SCALA_VERSIONS is empty" && exit 1; \

This comment has been minimized.

Copy link
@alexarchambault

alexarchambault Mar 25, 2019

Member

NIT Maybe we could put that command in a script, to make it easier to tweak it in the future.

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 25, 2019

Author Member

Agreed. That would also make it easier to reuse it outside of a docker build. I'll try to move it out.

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 26, 2019

Author Member

done

git add Dockerfile
git commit -m "almond $VERSION, scala $sv"
done
sbt '+ publishLocal'

This comment has been minimized.

Copy link
@alexarchambault

alexarchambault Mar 25, 2019

Member

Do we want to do that for releases? They should already be on Maven Central when this stage runs.

Maybe we could run that stage for snapshots (and push them with a snapshot tag say), and rely on publishLocal only for them.

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 25, 2019

Author Member

Right now it runs in parallel to the upload-launcher (see my comment about the stage) so the artifacts aren't available yet. I can change it back to run afterwards and use the published artifacts. Then only do a publishLocal for snapshots builds as you've suggested.

This comment has been minimized.

Copy link
@alexarchambault

alexarchambault Mar 25, 2019

Member

I guess we can run them sequentially, yes. Doing so allows to avoid the +publishLocal, so maybe it's not even slower…

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 26, 2019

Author Member

The script in combination with .travis.yml differentiates now between releases and a push to master. +publishLocal runs only on master now and creates a snapshot docker image from the current HEAD. On a tag, it should grab the published artifacts instead.

Show resolved Hide resolved scripts/update-docker-images.sh
Move kernel install commands into a dedicated shell script
- build snapshots only when pushing to master, not when pushing a tag
- move docker job back into its own stage

@sbrunk sbrunk added the enhancement label Mar 26, 2019

.travis.yml Outdated
@@ -18,7 +18,7 @@ stages:
- name: release
if: (branch = master AND type = push) OR (tag IS present)
- name: upload-launcher
if: (branch = master AND type = push) OR tag IS present

This comment has been minimized.

Copy link
@sbrunk

sbrunk Mar 27, 2019

Author Member

Good catch. I this change was supposed to be applied to the update-docker-images stage in order to support snapshot images.

This comment has been minimized.

Copy link
@alexarchambault

This comment has been minimized.

Copy link
@alexarchambault

alexarchambault Mar 27, 2019

Member

Ah, my bad, I opened #343 to re-instate it.

@alexarchambault

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

About pushing the images for snapshots (I suggested that, right?), maybe we could disable it after the next release, if the additional stage takes too long. But it's fine now to test that all of that works.

Edit: the condition of the docker stage wasn't changed, so it's still only built for releases.

@alexarchambault

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

So merging, thanks!

@alexarchambault alexarchambault merged commit 5fc241a into almond-sh:master Mar 27, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sbrunk

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

The docker stage fails because there's a versioning issue.

Cause by this linein update-docker-images.sh because on travis it uses git versioning.
Locally it bumps the minor version and adds SNAPSHOT which I have done in the script too.

@sbrunk

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

I guess the most robust fix would be to get the version out of sbt somehow.

@alexarchambault

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

We can always set the version right before the publishLocal, with something like sbt 'set version in ThisBuild := "0.1-docker"' +publishLocal, and use 0.1-docker down the line.

@sbrunk sbrunk referenced this pull request Mar 29, 2019

Closed

More generic Dockerfile #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.