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

Fix kaniko default behaviour #1139

Merged
merged 2 commits into from
Oct 12, 2018

Conversation

priyawadhwa
Copy link
Contributor

If buildcontext isn't specified or localdir is specified, return localdir as the source
context. Otherwise, return a GCS bucket. If no value is passed in for
the bucket, one will be inferred from the project ID.

If buildcontext isn't specified or localdir is specified, return localdir as the source
context. Otherwise, return a GCS bucket. If no value is passed in for
the bucket, one will be inferred from the project ID.
@codecov-io
Copy link

codecov-io commented Oct 10, 2018

Codecov Report

Merging #1139 into master will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1139      +/-   ##
==========================================
+ Coverage   42.23%   42.31%   +0.08%     
==========================================
  Files          90       89       -1     
  Lines        3990     3982       -8     
==========================================
  Hits         1685     1685              
+ Misses       2137     2129       -8     
  Partials      168      168
Impacted Files Coverage Δ
pkg/skaffold/sync/sync.go 90.32% <0%> (-4.68%) ⬇️
cmd/skaffold/app/cmd/diagnose.go 0% <0%> (ø) ⬆️
pkg/skaffold/runner/runner.go 50.2% <0%> (ø) ⬆️
pkg/skaffold/kubernetes/sync.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 017cf58...71622f2. Read the comment docs.

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.

Setting default config should be done in

if err := c.withKanikoConfig(
so that BuildContext

@@ -35,7 +35,7 @@ type BuildContextSource interface {

// Retrieve returns the correct build context based on the config
func Retrieve(cfg *latest.KanikoBuild) (BuildContextSource, error) {
if cfg.BuildContext.LocalDir != nil {
if cfg.BuildContext == nil || cfg.BuildContext.LocalDir != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Setting default config should be done in

if err := c.withKanikoConfig(
so that BuildContext is never nil

when parsing the config so that buildcontext is never nil
@dgageot dgageot merged commit 1af9b55 into GoogleContainerTools:master Oct 12, 2018
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