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

[kpt deployer] Add "local-config" annotation to kpt fn configs. #4803

Merged
merged 2 commits into from
Sep 19, 2020

Conversation

yuwenma
Copy link
Contributor

@yuwenma yuwenma commented Sep 18, 2020

Related #3904

Description

Add a config.kubernetes.io/local-config:true annotation to each kpt fn config from the rendered resources. This allows kpt deployer to skip deploying the kpt fn config to the cluster in skaffold dev and skaffold deploy.

more context: The declarative kpt fn is not config resource but an operation working on the resource. It should not be applied to the cluster. Applying the kpt fn to cluster could fail due to the different config requirements (e.g. kpt fn ConfigMap doesn't require the .metadata.name field to exist while normal ConfigMap does).

The `config.kubernetes.io/local-config:true` annotation will skip the kpt
 fn resource from deploying to the cluster in `kpt live apply`.
@yuwenma
Copy link
Contributor Author

yuwenma commented Sep 18, 2020

/assign @MarlonGamez
/cc @nkubala

@yuwenma yuwenma assigned yuwenma and MarlonGamez and unassigned yuwenma Sep 18, 2020
@codecov
Copy link

codecov bot commented Sep 18, 2020

Codecov Report

Merging #4803 into master will decrease coverage by 0.20%.
The diff coverage is 84.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4803      +/-   ##
==========================================
- Coverage   71.82%   71.62%   -0.21%     
==========================================
  Files         347      348       +1     
  Lines       11869    12051     +182     
==========================================
+ Hits         8525     8631     +106     
- Misses       2725     2781      +56     
- Partials      619      639      +20     
Impacted Files Coverage Δ
pkg/skaffold/deploy/kpt.go 84.61% <84.00%> (-1.27%) ⬇️
pkg/skaffold/build/tag/util.go 89.47% <0.00%> (-10.53%) ⬇️
pkg/skaffold/deploy/helm.go 74.61% <0.00%> (-2.54%) ⬇️
pkg/skaffold/debug/debug.go 38.59% <0.00%> (-2.15%) ⬇️
pkg/skaffold/runner/new.go 64.05% <0.00%> (-1.92%) ⬇️
pkg/skaffold/deploy/kustomize.go 76.19% <0.00%> (-0.24%) ⬇️
pkg/skaffold/kubectl/cli.go 100.00% <0.00%> (ø)
pkg/skaffold/deploy/kubectl/cli.go 87.34% <0.00%> (ø)
cmd/skaffold/app/cmd/filter.go 26.19% <0.00%> (ø)
pkg/skaffold/deploy/kubectl.go 66.00% <0.00%> (+0.26%) ⬆️
... and 4 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 9fb4e81...4e60daf. Read the comment docs.

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

Looks good, just have a suggested fix for the imports and a small nit

pkg/skaffold/deploy/kpt.go Outdated Show resolved Hide resolved
pkg/skaffold/deploy/kpt.go Outdated Show resolved Hide resolved
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

Looks good!

@MarlonGamez MarlonGamez merged commit e1339dd into GoogleContainerTools:master Sep 19, 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.

3 participants