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

feat: dry-run's client/server/none options with backwards compat #139

Merged
merged 5 commits into from
Mar 31, 2023

Conversation

colindean
Copy link
Contributor

@colindean colindean commented Dec 21, 2022

kubectl allows options [client|server|none] instead of just a boolean, starting at some version I'm not sure of. This enables that behavior with a fallback to if the user has passed true or false so folks don't need to update their configurations.

I've mapped true->client and false->none but otherwise treating the parameter as a string.

This is the warning seen when running plugin v0.3.0:

W1221 21:38:34.228830      22 helpers.go:653] --dry-run=false is deprecated (boolean value) and can be replaced with --dry-run=none.

kubectl allows options [client|server|none] instead of just a boolean,
starting at some version I'm not sure of. This enables that behavior
with a fallback to if the user has passed true or false so folks don't
need to update their configurations.

I've mapped true->client and false->none but otherwise treating the
parameter as a string.
@colindean colindean requested a review from a team as a code owner December 21, 2022 22:17
@codecov
Copy link

codecov bot commented Dec 23, 2022

Codecov Report

Merging #139 (082ec5a) into main (e22275c) will increase coverage by 0.38%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   60.00%   60.38%   +0.38%     
==========================================
  Files           9        9              
  Lines         615      621       +6     
==========================================
+ Hits          369      375       +6     
  Misses        244      244              
  Partials        2        2              
Impacted Files Coverage Δ
cmd/vela-kubernetes/main.go 0.00% <0.00%> (ø)
cmd/vela-kubernetes/apply.go 98.46% <100.00%> (+0.15%) ⬆️

@colindean
Copy link
Contributor Author

This getting kinda annoying the Vela logs:

WARNING: version difference between client (1.24) and server (1.21) exceeds the supported minor version skew of +/-1
$ /bin/kubectl --kubeconfig=/root/.kube/config apply --dry-run=false --filename=k8s/myproject/training_pvc.yaml
W0126 20:50:13.520375      22 helpers.go:653] --dry-run=false is deprecated (boolean value) and can be replaced with --dry-run=none.
persistentvolumeclaim/myteam--myproject--training--output unchanged
$ /bin/kubectl --kubeconfig=/root/.kube/config apply --dry-run=false --filename=k8s/anotherproject/training_pvc.yaml
W0126 20:50:14.763196      41 helpers.go:653] --dry-run=false is deprecated (boolean value) and can be replaced with --dry-run=none.
persistentvolumeclaim/myteam--anotherproject--training--output unchanged
$ /bin/kubectl --kubeconfig=/root/.kube/config apply --dry-run=false --filename=k8s/anotherproject/training_configmap.yaml
W0126 20:50:15.177479      59 helpers.go:653] --dry-run=false is deprecated (boolean value) and can be replaced with --dry-run=none.
configmap/myteam--anotherproject--training--config unchanged
$ /bin/kubectl --kubeconfig=/root/.kube/config apply --dry-run=false --filename=k8s/anotherproject/tag-and-deploy_configmap.yaml
W0126 20:50:15.588353      78 helpers.go:653] --dry-run=false is deprecated (boolean value) and can be replaced with --dry-run=none.
configmap/myteam--anotherproject--tag-and-deploy--config created

Can this PR get merged and released?

@plyr4 plyr4 changed the title Enables dry-run's client/server/none options with backwards compat feat: dry-run's client/server/none options with backwards compat Mar 30, 2023
cmd/vela-kubernetes/apply_test.go Outdated Show resolved Hide resolved
cmd/vela-kubernetes/apply_test.go Outdated Show resolved Hide resolved
cmd/vela-kubernetes/apply.go Outdated Show resolved Hide resolved
@plyr4
Copy link
Contributor

plyr4 commented Mar 30, 2023

hey @colindean sorry to review this so late, but it looks great to me, thank you

there are a couple tiny linter issues that can be resolved pretty easy so i left some suggestion comments.
once we get those i can approve :)

@colindean colindean requested a review from plyr4 March 30, 2023 20:45
@plyr4
Copy link
Contributor

plyr4 commented Mar 30, 2023

lgtm, ty! :D

@wass3rw3rk wass3rw3rk merged commit 28bbcc3 into go-vela:main Mar 31, 2023
@colindean colindean deleted the dryrun-newer-options branch May 3, 2023 16:49
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