Skip to content

Allow for a docker pull bypass for docker actions where the prefix maches the invoker's docker prefix#3052

Merged
csantanapr merged 1 commit intoapache:masterfrom
rabbah:localimages
Dec 13, 2017
Merged

Allow for a docker pull bypass for docker actions where the prefix maches the invoker's docker prefix#3052
csantanapr merged 1 commit intoapache:masterfrom
rabbah:localimages

Conversation

@rabbah
Copy link
Copy Markdown
Member

@rabbah rabbah commented Dec 3, 2017

This is a proposed patch to address the feature request in #2852.

The presence of an property bypassPullForLocalImages in the runtime manifest will toggle the feature on or off allowing a docker pull to be skipped if the docker action's image name has a prefix matches the local docker build prefix.

See https://github.com/apache/incubator-openwhisk/pull/3052/files#diff-d895f19f56edd5b725a1696b7b9d8877R196 for an example.

@alexkli can you take a look?

@rabbah rabbah force-pushed the localimages branch 2 times, most recently from c9d2811 to 7ef28cc Compare December 3, 2017 20:44
*/
def readFromEnv(key: String): Option[String] = sys.env.get(asEnvVar(key))

private def whiskPropertiesFile: File = {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this file is now only used for tests and tests should use WhiskProperties which already have to discover this file.

@rabbah rabbah added the wip label Dec 3, 2017
@rabbah rabbah force-pushed the localimages branch 8 times, most recently from 989915f to e038f31 Compare December 7, 2017 17:54
whisk_logs_dir: /Users/Shared/wsklogs
docker_registry: ""
docker_dns: ""
bypass_pull_for_local_images: true
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@csantanapr what do you think about making this true for the local envs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes make it true for local 👍

@rabbah rabbah added review Review for this PR has been requested and yet needs to be done. and removed wip labels Dec 7, 2017
@rabbah rabbah force-pushed the localimages branch 11 times, most recently from 33290b8 to 6f4817a Compare December 8, 2017 18:11
Copy link
Copy Markdown
Member

@csantanapr csantanapr left a comment

Choose a reason for hiding this comment

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

LGTM

@alexkli
Copy link
Copy Markdown
Contributor

alexkli commented Dec 11, 2017

@rabbah I don't have a chance to test the PR, but IIUC:

  1. inside the manifest (where is that btw, i have never configured it?):
    • set bypassPullForLocalImages to true
    • set defaultImagePrefix to e.g. whisk
  2. give the docker images you develop that prefix in their tag name, e.g. whisk/*
  3. start OW
  4. then it would never pull images with this prefix for --docker actions

I think that should work for the local dev/testing or demo use case.

With this in place, feel free to close #2852.

@rabbah
Copy link
Copy Markdown
Member Author

rabbah commented Dec 11, 2017

If you’re using the local or Mac environments then 1 is done for you automatically.

This commit adds a deployment flag which allows a docker action to be treated as a native image. A native image may eschew a docker pull. It is defined as one that has a prefix matching the docker prefix for managed images.

Also added some missing tids.
@rabbah rabbah added the pgapproved Pipeline has approved this change. label Dec 13, 2017
@csantanapr csantanapr merged commit 489a2bc into apache:master Dec 13, 2017
@rabbah rabbah deleted the localimages branch January 3, 2018 15:50
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
This commit adds a deployment flag which allows a docker action to be treated as a native image. A native image may eschew a docker pull. It is defined as one that has a prefix matching the docker prefix for managed images.

Also added some missing tids.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pgapproved Pipeline has approved this change. review Review for this PR has been requested and yet needs to be done.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants