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

Implements setting environment variable in kaniko pod #3227 #3287

Merged
merged 11 commits into from
Jan 27, 2020
Merged

Implements setting environment variable in kaniko pod #3227 #3287

merged 11 commits into from
Jan 27, 2020

Conversation

ErmakovDmitriy
Copy link
Contributor

Relates to setting environment variable in kaniko pod #3227

Should merge after : #3284

Description

Implements a possibility to configure custom environment variables using name-value, name-ConfigMap and name-Secret pairs.

User facing changes

Adds to pull request #3284 a posiibility to set EnvVars as described below

build:
  cluster:
    envVars:
      - name: MY_ENV_VAR
        value: MY_ENV_VAR_VALUE
      - name: MY_ENV_VAR_2
        secret:
          name: kubernetes-secret-name
          key: secret-key
      - name: MY_ENV_VAR_3
        configMap:
          name: kubernetes-config-map
          key: config-map-data

Before

No possibility to set custom Env Vars

After

User can use key-value pairs, Secrets or ConfigMaps as a source for Env Vars.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Mentions any output changes.
  • Adds documentation as needed: user docs, YAML reference, CLI reference.
  • Adds integration tests if needed.

Reviewer Notes

  • The code flow looks good.
  • Unit test added.
  • User facing changes look good.

Release Notes

  • Skaffold config adds build#cluster#envVars section.

@codecov
Copy link

codecov bot commented Nov 24, 2019

Codecov Report

Merging #3287 into master will increase coverage by 0.7%.
The diff coverage is 43.24%.

Impacted Files Coverage Δ
cmd/skaffold/app/cmd/flags.go 100% <ø> (ø) ⬆️
cmd/skaffold/app/cmd/credits/export.go 0% <0%> (ø)
cmd/skaffold/app/cmd/cmd.go 67.44% <100%> (+0.25%) ⬆️
cmd/skaffold/app/cmd/schema/list.go 100% <100%> (ø)
cmd/skaffold/app/cmd/credits.go 100% <100%> (ø) ⬆️
cmd/skaffold/app/cmd/init.go 57.69% <20%> (+1.17%) ⬆️
cmd/skaffold/app/cmd/schema/print.go 80% <80%> (ø)
cmd/skaffold/app/cmd/schema.go 80% <80%> (ø)
pkg/skaffold/build/cluster/kaniko.go 0% <0%> (-55.13%) ⬇️
pkg/skaffold/initializer/init.go 29.12% <0%> (-29.32%) ⬇️
... and 100 more

@ErmakovDmitriy ErmakovDmitriy marked this pull request as ready for review November 24, 2019 07:27
@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Nov 27, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Nov 27, 2019
@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Dec 31, 2019
@dgageot
Copy link
Contributor

dgageot commented Jan 15, 2020

Hi @ErmakovDmitriy I'm afraid kaniko code has changed a lot since then. Could you try to rebase your PR? If you can't, let me know, I might be able to continue your work

@dgageot dgageot self-assigned this Jan 15, 2020
@ErmakovDmitriy
Copy link
Contributor Author

Hi @dgageot
I will try to do it at the weekend because I'm quite busy this week.

The same about pull #3284

@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jan 18, 2020
Signed-off-by: Dmitriy Ermakov <[email protected]>
@ErmakovDmitriy
Copy link
Contributor Author

Hi @dgageot
I made some changes to use Kubernetes API V1 packages for Volumes and VolumeMounts.
Unfortunately, I get Travis Integration test failure.

Here is the link to test https://travis-ci.com/GoogleContainerTools/skaffold/jobs/277179064

If I am not mistaken, all the failed tests are related to jib:

=== Failed Tests ===

integration/TestExpectedBuildFailures/jib_is_too_old

integration/TestExpectedBuildFailures

integration/TestDebug/kubectl

integration/TestDebug/kustomize

integration/TestDebug

integration/TestDiagnose/jib

integration/TestDiagnose/jib-multimodule

integration/TestDiagnose

integration/TestRun/jib

integration/TestRun

for instance:

time="2020-01-18T07:51:09Z" level=info msg="build output: Generating tags...\n - skaffold-jib -> gcr.io/k8s-skaffold/skaffold-jib:v1.2.0-445-g4c5f16d\nChecking cache...\n - skaffold-jib: time=\"2020-01-18T07:51:09Z\" level=warning msg=\"error checking cache, caching may not work as expected: getting hash for artifact skaffold-jib: getting dependencies for skaffold-jib: getting jib-maven dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies: Running [/skaffold/integration/testdata/jib/mvnw jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=1.4.0 -Djib.maven-plugin-version=1.3.0 --non-recursive jib:_skaffold-files-v2 --quiet]\\n - stdout: \\n - stderr: Exception in thread \\\"main\\\" java.io.IOException: Server returned HTTP response code: 403 for URL: https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.6.1/apache-maven-3.6.1-bin.zip\\n\\tat sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1900)\\n\\tat sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1498)\\n\\tat sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:268)\\n\\tat org.apache.maven.wrapper.DefaultDownloader.downloadInternal(DefaultDownloader.java:73)\\n\\tat org.apache.maven.wrapper.DefaultDownloader.download(DefaultDownloader.java:60)\\n\\tat org.apache.maven.wrapper.Installer.createDist(Installer.java:64)\\n\\tat org.apache.maven.wrapper.WrapperExecutor.execute(WrapperExecutor.java:121)\\n\\tat org.apache.maven.wrapper.MavenWrapperMain.main(MavenWrapperMain.java:55)\\n: exit status 1\"\nError checking cache. Rebuilding.\nFound [kind-kind] context, using local docker daemon.\nBuilding [skaffold-jib]...\nException in thread \"main\" java.io.IOException: Server returned HTTP response code: 403 for URL: https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.6.1/apache-maven-3.6.1-bin.zip\n\tat sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1900)\n\tat sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1498)\n\tat sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:268)\n\tat org.apache.maven.wrapper.DefaultDownloader.downloadInternal(DefaultDownloader.java:73)\n\tat org.apache.maven.wrapper.DefaultDownloader.download(DefaultDownloader.java:60)\n\tat org.apache.maven.wrapper.Installer.createDist(Installer.java:64)\n\tat org.apache.maven.wrapper.WrapperExecutor.execute(WrapperExecutor.java:121)\n\tat org.apache.maven.wrapper.MavenWrapperMain.main(MavenWrapperMain.java:55)\ntime=\"2020-01-18T07:51:09Z\" level=fatal msg=\"build failed: building [skaffold-jib]: build artifact: maven build failed: exit status 1\"\n"

--- FAIL: TestExpectedBuildFailures (1.94s)

    --- FAIL: TestExpectedBuildFailures/jib_is_too_old (1.94s)

        build_test.go:140: build failed but for wrong reason

Could you take a look at the Travis Test log because I do not understand how my changes are related to the failed tests?

@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Jan 18, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jan 18, 2020
@dgageot
Copy link
Contributor

dgageot commented Jan 18, 2020

@ErmakovDmitriy No worries, it's not related. There's an issue with the site we download Maven from. So all the Jib related tests are flakey today.

Copy link
Contributor

@dgageot dgageot left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'll do a second pass later when all the tests are green

@dgageot dgageot merged commit bcd903a into GoogleContainerTools:master Jan 27, 2020
@ErmakovDmitriy ErmakovDmitriy deleted the Env-Vars branch January 27, 2020 09:14
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.

4 participants