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

Added interpolation to docker option in manifest.yaml #1100

Merged
merged 8 commits into from
Aug 21, 2020

Conversation

huydoan2
Copy link
Contributor

@huydoan2 huydoan2 commented Aug 7, 2020

This PR adds environmental variable interpolation to docker action so that the image name can be provided through an environmental variable.

You now can do this:

action:
  function: myFunc
  docker: $image_name

@mrutkows
Copy link
Contributor

mrutkows commented Aug 10, 2020

@huydoan2 It appears that one of the go libs. wskdeploy uses has updated its dependencies and now only works with go 1.13:
stretchr/testify#991

@mrutkows
Copy link
Contributor

@huydoan2 It looks like the commit includes files from your other PR... it seems like you need to push from a rebased branch...

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.

Please limit files committed to just ones that permit interpolation (and not ones the upgrade project to Go 1.14 and go mod).

@@ -29,6 +29,8 @@ import (

"gopkg.in/yaml.v2"

"net/url"
Copy link
Contributor

Choose a reason for hiding this comment

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

better ordering of dependencies

@@ -796,7 +797,7 @@ func (dm *YAMLParser) composeActionExec(manifestFilePath string, manifestFileNam
if action.Native {
exec.Image = NATIVE_DOCKER_IMAGE
} else {
exec.Image = action.Docker
exec.Image = wskenv.InterpolateStringWithEnvVar(action.Docker).(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Interpolation feature in 1 line! Nice!

@@ -960,8 +960,15 @@ func TestComposeActionsForDocker(t *testing.T) {
assert.NotNil(t, action.Action.Exec.Code, TEST_MSG_ACTION_CODE_MISSING)
assert.Equal(t, runtimes.BLACKBOX, action.Action.Exec.Kind, fmt.Sprintf(TEST_MSG_ACTION_DOCKER_KIND_MISMATCH, action.Action.Exec.Kind))
assert.Equal(t, "mydockerhub/myimage", action.Action.Exec.Image, TEST_MSG_ACTION_DOCKER_IMAGE_MISMATCH)
case "CustomDockerAction6":
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding a unit test

@mrutkows mrutkows self-requested a review August 18, 2020 19:19
@mrutkows
Copy link
Contributor

@huydoan2 Could you please update the docs to indicate this value can now be interpolated? Here is the file I was thinking of:
https://github.com/apache/openwhisk-wskdeploy/blob/master/docs/wskdeploy_interpolation.md

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.

Thanks for updating the docs!

@mrutkows mrutkows merged commit 1979827 into apache:master Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants