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

Use latest claims code in cnab-go #1065

Merged
merged 12 commits into from Jul 6, 2020

Conversation

carolynvs
Copy link
Member

@carolynvs carolynvs commented Jun 4, 2020

What does this change

This gets porter using the latest claims spec that is now supported in cnab-go. It doesn't take advantage yet of new features that we could do with the additional data, just ensures that we still build and work.

What issue does it fix

Closes #935 Store claim revisions separately
Closes #836 Outputs from previous actions are lost
Closes #770 Keep Claim for uninstalled bundles

Notes for the reviewer

Checklist

  • Unit Tests
  • Documentation
  • Schema (porter.yaml) - not impacted

@carolynvs carolynvs added this to the Claims milestone Jun 4, 2020
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Looking great. A few items noticed while still in draft...

pkg/cnab/provider/action.go Outdated Show resolved Hide resolved
pkg/cnab/provider/install.go Outdated Show resolved Hide resolved
pkg/cnab/provider/uninstall.go Outdated Show resolved Hide resolved
pkg/cnab/provider/uninstall.go Outdated Show resolved Hide resolved
pkg/porter/list.go Outdated Show resolved Hide resolved
pkg/storage/pluginstore/store.go Outdated Show resolved Hide resolved
@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@carolynvs
Copy link
Member Author

I moved this out of draft because otherwise I don't get warned about merge conflicts. I still need to address your comments though!

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

LGTM!

pkg/cnab/provider/uninstall.go Outdated Show resolved Hide resolved
scripts/test/test-hello.sh Outdated Show resolved Hide resolved
@carolynvs carolynvs mentioned this pull request Jun 16, 2020
4 tasks
@carolynvs carolynvs changed the base branch from master to main June 20, 2020 21:30
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Re-iteration of the LGTM after reviewing commits since last approval.

@carolynvs carolynvs changed the base branch from main to release/v0.28.0 June 25, 2020 21:38
carolynvs-msft and others added 11 commits July 1, 2020 16:02
This gets porter using the latest claims spec that is now supported in
cnab-go. It doesn't take advantage yet of new features that we could do
with the additional data, just ensures that we still build and work.
Co-authored-by: Vaughn Dice <vadice@microsoft.com>
The integration tests don't use the PORTER_HOME env var, they
programmatically set the porter home directory on the porter app. So
when Porter creates a plugin, we need to explicitly set the home dir on
the plugin when it's instantiated since it won't get PORTER_HOME
automatically.
Bump cnab-go to get fix for displaying instance status. We need the
entire installation history so that we have the installation time (the
first claim), and ReadInstallation needs to read claims AND results.
* Change error handling when we can't find the install time
* Move definition of common runtime functions
* Remove accidental early return
ONE FUNCTION TO RULE THEM ALL!
@carolynvs carolynvs merged commit d88c780 into getporter:release/v0.28.0 Jul 6, 2020
@carolynvs carolynvs added this to Done in Porter and Mixins [OLD] via automation Jul 6, 2020
@carolynvs carolynvs deleted the bump-claims branch July 6, 2020 20:44
carolynvs added a commit that referenced this pull request Jul 27, 2020
* List all pages in a section

The template was assuming a paginator, but our template
doesn't use one, so print all pages

* Put release branches in a "directory" structure

We are using release/VERSION instead of release-VERSION, to take
advantage of tooling that collapses releases.

Allow for triggering tests against other gitflow named branches as well.

* Rename instances to installations (#1102)

* fixes to install scripts for path set up

* fixes to install scripts for path set up

* instances to installations change

* updates

* Update show.go

Accidentally changed showInstallations to showInstances but now changed it back

* claim to installation fixes

* claim to installation change in invoke_test.go and parameters_test.go

* Use latest claims code in cnab-go (#1065)

* Use latest claims code in cnab-go

This gets porter using the latest claims spec that is now supported in
cnab-go. It doesn't take advantage yet of new features that we could do
with the additional data, just ensures that we still build and work.

* Apply suggestions from code review

Co-authored-by: Vaughn Dice <vadice@microsoft.com>

* Fix test-cli

* Set PORTER_HOME in integration tests on plugins

The integration tests don't use the PORTER_HOME env var, they
programmatically set the porter home directory on the porter app. So
when Porter creates a plugin, we need to explicitly set the home dir on
the plugin when it's instantiated since it won't get PORTER_HOME
automatically.

* Fix show instance status

Bump cnab-go to get fix for displaying instance status. We need the
entire installation history so that we have the installation time (the
first claim), and ReadInstallation needs to read claims AND results.

* Update slide reference to claim file

* Resolve merge conflicts with paramset feature

* Incorporate review feedback

* Change error handling when we can't find the install time
* Move definition of common runtime functions
* Remove accidental early return

* Consolidate logic for cnab provider

ONE FUNCTION TO RULE THEM ALL!

* Move UseFilesystem onto TestContext

* bump cnab-go

* Final cnab-go bump v0.13.0-beta1

Co-authored-by: Vaughn Dice <vadice@microsoft.com>

* Fix TestDependencySolver_ResolveDependencies Flake

We were relying on an array order that was generated from iterating
upon a map. So it was unreliable in our tests. This ensures that
we are looking at the correct elements when evaluating the test results.

* Claim data migration (#1090)

* Migrate to new claim layout on read/list

When we request claim data, detect if it's the old layout and
automatically migrate all the claim data to the new layout. At the same
time detect older schema (e.g. when claim.Installation was claim.Name)
and handle that migration too.

When we cut to v1.0 we will stop supporting older claim data stores.

* Use a schema file at the root of home

Place a schema file at the root of home, schema.json, that says the
layout of the files used so we can detect if we should migrate.

{
  "claims": "CLAIM SCHEMA"
}

* Fill in the credentials spec version

* Param source (#1129)

* Refactor before parameter sources feature

* Add action to ActionArguments
* Replace cnab Install/Upgrade/Invoke/Uninstall with Execute
* Use claim.Action* from cnab-go instead of our own definition
  with manifest.Action*
* Fix loadParameters to apply string -> type conversions for all
parameters, not just those supplied on the command-line.

* Apply parameter sources when executing a bundle

When executing a bundle, look for the parameter sources extension and
apply any defined (we only support outputs as sources for now) and use
them as parameter values. The precedence order is:

0. user defined overrides on command-line
1. parameter sets
2. parameter sources
3. bundle defaults

* Use CreateClaim helper to shorten tests

* Incorporate review feedback

* Show installation history

Print out the history of what has happened to a bundle installation,
instead of just the last action when you run `porter show`.

I've added a new printer method that prints out a table section and I'll
follow up and redo other sections so that they use it too.

* Hold open plugin connection during schema migration (#1155)

* Hold open plugin connection during schema migration

* Make sure we don't open/close the plugin connection during the schema
migration check.
* Bubble through the Connect call to the wrapped backing store.

* Add failing test for panic during migration

* Reuse backing store during claims migration

Use the same backing store for manual access and for the claim store
itself so that we can better manage the connection, i.e. connecting one
connects the other.

* Trigger build all branches (#1157)

* Fix running integration tests for any branch

The * only matches a single directory deep. Removing the pr
configuration entirely triggers builds for all branches.

* Fix failing test

The test setup should ignore removing a directory that doesn't exist on
the CI server.

* Generate and resolve parameter sources from templating (#1158)

* Refactor before adding parameter sources

* Determine required extensions based on what has been populated in the
bundle.json, not by what is in the manifest. We know it is required
because we have already populated data for that extension.
* Some additional bundle.bundle pointer to value cleanup.
* Added extension.HasDependencies and extension.HasParameterSources
functions so we can check if we should attempt to read extension data or
not from a bundle.

* Generate parameter sources from porter templating

* Directly connect outputs to parameters in the manifest

* Consolidate logic in manifest

Consolidate logic for the wiring name or identifying outputs in the
manifest so that we can reuse it between the bundle adapter and the
runtime.

* Document parameter source in manifest

* Handle file type parameters in parameter sources

Test out the change in our exec-outputs example

* Simplify tests

* Update exec-outputs example to modify parameter-source

Have the exec-outputs example consume an ouput as a parameter then
modify it, similar to how terraform will do so with tfstate.

* Incorporate review feedback

Co-authored-by: gaurimadhok <gmadhok@usc.edu>
Co-authored-by: Vaughn Dice <vadice@microsoft.com>
@vdice vdice removed this from Done in Porter and Mixins [OLD] Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants