Reconcile installation upon apply#1764
Merged
carolynvs merged 5 commits intogetporter:release/v1from Sep 28, 2021
Merged
Conversation
Currently we only validate the resulting merged data structure before saving it to the database. But that doesn't catch when they import a document with an old schema. This will enforce that the schema of the important document is supported. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Moving the yq editor into the yaml package so that it's more of a generic editor for any yaml, not just the porter manifest. I'm going to use it in our tests. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Member
Author
|
/azp run porter-integration |
|
Azure Pipelines successfully started running 1 pipeline(s). |
When we capture output we should capture: * stdout only (so that we can parse it for json) * combined stdout + stderr, which is what the user would see in the console. This helps us search the output in tests, or just see what the user would see. * stderr as a Go error Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs
force-pushed
the
reconcile
branch
from
fbe911a to
01f8fa6
Compare
Reconcile the imported changes against the current state of the installation, and call the appropriate command to match the desired state. Right now we only trigger under the following conditions: * different bundle digest * different resolved parameters * different credential set _names_ (I don't compare resolved credentials) I've added a --force flag to apply so that you can fix stuff when porter detects no changes but things are borked in reality. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs
force-pushed
the
reconcile
branch
from
01f8fa6 to
8010fe3
Compare
Member
Author
|
@vdice Okay all the documentation is completed in the PR (links in the OP). Please take a look. |
vdice
reviewed
Sep 27, 2021
Member
vdice
left a comment
vdice
left a comment
There was a problem hiding this comment.
Looks great! Only really had a few clarifying questions/thoughts...
Do we want to resolve and compare credentials? Right now I am only comparing credential names because we do not store credential values used in a run.
Hmmm... not sure. Should we file an issue in the backlog (if not already there)?
| The following will result in Porter executing the bundle: | ||
| * The installation has not completed successfully yet. | ||
| * The bundle reference has changed. The bundle reference is resolved using the bundleRepository, bundleVersion, bundleDigest, and bundleTag fields. | ||
| * The resolved parameter values have changed, either because an associated parameter set has change, the parameters defined on the bundle have changed, or the values resolved by any parameter sets have changed. |
Member
There was a problem hiding this comment.
Suggested change
| * The resolved parameter values have changed, either because an associated parameter set has change, the parameters defined on the bundle have changed, or the values resolved by any parameter sets have changed. | |
| * The resolved parameter values have changed, either because an associated parameter set has changed, the parameters defined on the bundle have changed, or the values resolved by any parameter sets have changed. |
|
|
||
| ```yaml | ||
| parameters: | ||
| user: vdice |
Member
Author
|
I have added an issue to further improve reconciliation by storing hashes of the resolved parameters and credentials when Porter is run. |
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What does this change
Validate the input file during apply
Currently we only validate the resulting merged data structure before saving it to the database. But that doesn't catch when they import a document with an old schema.
This will enforce that the schema of the important document is supported.
Move yq editor into the yaml package
Moving the yq editor into the yaml package so that it's more of a generic editor for any yaml, not just the porter manifest. I'm going to use it in our tests.
Improve output capture in the smoke tests
When we capture output we should capture:
Call reconcile during porter installation apply
Reconcile the imported changes against the current state of the installation, and call the appropriate command to match the desired state. Right now we only trigger under the following conditions:
I've added a --force flag to apply so that you can fix stuff when porter detects no changes but things are borked in reality, and also a --dry-run flag so that someone can easily tell if porter detects any changes without running the bundle.
https://deploy-preview-1764--porter.netlify.app/quickstart/desired-state/
https://deploy-preview-1764--porter.netlify.app/end-users/installations/
https://deploy-preview-1764--porter.netlify.app/operator/
What issue does it fix
Closes #1704
Notes for the reviewer
Checklist