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

Dockerfile version for power ppc64le #305

Merged
merged 4 commits into from
Dec 9, 2016

Conversation

betamaniac
Copy link

I'm a part of power team which does port the project to power architecture. In this case we need to have another Dockerfile. It will help to run Jupyter Notebook on power.

@parente
Copy link
Member

parente commented Nov 30, 2016

Hi @betamaniac. Thanks for taking the time to put together this Dockerfile. I'm not sure if it makes sense to merge it into this repo, mainly because we don't have a way to build and test it at the moment. (We have an x86 VM on which we build all the other images.)

As an alternative, what do you think about putting the Dockerfile up as a reference in a gist or other repo, and then linking to it from the recipes wiki page?

@betamaniac
Copy link
Author

Hi @parente could you please request a free ppc64le VM from Oregon State University from here: http://osuosl.org/services/powerdev/request_hosting/

@ipoddar-ibm
Copy link

Hello @parente, we at IBM Systems are willing to provide POWER systems resources for continuous build of these docker files from the site @betamaniac has referenced. Will you be able to work with us to ensure that the Dockerfile is upstreamed? thanks I.P

@parente
Copy link
Member

parente commented Dec 3, 2016

Hi @ipoddar-ibm. The Dockerfiles submitted as PRs to this repo are built on Travis CI, reviewed, merged, rebuilt on an x86 VM, and pushed to Docker Hub. If you're offering to help out with setting up and helping to maintain that flow (or a better one!) for PPC images, then, yes, let's work together. It'll likely involve:

  1. Understanding if there will only be the one base image for the Dockerfile you submitted, or a PPC equivalent for each of the existing images
  2. Finding a way to automate test builds of pull requests that touch the PPC Dockerfile(s). (I don't think Travis CI has a PPC option, but I may be wrong.)
  3. Updating the Makefile that builds the images to also account for the PPC Dockerfile(s).
  4. Getting setup to run the builds after merge on the PPC site mentioned.

Are you interested?

@minrk, @jakirkham, @rgbkrk what do you guys think?

@rgbkrk
Copy link
Member

rgbkrk commented Dec 4, 2016

I whole heartedly agree that if you want an architecture other than x86 supported you'll need to devote resources towards everything Peter outlined. Free compute isn't the issue, it's the continuous maintenance. In following with the rest of the standards of the docker-stacks project, all must be automated and auditable.

@minrk
Copy link
Member

minrk commented Dec 5, 2016

I'm happy to add anything here that's maintainable. I haven't dealt with Docker on other architectures yet, so I don't know what it would involve. It would be ideal for me, if:

  1. changes could be made as fixes to existing Dockerfiles, rather than a clone of the whole stack
  2. ppc builds were automated and publicly visible (bonus: set CI status)

But since Dockerfiles make an immutable trie and can't extend each other, I'm not sure if that's doable since it's the FROM line that changes. Is it just the FROM line? If so, perhaps doing this with generated Dockerfiles, substituting the FROM line, will be the best approach.

@ipoddar-ibm
Copy link

Hello @parente , @rgbkrk , @minrk , thanks very much for your comments. I believe we can make the Dockerfiles maintainable on ppc64le in a phased manner. @betamaniac and I are happy to help set up the automation based on what you already have working on x86 and with small changes for supporting ppc64le. Regarding what @parente requested, here is my proposal for changes in phases:

PHASE 1 ( jupyter dockerfile with a patch):

  1. we check in a patch file: Dockerfile.ppc64le.patch which includes only the diffs from the Dockerfile for x86. we do this only for the jupyter dockerfile.
  2. update your Makefile so that if [ uname -m == "ppc64le" ]; then first patch the existing Dockerfile for x86 for jupyter and then run docker build.

PHASE 2 (jupyter image in dockerhub for ppc64le) :

  1. docker push the power image for jupyter in dockerhub under the ppc64le tag: https://hub.docker.com/u/ppc64le/

PHASE 3 (images in dockerhub for other stacks for ppc64le):

  1. we create docker images for all the other stacks (tensorflow, sci-py, all-spark etc) under the ppc64le.

PHASE 4 (multi-arch images for docker stacks in ppc64le):

  1. Create a multi-arch docker images: https://eyskens.me/multiarch-docker-images/ similar to this one: https://hub.docker.com/u/ibmblockchain/

PHASE 5 (Jenkins CI on ppc64le):
Regarding Travis CI: unfortunately, we do not have ppc64le support in travis ci yet. We can try to set up a jenkins build on the free power machine similar to this: https://gist.github.com/christianchristensen/5519757

We can put in the major effort required to do this over a few months into 2017. But we need phase 1 to be completed quickly and upstreamed ASAP, so that some of our users on power /ppc64le (e.g. dashdb) can at least build a docker image for jupyter on their own. Is this plan acceptable?

@rgbkrk
Copy link
Member

rgbkrk commented Dec 5, 2016

This plan is good with me, I'm happy with the patch approach and your investment of effort.

@minrk
Copy link
Member

minrk commented Dec 5, 2016

That proposal sounds great to me! I'm AOK with the plan to update this PR to contain the patch and makefile changes, then we'll be checking that Travis still works and trust you folks that ppc is working until a CI setup is online.

@parente
Copy link
Member

parente commented Dec 5, 2016

Sounds like a plan. Just some comments that I think will also apply to the patch file.

…updating the Makefile to make use of the patch file
@ipoddar-ibm
Copy link

we just added a patch file. This completes PHASE 1. Can this be merged ASAP?

@@ -0,0 +1,105 @@
# Copyright (c) IBM Corporation 2016
Copy link
Member

Choose a reason for hiding this comment

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

Since the patch has been added, presumably this file should be removed?

Choose a reason for hiding this comment

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

Well, I have found it easier to keep the full Dockerfile.ppc64le handy, to generate the patchfile like this: "diff -Naur Dockerfile Dockerfile.ppc64le > Dockerfile.ppc64le.patch" if we need to create a new patch file. Also it is good to have the full Dockerfile as a reference that we can quickly link to.

Copy link
Member

Choose a reason for hiding this comment

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

OK. I don't fully understand the benefit of the patch file if it's only used to generate a file that's already there, but we can refine later.

One last question: is there a reason that you have changed the license of this file to be different from the rest? Is it possible to leave the license as BSD so that it's consistent with the rest of the repository?

Choose a reason for hiding this comment

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

OK, if you feel strongly against having Dockerfile.ppc64le in the repo, we will remove it.

Regarding the license, I will need to check that and get back to you.

Copy link
Member

Choose a reason for hiding this comment

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

It's fine to have the file in there if it makes things simpler. We can work on it in the future. I think we would need a pretty strong reason to have one file with a different license in the repo, though.

Copy link
Author

Choose a reason for hiding this comment

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

we leave the license as BSD so that it's consistent with the rest of the repository

Choose a reason for hiding this comment

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

Also there is a small change to the Makefile to ensure that the Dockerfile is backed up only if we patch it.

Please review and let us know if this can be merged.

thanks

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, sorry for the delay. I was waiting for Travis' green check, but had to get on a plane. Thanks for your patience!

@minrk minrk merged commit 4c36387 into jupyter:master Dec 9, 2016
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.

6 participants