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

Script to build docker images locally #135

Merged

Conversation

meenakshimadugula95
Copy link
Contributor

I was following the instructions to build the docker file locally by following the tutorial on here.
However the command for docker build was failing. I noticed failures in building the docker files for all available runtimes. The reason being the expected dependent files/folders were not available in the docker context. The Gradle builds would essentially pass as they are copying the dependencies before building the docker. For example https://github.com/apache/openwhisk-runtime-python/blob/master/core/python3Action/build.gradle#L25

To solve this problem, I had brainstormed multiple solutions, I came up with this one as it does not affect any dependencies on the dockerfile. The gradle builds should be able to pass as they were earlier.

Solution:

  • Create a script that would take in two arguments
    1. Runtime folder name - The runtime that needs to be built
    2. Docker Image tag - Name of the docker image
  • Example command ./tutorials/local_build.sh -r python3Action -t action-python-v3.7:1.0-SNAPSHOT

The script tries to mimic the Gradle files by copying the dependent folder/files, building the docker file and then deleting the dependent folders/files. I have updated the tutorial document with the same instructions. This PR would help anyone trying to build the runtime images locally.

@meenakshimadugula95 meenakshimadugula95 marked this pull request as draft November 3, 2022 00:15
@meenakshimadugula95 meenakshimadugula95 marked this pull request as ready for review November 3, 2022 21:22
@mrutkows
Copy link
Contributor

@meenakshimadugula95 Apart from specific review comments... Could we also, in this PR, update the top-level README to better inform people what they will find when referencing the "Docker" build path and also perhaps simplify (correct) the "two ways to build" section which is confusing IMO.

@@ -33,9 +33,12 @@ cd openwhisk-runtime-python
2. Build docker

Build using Python 3.7 (recommended). This tutorial assumes you're building with python 3.7.

Run local_build.sh with the correct parameters to build docker
Copy link
Contributor

Choose a reason for hiding this comment

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

Hoping that you could help out and make this markdown more readable?

  • Reduce the use of numbered lists (and do not start at zero 0.)
  • Create more subsection headings to break up the numbered lists
    • e.g., "Build the iamge", "Run the image", "Test the image" (with Curl) etc.
  • Create subsections that show case "Testing with Helloworld" and explain init/run entry points into image.

May wish to align with work in the nodejs runtime's README and docker build/run/test README:
apache/openwhisk-runtime-nodejs#227

Please know further updates to the files there are expected.

Refactoring structure of the tutorial for ease of readability
@meenakshimadugula95
Copy link
Contributor Author

@mrutkows thank you for the comments. I have modified the tutorial file so that it is better structured and easily readable. Please review this while I edit the top level readme.

@meenakshimadugula95
Copy link
Contributor Author

meenakshimadugula95 commented Nov 22, 2022

I have corrected the top level readme to reflect the two option to build python runtimes. Please take a look and review

Copy link
Contributor

@mrutkows mrutkows left a comment

Choose a reason for hiding this comment

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

@meenakshimadugula95 I verified the instructions and the script. Nicely done! If you have more time, I would love to simplify the examples (init/run) which are still a bit confusing/verbose. Approving...

@mrutkows mrutkows merged commit 5693058 into apache:master Nov 29, 2022
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.

3 participants