DockerHub fix #2 #3631

aasifkhan7 · 2020-07-27T21:16:17Z

@cdrini Seems like DockerHub is itself able to pull the submodules before the any of the steps in Dockerfile are run (effectively doing git pulling of submodules before the docker build)

Hence another solution for this issue, in addition to #3628 might be to add a check - for the sake of checking if DockerHub has already recursively cloned the submodules, and run git submodule commands in git rule of Makefile only when vendor/infogami is empty implying that submodule hasn't yet been cloned (we could have chosen any of the 3 submodules for checking if they're empty, the choice of vendor/infogami is entirely random).

This seems a better fix to the DockerHub publishing issue rather than the related PR: #3628

I was able to build on publish the image on DockerHub using this.

cclauss · 2020-07-28T07:00:53Z

In the current effort to port to Python 3 #3333 we want the flexibility to use the local infogami (whatever the current local checked out branch is in /vendor/local) or the branch that Python 2 production uses or the master branch that we are trying to get to be fully Python 3 compatible. This complication means that we might not always want the master branch of infogami. Hopefully, we can move quickly to a world where Python 2 is deprecated and we usually want the master branch of infogami.

cdrini · 2020-08-05T02:13:26Z

Hey @aasifkhan7 , nice work! In docker hub, we can add build environment variables; we could define a DOCKER_HUB environment variable, which, if set, does not call make git! :) Then we could do something like this: https://github.com/internetarchive/openlibrary/compare/docker-test-hub to checkout the git submodules; not sure if that would work, but do you think you could give it a try? That helps us (1) avoid code duplication in #3628, and (2) limit the number of changes we have to make at to just docker hub files.

cclauss · 2020-08-05T05:55:22Z

docker/hooks/post_checkout

@@ -0,0 +1,2 @@
+#!/bin/bash
+export DOCKER_HUB=TRUE


Should TRUE be uppercase or lowercase?

On the csh or zsh command line, man true provides a man page but man TRUE does not.

doesn't matter, I guess. ifneq is not a shell "command". I tried with even TRue and ifneq ($(DOCKER_HUB),TRue) - works good. (it's string matching, in my opinion)

I think we will be OK but try doing the same with false...

bash $ if true == "true" ; then echo yes; fi yes $ if false == "false" ; then echo yes; fi <nothing>

tried with fAlse as well. works like string matching for that as well.

Not in bash, but it's easier to write an if using ifeq() than figuring out its bash equivalent for the Makefile.

aasifkhan7 · 2020-08-05T08:52:24Z

@cdrini Added the environment variable and a build hook - docker/hooks/build.
Tested this and it builds successfully on Docker Hub as well as on local machine. (and it's cleaner than counting the files in vendor/infogami :) )

cdrini

Awesome! This is looking great :) A few things to test:

Try editing the Dockerfile.oldev to FROM aasifkhan7/olbase:latest (what is the link you're pushing to?), then try rebuilding web docker-compose build web and see if it runs! I think we might need to do the post_checkout hook as well as the build hook, because I don't believe the submodules get initialized at all otherwise. (The build won't fail if the submodules aren't initialized, but the site won't run :P)

docker/hooks/build

Makefile

docker/Dockerfile.olbase

cdrini · 2020-08-11T01:33:38Z

Hi @aasifkhan7 ! I tried pulling and it worked 🤩 I also confirmed that vendor/infogami was defined inside the docker image.

Hmmm, IMAGE_NAME should be defined when this is built in docker hub. Are you running the build step on your computer, and then running docker push? Or have you set up an automatic build on docker hub?

aasifkhan7 · 2020-08-11T02:02:43Z

@cdrini I have setup automatic build on dockerhub - no pushing from my computer. 🙂
I created a repository named openlibrary - and it built the image with the name martiansoul/openlibrary by default.
For central DockerHub repositories - we can have a user openlibrary on Dockerhub and make a public repository with name olbase for olbase and oldev for oldev and so on.

cdrini · 2020-08-12T00:16:54Z

I was getting an error when trying this on my test repo; docker hub said:

Successfully built 49544179094f
Successfully tagged olbase:latest
Pushing index.docker.io/cdrini/openlibrary-base:docker-test-gitfix...
Push failed. Attempt 2 in 60 seconds.
Push failed. Attempt 3 in 60 seconds.
Push failed. Attempt 4 in 60 seconds.
Push failed. Attempt 5 in 60 seconds.
{u'message': u'An image does not exist locally with the tag: cdrini/openlibrary-base'}

I'm trying changing the build hook right now 👍

cdrini · 2020-08-12T00:19:49Z

You can set the image name (e.g. olbase) and tags (e.g. latest) from the dockerhub UI; that's what the IMAGE_NAME is for. I'm going to see if we can actually remove the build hook entirely; the Makefile changes might be enough to make this work. I can set environment variables from the docker hub UI:

That might be enough!

cdrini · 2020-08-12T00:34:28Z

Didn't work :/ Looks like that doesn't set it during build for some reason. Going to try changing build hook to use IMAGE_NAME

cdrini · 2020-08-12T00:55:10Z

That did it! First successful build 🎉

This is needed in order to let us set the build name/tag and dockerfile path from the docker hub UI. See https://docs.docker.com/docker-hub/builds/advanced/#environment-variables-for-building-and-testing

cdrini

Lgtm; tested it builds the image, and that the built image can run (not the latest iteration, but a recent one).

cdrini · 2020-08-12T02:49:00Z

Here's the docker hub repo; I'll test a little more, and if it works, update the docs!

https://hub.docker.com/r/openlibrary/olbase

cclauss · 2020-08-12T04:15:06Z

Awesome work here!! Well done @aasifkhan7

aasifkhan7 added 3 commits July 27, 2020 20:56

added a check in git make rule

4682df9

fixed indentation in Makefile

6570922

count files in vendor/infogami

26a3e26

aasifkhan7 changed the title ~~Dockerhub fix #2~~ DockerHub fix #2 Jul 27, 2020

cdrini self-assigned this Aug 4, 2020

aasifkhan7 added 6 commits August 5, 2020 08:43

added environment variable for DockerHub

9b46fd4

added shebang in post_checkout

6cec82f

fixed variable naming in Makefile

af218c8

fixed variable name in Makefile

8f6094c

used ifneq from Make syntax

5bd9b70

echo DOCKER_HUB

4c90d8f

cclauss reviewed Aug 5, 2020

View reviewed changes

aasifkhan7 added 8 commits August 5, 2020 11:52

see environment variables

b322dd7

custom build hook

55f8f3f

TRue to TRUE

2b1b26b

modified path in build hook

76f1e94

cd in build hook

eb5984c

added ARG in Dockerfile.olbase

77b1886

delete post_checkout

1ffa148

removed numFiles and echo from Makefile

c1bc342

cdrini reviewed Aug 5, 2020

View reviewed changes

docker/hooks/build Outdated Show resolved Hide resolved

Makefile Show resolved Hide resolved

cclauss reviewed Aug 5, 2020

View reviewed changes

Makefile Show resolved Hide resolved

cclauss reviewed Aug 5, 2020

View reviewed changes

docker/Dockerfile.olbase Show resolved Hide resolved

added a comment for Dockerhub

78510ce

cdrini added this to the Active Sprint milestone Aug 10, 2020

Use IMAGE_NAME in docker build hook

b6b03af

This is needed in order to let us set the build name/tag and dockerfile path from the docker hub UI. See https://docs.docker.com/docker-hub/builds/advanced/#environment-variables-for-building-and-testing

cdrini approved these changes Aug 12, 2020

View reviewed changes

cdrini merged commit 61096f2 into internetarchive:master Aug 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DockerHub fix #2 #3631

DockerHub fix #2 #3631

aasifkhan7 commented Jul 27, 2020

cclauss commented Jul 28, 2020

cdrini commented Aug 5, 2020

cclauss Aug 5, 2020

aasifkhan7 Aug 5, 2020 •

edited

Loading

cclauss Aug 5, 2020

aasifkhan7 Aug 5, 2020 •

edited

Loading

cclauss Aug 5, 2020

aasifkhan7 Aug 5, 2020

aasifkhan7 commented Aug 5, 2020 •

edited

Loading

cdrini left a comment

cdrini commented Aug 11, 2020 •

edited

Loading

aasifkhan7 commented Aug 11, 2020 •

edited

Loading

cdrini commented Aug 12, 2020

cdrini commented Aug 12, 2020

cdrini commented Aug 12, 2020

cdrini commented Aug 12, 2020

cdrini left a comment

cdrini commented Aug 12, 2020 •

edited

Loading

cclauss commented Aug 12, 2020

DockerHub fix #2 #3631

DockerHub fix #2 #3631

Conversation

aasifkhan7 commented Jul 27, 2020

cclauss commented Jul 28, 2020

cdrini commented Aug 5, 2020

cclauss Aug 5, 2020

Choose a reason for hiding this comment

aasifkhan7 Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

cclauss Aug 5, 2020

Choose a reason for hiding this comment

aasifkhan7 Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

cclauss Aug 5, 2020

Choose a reason for hiding this comment

aasifkhan7 Aug 5, 2020

Choose a reason for hiding this comment

aasifkhan7 commented Aug 5, 2020 • edited Loading

cdrini left a comment

Choose a reason for hiding this comment

cdrini commented Aug 11, 2020 • edited Loading

aasifkhan7 commented Aug 11, 2020 • edited Loading

cdrini commented Aug 12, 2020

cdrini commented Aug 12, 2020

cdrini commented Aug 12, 2020

cdrini commented Aug 12, 2020

cdrini left a comment

Choose a reason for hiding this comment

cdrini commented Aug 12, 2020 • edited Loading

cclauss commented Aug 12, 2020

aasifkhan7 Aug 5, 2020 •

edited

Loading

aasifkhan7 Aug 5, 2020 •

edited

Loading

aasifkhan7 commented Aug 5, 2020 •

edited

Loading

cdrini commented Aug 11, 2020 •

edited

Loading

aasifkhan7 commented Aug 11, 2020 •

edited

Loading

cdrini commented Aug 12, 2020 •

edited

Loading