This repository was archived by the owner on Feb 6, 2025. It is now read-only.
Enhance skuba addon upgrade [plan|apply] #1247
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 5 commits
July 13, 2020 10:52
Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>
executes `skuba addon upgrade plan`, besides display new addon or addon to be upgrade, also try server-side validation to validates the base+patch manifests. Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>
perform check local addons manifests before - `skuba addon upgrade plan` - `skuba addon upgrade apply` Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>
add a unit test to check local addons base manifests behavior is as expected. Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>
jenting
requested review from
mssola,
innobead,
flavio,
maximenoel8,
c3y1huang,
cclhsu and
evrardjp
innobead
reviewed
Jul 13, 2020
innobead
reviewed
Jul 13, 2020
innobead
approved these changes
Jul 13, 2020
evrardjp
reviewed
Jul 14, 2020
| @@ -340,28 +370,39 @@ func (addon Addon) Write(addonConfiguration AddonConfiguration) error { | |||
| // applyPreflight applies the preflight deployment/daemonset manifest if such | |||
| // is defined by the addon. It returns a bool whether preflight was defined | |||
| // by the addon and an error. | |||
| func (addon Addon) applyPreflight(addonConfiguration AddonConfiguration, sandboxDir string) (bool, error) { | |||
| func (addon Addon) applyPreflight(addonConfiguration AddonConfiguration, rootDir string, dryRun bool) (bool, error) { | |||
There was a problem hiding this comment.
Why the change in the rootDir?
There was a problem hiding this comment.
Just make the naming all the same cross files 😃
Itxaka
pushed a commit
to Itxaka/skuba
that referenced
this pull request
Aug 4, 2020
`skuba addon upgrade plan` will: 1. check local base addon manifest is up-to-date 2. display new addon or addon to be upgraded 3. generates kustomization.yaml inside addons/<addon> 4. executes server-side validation to validates the base+patch manifests `skuba addon upgrade apply` will: 1. check local base addon manifest is up-to-date 2. generates kustomization.yaml inside addons/<addon> 3. applied new addon or addon to be upgraded into the current cluster Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why is this PR needed?
The users would use kustomize to overlay the default base addon manifest value.
However, we did not perform a dry-run check before applied to the current Kubernetes cluster, the user provides kustomize patch manifests might have some problem like manifest indentation error.
Therefore, for
skuba addon upgrade plan, we not only display is there any new addon or existed addon need to be upgraded, moreover, we also perform the Kubernetes server-side validation by runningkubectl apply -k addons/<addon> --dry-run=serverand outputs the stderr to the user if present.Besides, we'll generate
kustomization.yamlinsideaddons/<addon>to make the user have the ability to runkubectl apply -k addons/<addon>after addons upgraded because the user might want to add more patches into the cluster and make sure the patches manifests will keep overtime for each addon upgrade.By the way, before actually perform the
skuba addon upgrade [plan|apply], we will check the local addons base manifests match to the current Kubernetes cluster version, to make sure it's up-to-date.The command
skuba addon refresh localconfigis in another PR #1226 to not make a single PR too large.Fixes https://github.com/SUSE/avant-garde/issues/1765
What does this PR do?
skuba addon upgrade [plan|apply].kustomization.yamlinsideaddons/<addon>forskuba addon upgrade [plan|apply]skuba addon upgrade planAnything else a reviewer needs to know?
N/A
Info for QA
This is info for QA so that they can validate this. This is mandatory if this PR fixes a bug.
If this is a new feature, a good description in "What does this PR do" may be enough.
Related info
An updated RFC for this PR.
Status BEFORE applying the patch
kustomization.yamlwon't exist insideaddons/<addon>.skuba addon upgrade planwon't do Kubernetes server-side dry-run.Status AFTER applying the patch
skuba addon upgrade [plan|apply]kustomization.yamlwill exist afterskuba addon upgrade [plan|apply]skuba addon upgrade planwill do Kubernetes server-side dry-run.Docs
SUSE/doc-caasp#923
Merge restrictions
(Please do not edit this)
We are in v4-maintenance phase, so we will restrict what can be merged to prevent unexpected surprises: