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

PEP 003 - Dependency Namespaces and Labels #8

Merged
merged 4 commits into from
Mar 3, 2022

Conversation

carolynvs
Copy link
Member

New PEP Checklist

  • Is your pull request named: PEP NNN - <TITLE>?
  • Have you posted your proposal in the forum?
  • Have you gotten approval and a PEP number from a maintainer?
  • Did you use the PEP Template?

Forum Suggestion Link

getporter/porter#1441

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs added this to In Progress in Porter and Mixins via automation Feb 24, 2021
For example, a manually set up on-premise shared database server, the 64 core beast that you keep in the office closet.
I want to automatically inject the connection string for the database into the bundles I install.

### Use Case: Polymorphic Resource
Copy link
Member

Choose a reason for hiding this comment

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

I might suggest that this Polymorphic Resource case and the broader discussion around Dependency Interfaces be split out into a separate proposal -- assuming, if I understand correctly, the rest of the use cases don't require the interface approach (but rather all hinge on namespaces and labels).

In doing so, we can narrow the scope for this proposal and reserve discussion for polymorphic dependencies for its own proposal -- which may prove rich in detail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that interfaces are part of the upcoming CNAB spec, I've expanded on how we will resolve interfaces and kept this use case in the proposal.

pep/003-dependency-namespaces-and-labels.md Outdated Show resolved Hide resolved
pep/003-dependency-namespaces-and-labels.md Outdated Show resolved Hide resolved
pep/003-dependency-namespaces-and-labels.md Outdated Show resolved Hide resolved

New configuration at the bundle level and at the point when a bundle action is run should allow people to control this behavior.

All bundle actions (install, upgrade, invoke, uninstall) have a common flag, `--dependencies=true|false`, that let the user override the dependency behavior set in the bundle. This flag controlls whether or not Porter's dependency management step should be executed or not, and defaults to true.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should elaborate on how --dependencies=true|false interacts with bundles that have explicit management declarations (as below). For instance, if a bundle has manages: false for the dependency it declares, I would assume running porter install defaults to --dependencies=false, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have removed lifecycle management from the scope of the proposal. We will need to discuss it further and decide how to manage an installation with dependencies going forward, and this proposal will focus just on defining and resolving an installation's dependencies.

uninstall: false
```


Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should have a section elaborating on the behavior of multiple bundles sharing a dependency and one or more may declare that it is managing the dependency... but we wouldn't want to allow any single bundle to uninstall the dep, right? (Arguably, we might not want any single bundle to perform any action on the shared dependency installation...)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a great question! I'll make sure to include that in the follow-up proposal around lifecycle management, which will get into identifing who owns which installation, what gets upgraded and deleted, etc.


The dependencies section in porter.yaml is changing its format.
Currently the list of required bundles is directly underneath dependencies, and this proposal changes it to be under dependencies.requires.
Since we are pre 1.0, it's an acceptable breaking change but we will need to warn people before the release and call it out in the release notes.
Copy link
Member

Choose a reason for hiding this comment

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

Do we wish to make an effort to support the older way of declaring deps (support older manifests) and just run the 'classic' mode of dependency lifecycle management (and one dep per bundle) when the older bundle is installed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that we should make a clean break and support dependencies in only the improved model outlined here. There are a lot of unaddressed bugs and gaps in the original implementation that I would like to leave behind that this proposal answers (such as passing outputs between dependencies, and improved wiring of passing inputs to dependencies).


## Open Questions

* I'm not sure yet how to handle passing credentials to the dependency that implements an interface, as that isn't possible to standardize in the interface.
Copy link
Member

Choose a reason for hiding this comment

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

Yea, this is tricky... and ripe for brainstorming!

Copy link
Member Author

Choose a reason for hiding this comment

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

This is now answered in the Q&A section "How can we use an interface when implementations have different credentials?"

I'm not sure that I have a use case for allowing namespace to be set to anything, we certainly don't want it hardcoded or again invite people to create parameters for it.
For example instead of `namespace: "{{ installation.namespace }}"`, we could use `scope: namespace|global` which translates to that under the covers.
By limiting the values in the porter.yaml, we would be making it easier to do it the right way.
* For the `dependencies.lifecycle` do we need both managed and the actions? I'm concerned it will confuse people since they are intended to be one or the other and not merged.
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm good Q... not sure here myself. Good to raise, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Deferring to the follow-up lifecycle management proposal.

For example instead of `namespace: "{{ installation.namespace }}"`, we could use `scope: namespace|global` which translates to that under the covers.
By limiting the values in the porter.yaml, we would be making it easier to do it the right way.
* For the `dependencies.lifecycle` do we need both managed and the actions? I'm concerned it will confuse people since they are intended to be one or the other and not merged.
* Does user provided resources (i.e. creating a meta bundle for an existing resource created outside of porter) feel like _too_ much of a hack?
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps. My initial thinking was: Why not just forgo the meta dep bundle altogether and just bake the req'd outputs into the parent bundle as params/creds? e.g., if my bundle needs a mysql connection string and I already have an external resource that can fulfill that need, just send that value in via a bundle param or cred?

I suppose it makes more sense when the parent bundle isn't one I have written or manage. Say it's one that is already published that I can pull down and that requires a mysql dependency. And, say I have that external mysql resource... then, indeed, I'd need to create a meta bundle for it in this instance to use the parent bundle -- right?

Copy link
Member Author

Choose a reason for hiding this comment

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

The cool thing about making a bundle/installation to represent a resource that is not managed by Porter is that going forward, when another bundle depends upon something that matches its interface, Porter can automatically resolve it.

So for example, in an environment with a bare metal sql server, the sysadmin can represent it with a bundle/installation, and then anyone who uses porter in that environment will automatically resolve a sql sql server dependency to it.

```

```console
$ porter install --reference myapp:v1.0.0 --dependency sqlserver=shared-dev-sql-server
Copy link
Member

Choose a reason for hiding this comment

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

Question: If I forget to supply/specify the same --dependency on the next action on the installed bundle here (say, upgrade), what should the behavior be? Perhaps Porter warns, something along the lines of, "It looks like this installation is currently using the specified dependency installation, but this action would require <either the default dependency declared in the bundle, or, if another value was supplied via --dependency, that value>"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking (but it's not in the PEP yet) is that we need a dependency lockfile that has the resolved graph, including user overrides. So when you override with --dependency, the graph is updated to reflect that. When you do an upgrade, you won't need to re-specify.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll make sure to transfer this over to the follow-up lifecycle management proposal.

@carolynvs
Copy link
Member Author

After chatting with @MChorfa and @vdice, here are the PEPs we think that will be necessary to fully implement dependencies as we have discussed so far.

  1. Labels and Namespaces
  2. Polymorphic Resources (having a bundle satisfy a required interface)
  3. Dependency Graph resolution and lockfiles (deep resolution, preserving state of what was resolved in a lockfile)
  4. Dependency Lifecycle Management (i.e. how to upgrade when dependencies are involved)

carolynvs added a commit to carolynvs/porter-proposals that referenced this pull request Mar 22, 2021
Organize and isolate porter documents using labels and namespaces.

Originally proposed as part of PEP 003: Dependency Namespaces and
Labels. getporter#8

This lays storage foundation that PEP003 relies upon.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs mentioned this pull request Mar 22, 2021
4 tasks
carolynvs added a commit to carolynvs/porter-proposals that referenced this pull request Mar 22, 2021
Organize and isolate porter documents using labels and namespaces.

Originally proposed as part of PEP 003: Dependency Namespaces and
Labels. getporter#8

This lays storage foundation that PEP003 relies upon.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit to carolynvs/porter-proposals that referenced this pull request Mar 22, 2021
Organize and isolate porter documents using labels and namespaces.

Originally proposed as part of PEP 003: Dependency Namespaces and
Labels. getporter#8

This lays storage foundation that PEP003 relies upon.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit to carolynvs/porter-proposals that referenced this pull request Mar 22, 2021
Organize and isolate porter documents using labels and namespaces.

Originally proposed as part of PEP 003: Dependency Namespaces and
Labels. getporter#8

This lays storage foundation that PEP003 relies upon.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit to carolynvs/porter-proposals that referenced this pull request Mar 22, 2021
Organize and isolate porter documents using labels and namespaces.

Originally proposed as part of PEP 003: Dependency Namespaces and
Labels. getporter#8

This lays storage foundation that PEP003 relies upon.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit that referenced this pull request Mar 30, 2021
* PEP 004 - Labels and Namespaces

Organize and isolate porter documents using labels and namespaces.

Originally proposed as part of PEP 003: Dependency Namespaces and
Labels. #8

This lays storage foundation that PEP003 relies upon.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* Fix copy/paste error

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* Review feedback

* Clarify how labels are applied to installations
* Link to open upstream PR for namespaces and labels

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* Specify custom porter.sh/ label prefix and behavior

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* Clarify how to query across namespaces

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>

* Update commands to include labels and namespace

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs
Copy link
Member Author

Sorry that this hasn't been updated yet. I am starting to implement namespaces and labels for the CNAB installation spec and we should get this in since it's all related. I'll address your comments this week.

My application has 4 micro-services and a shared postgres database.
When I install the first service, a database should be created for it.
When I install the other services, I want to share the database (and the connection string) between them.

Copy link

Choose a reason for hiding this comment

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

For instance, when using helm mixinm the release name could be used as identifier and helm will insure the idempotence of the operation

Copy link

Choose a reason for hiding this comment

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

Use Case: Bundle to Bundle Reference

The current bundle is dependents on another bundle to allow cascading mechanism.
The parameters provided to the root bundle should be propagated to all inner bundles

#### Interface Selector

Bundles can declare that it implements a well-known bundle interface, which encompasses the app installed by the bundle (mysql), the application version (5.7.12), parameters and credentials required by the bundle, and the outputs generated by the bundle (connection string).

Copy link

Choose a reason for hiding this comment

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

Is the interface is kind of release definition?

dependencies:
requires:
- name: sqlserver
reference: azure/sqlserver # Use this bundle if we can't find an existing one
Copy link

Choose a reason for hiding this comment

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

Bundle should be deployed as OCI package and allow reference to that package using name + tag

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry I'm not understanding what the question is? Porter already supports requiring a dependency by just its OCI reference.

```console
$ porter install myapp --reference myorg/mybundle --dependencies=false --cred logger
# the logger credential contains the logger endpoint normally output by the logger dependency
```
Copy link

Choose a reason for hiding this comment

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

How to push more parameters from the top bundle to the inner one?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a known gap and is being tracked by this issue getporter/porter#990.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated my answer based on recent changes to the revised CNAB Dependencies spec. See "Wiring Dependencies" section for more details.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs self-assigned this Feb 19, 2022
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Clarify how to set creds/params of a dependency

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs
Copy link
Member Author

@vdice I've updated the proposal based on recent changes to the revised CNAB Dependency spec. Can you take another look?

Based on the PEP process, I've been doing this wrong. We should merge a proposal once it meets the bar of "we want to solve this and it's going down the right path". Proposals should be merged quickly so that other people can open issues and contribute to the PEP instead of having a year long PR that is constantly getting tweaked. 😁 I should have merged this long ago and just made new PRs against this PEP as we figured things out.

@carolynvs carolynvs merged commit 26246a8 into getporter:main Mar 3, 2022
Porter and Mixins automation moved this from In Progress to Done Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants