cnabio / cnab-spec

Cloud Native Application Bundle Specification
https://cnab.io
Other
956 stars 99 forks source link

Split claim into execution inputs and results #339

Closed carolynvs closed 4 years ago

carolynvs commented 4 years ago

Any fields that are modified based on the bundle execution: outputs, modified have moved to claim.results. The action, e.g. "install", has always been an input and has moved out of result to the top level claim.

Now claim (with the exception of its child result) does not change after creation. A tool could create a digest from it if needed.

Also results.outputs has been modified to only contain the output names instead of the entire output contents, which can be prohibitively large. The tool can request the outputs at the end of the operation to persist them separate from the claim.

Closes #329 - Move output names into the results sub-document Closes #238 - Move action to the top level claim document Closes #326 - Store outputs outside of the claim

This is a breaking change to Porter at least, which relied on having the outputs in the claims to make bundle dependencies work. A follow-up change to the dependency spec will be necessary.

carolynvs commented 4 years ago

When @vdice and I were discussing the retry case, we were having a hard time understanding whether or not a retry would be able to reuse a claim as-is (i.e. not change the revision):

revision (REQUIRED): An ULID that MUST change each time the claim is modified. It MUST NOT change when a non-modifying operation is performed on an installation.

from Claims Spec

Now that we are working to make all of the fields on the claim (other than result) immutable, this text is not longer clear.

jlegrone commented 4 years ago

When @vdice and I were discussing the retry case, we were having a hard time understanding whether or not a retry would be able to reuse a claim as-is (i.e. not change the revision)

I think there are several reasons why a claim could be reused, and that it should be up to the runtime to determine whether this is the right approach. For instance, the Kubernetes driver could return an error result when there are no compute resources available in the cluster to run an invocation image, but a higher-order driver could then pass off the same claim to a secondary driver instance running in a different cluster.

Another use case would be to allow a client to retry stateless or modifies=false actions without generating a new claim. Stateless action results could even be cached by the runtime in some cases, though the spec doesn't require invocation image outputs to be deterministic.

technosophos commented 4 years ago

The purpose of a claim is to record an event/action that mutates the resources of a cluster as a result of a CNAB operation.

Any operation that does not mutate the installed sources must not generate a new claim (e.g. if an action has modifies=false, it must not generate a claim, and this is probably true of all stateless commands as well). Claims are to record any case where the managed artifacts were modified by a CNAB tool. (If I need to clarify this more, I will. I can see how this might not be totally clear from 400 alone)

Ultimately, it is up to the runtime to determine whether it will "reuse a claim", but things will get awfully confusing if some systems decide not to record new claims when they change installations.

jlegrone commented 4 years ago

Any operation that does not mutate the installed sources must not generate a new claim (e.g. if an action has modifies=false, it must not generate a claim, and this is probably true of all stateless commands as well). Claims are to record any case where the managed artifacts were modified by a CNAB tool. (If I need to clarify this more, I will. I can see how this might not be totally clear from 400 alone)

I'm not sure this works given the current model of one claim embedding one result. What should a runtime do if a user runs a modifies=false action that generates an output? Modify the last claim? Do we want the spec to lay out how runtimes should maintain an audit log of all actions that were executed against an installation, including stateless and modifies=false?

technosophos commented 4 years ago

I am not convinced that outputs/results really do belong in claims. I would rather seem them as separate. The big question for me is what do we want the relationship to be between claims and results? Can we just have a claim point to an output (or vice-versa)?

My database brain does not like the idea of just nesting one inside the other, because regardless of which way we structure it, we will be saying one of them "belongs" to the other, which isn't necessarily true. In my mind, while the claim is intended to capture the responsibility for a change to resources, a "result" is merely the stored output of any operation (regardless of whether it changed anything).

One could imagine a system that stored claims, but not results.

One can imagine cases where one is only really concerned with results from non-mutating actions, but not really with claims at all.

All of that said, I'm certainly open to being convinced otherwise.

carolynvs commented 4 years ago

/brig run

carolynvs commented 4 years ago

Perfect, yeah let's find a way for result to live on its own in a separate PR then. I'm not a codeowner, so once y'all are happy with this one, would someone else merge please?