Skip to content

fix(ko): Fall back to build configs in .ko.yaml #6821

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

Merged
merged 2 commits into from
Nov 10, 2021

Conversation

halvards
Copy link
Contributor

@halvards halvards commented Nov 9, 2021

ko build configs are optional, and can either be specified in skaffold.yaml or .ko.yaml.

Prior to this change, the Skaffold ko builder would ignore any build configs present in .ko.yaml. This happened because we always passed a build config map with one entry, corresponding to the artifact being built.

With this change, build configs from .ko.yaml will be used as a fallback if the artifact config in skaffold.yaml does not specify any of the fields that are part of the build config. This is helpful to existing ko users who want to adopt Skaffold.

We achieve the fallback behavior by passing an empty build config map, which hits this conditional in ko: https://github.com/google/ko/blob/0015a815375d456baed846579ec02cf936b99307/pkg/commands/options/build.go#L120

Tracking: #6041

ko build configs are optional, and can either be specified in
`skaffold.yaml` or
[`.ko.yaml`](https://github.com/google/ko#overriding-go-build-settings).

Prior to this change, the Skaffold ko builder would ignore any build
configs present in `.ko.yaml`. This happened because we always passed
a build config map with one entry, corresponding to the artifact being
built.

With this change, build configs from `.ko.yaml` will be used as a
fallback if the artifact config in `skaffold.yaml` does not specify any
of the fields that are part of the build config. This is helpful to
existing `ko` users who want to adopt Skaffold.

We achieve the fallback behavior by passing an empty build config map,
which hits this conditional in `ko`:
https://github.com/google/ko/blob/0015a815375d456baed846579ec02cf936b99307/pkg/commands/options/build.go#L120

Tracking: #6041
@halvards halvards requested a review from a team as a code owner November 9, 2021 23:42
@halvards halvards requested a review from nkubala November 9, 2021 23:42
@google-cla google-cla bot added the cla: yes label Nov 9, 2021
@halvards halvards marked this pull request as draft November 9, 2021 23:49
@halvards
Copy link
Contributor Author

halvards commented Nov 9, 2021

Forgot to update the docs. Converting to draft while I do that.

@halvards halvards marked this pull request as ready for review November 10, 2021 00:02
@halvards halvards added the kokoro:run runs the kokoro jobs on a PR label Nov 10, 2021
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Nov 10, 2021
@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #6821 (9373eca) into main (290280e) will decrease coverage by 1.22%.
The diff coverage is 64.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6821      +/-   ##
==========================================
- Coverage   70.48%   69.26%   -1.23%     
==========================================
  Files         515      546      +31     
  Lines       23150    24986    +1836     
==========================================
+ Hits        16317    17306     +989     
- Misses       5776     6517     +741     
- Partials     1057     1163     +106     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/flags.go 91.00% <0.00%> (+0.18%) ⬆️
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/skaffold.go 76.19% <70.00%> (-8.43%) ⬇️
cmd/skaffold/app/cmd/inspect_build_env.go 65.11% <75.00%> (+6.39%) ⬆️
... and 156 more

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 66df86a...9373eca. Read the comment docs.

@tejal29 tejal29 enabled auto-merge (squash) November 10, 2021 19:59
@tejal29 tejal29 merged commit 38cabb5 into main Nov 10, 2021
@tejal29 tejal29 deleted the ko-yaml-build-config-fallback branch November 10, 2021 19:59
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.

3 participants