-
Notifications
You must be signed in to change notification settings - Fork 179
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
[RFC] App Manifests v2 #1019
base: main
Are you sure you want to change the base?
[RFC] App Manifests v2 #1019
Conversation
- Update network policy section to reflect thinking about cross-space resource sharing
- Add fields for provisioning/updating service instances - Add user-provided service instances - Break service bindings into their own section. They don't fit well nested under service instances: Apps depend on services; services don't depend on apps. For "app" bindings, it might make sense to nest under apps, but "key" and "route" bindings don't make sense under apps. Instead, break bindings into a top-level node.
- Fix inconsistent health check configuration - Add `scale` node to processes to organize related nodes
- To add more hierarchy and closer match API design
- Small style updates and typo fixes - Add additional risk to declarative manifests
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking the time to write this down! I really like the overall approach.
available with v2 manifests. These extensions are less-developed than the core | ||
v2 manifest design, and may change significantly prior to implementation. | ||
|
||
#### Merge State |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not that active on the developer side of CF so take this comment with a grain of salt.
The way the RFC is written the use-case of managing the entire space in a single manifest seems to be the default (looking at the listed issues I can see why that is), conversely managing only a sub-set of resources is supported but requires additional configuration and might not be supported initally. Does that reflect how users typically use CF (manifests)?
Intuitively CF feels very app-centred for me and for the few apps we have the manifest is checked-in together with the app code. No assumptions about the space are made and the space might be shared between multiple apps in a similar style.
Taking inspiration from Kubernetes: cf push
on a full space manifest seems similar to kubectl apply
of a multi-document YAML file or directory and it will only ever add & modify resources by default. Deletion is supported via --prune
and has to be explicitly set.
I would prefer if we go a similar route: resource themselves are fully declarative but a resource not listed should not be deleted implicitly following the principle of least astonishment (imagine the surprise of deleting a production database upon pushing a hello-world app to the same space).
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking further about this most probably there should be a different delete strategy for resources referenced by an application like a service binding
or a sidecar
. They should be deleted when they are not referenced any more by the application because they are "part" of the application definition. E.g. it is painful to delete a sidecar
now days because you have to go over CF APIs. There is no support in the CF CLI or app manifest for this. Would be interested to see more comments on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm also uncertain about how aggressive the current proposal is.
My primary goal is to have something that can easily be explained. It's easier to explain "everything is declarative", vs "configuration of resources is declarative, but the set of resources themselves are not declarative, unless you pass a --prune flag", especially when the boundary between a "resource" and "configuration of a resource" is murky.
Taking inspiration from Kubernetes: cf push on a full space manifest seems similar to kubectl apply of a multi-document YAML file or directory and it will only ever add & modify resources by default. Deletion is supported via --prune and has to be explicitly set.
Yea, the big distinction for me here is that k8s breaks things into multiple documents, so there is a clear boundary between what is declarative-by-default vs what is not. I could imagine a more revolutionary manifest proposal, where we do something similar for CF:
---
kind: app
spec:
name: my-app
---
kind: service_instance
spec:
name: my-service-instance
The other inspiration is BOSH, which more-or-less has a single manifest that is strictly declarative.
I would prefer if we go a similar route: resource themselves are fully declarative but a resource not listed should not be deleted implicitly following the principle of least astonishment (imagine the surprise of deleting a production database upon pushing a hello-world app to the same space).
Yes, this resonates with me. I'll think about this some more. I'm eager to hear additional feedback/suggested as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My primary goal is to have something that can easily be explained. It's easier to explain "everything is declarative", vs "configuration of resources is declarative, but the set of resources themselves are not declarative, unless you pass a --prune flag", especially when the boundary between a "resource" and "configuration of a resource" is murky.
I agree, this needs careful consideration and there would need to be a strong convention to prevent the varying behaviours we have in v1 manifests.
Taking inspiration from Kubernetes: cf push on a full space manifest seems similar to kubectl apply of a multi-document YAML file or directory and it will only ever add & modify resources by default. Deletion is supported via --prune and has to be explicitly set.
Yea, the big distinction for me here is that k8s breaks things into multiple documents, so there is a clear boundary between what is declarative-by-default vs what is not. I could imagine a more revolutionary manifest proposal, where we do something similar for CF: [...]
The other inspiration is BOSH, which more-or-less has a single manifest that is strictly declarative.
The difference to BOSH is that BOSH only has one manifest managing one resource. In that sense CF is probably closer to Kubernetes than BOSH. On the other hand moving towards Kubernetes-like primitives could also send the wrong signal to users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tim also pointed me to this blog post, which is an interesting industry survey of declarative deletion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the blog post link! A really good summary for declarative deletion. Based on the experience shared in the blog it reads to me that a well supported imperative deletion should be our priority. Extending that to the app manifest could be done in a later point if desired by users. Regarding the imperative approach if we want to have a resource-by-resource "pruning" my understanding is that we will need something like:
cf push -f manifest.yml --prune_apps=app-1,app-2 --prune_routes=route-1,route-2
and on API side:
POST /v3/spaces/:guid/actions/apply_manifest?prune_apps=app-1,app-2&prune_routes=route-1,route-2
@Gerg Is that what you meant in your example above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My example was inspired by kubectl apply
's --prune-allowlist
flag [docs]. You give it a list of the kinds of resource you want to delete. For example:
kubectl apply --prune -f manifest.yaml --all --prune-allowlist=core/v1/ConfigMap
would prune all excess ConfigMaps
, but leave other kinds of resources alone.
My intention was for:
cf push -f manifest.yml --prune=apps,routes
to prune excess apps and routes, but leave other resources alone.
Regarding imperative deletion, I think we already have that well covered via the cf delete-*
commands. For example, if the developer already has a list of specific routes to delete, it doesn't seem much harder to pass those routes to cf delete-route
vs a --prune-routes
flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking into this from the perspective where one space is managed by multiple manifests and then the approach with cf push -f manifest.yml --prune=apps,routes
doesn't fit so well but I can see that for the use case you described above:
The motivating usecase that we're designing around is an upstream continuous deployment system that assembles desired state upstream, builds a space-level manifest, and applies it to CF, ideally without requiring imperative commands. We're probably over-fitting this design to that vision.
it makes sense.
I agree with:
Regarding imperative deletion, I think we already have that well covered via the cf delete-* commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I substantially updated the declarative resource deletion section in 17db5d1, based on this thread. Let me know what you think.
available with v2 manifests. These extensions are less-developed than the core | ||
v2 manifest design, and may change significantly prior to implementation. | ||
|
||
#### Merge State |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
cc
Feel free to cc any other WG areas y'all think are affected by this RFC. |
This comment was marked as resolved.
This comment was marked as resolved.
- Wikipedia article was low quality (per their own standards) - Based on PR feedback - Add list of additional references
- Remove disclaimer about tolorating hyphenated keys - This was due to a misunderstanding about the original RFC. The per-route options are NOT opaque to CC.
- Update `docker` lifecycle app to use consistent access_credentials format
Hello CF colleagues, I’m writing on behalf of the MultiApps project, part of WG App Runtime Interfaces, team at SAP. Our service focuses on managing the lifecycle of microservices (Cloud Foundry apps), service instances, routes, bindings, and keys. These microservices are referred to as multitarget applications, which share many similarities with the current RFC. In our approach, applications, service instances, and routes are defined in a deployment descriptor file called mtad.yaml. The applications themselves are packaged into a single file, ((MTA)).mtar, which adheres to the JAR specification. The service operates in a relatively declarative manner: when redeploying, it removes applications that have been deleted from the descriptor. However, service instances that are no longer part of the descriptor are not removed unless the user explicitly specifies the --delete-service-instances option in the CLI. The service consists of a client application and server application: Client: https://github.com/cloudfoundry/multiapps-cli-plugin Server: https://github.com/cloudfoundry/multiapps-controller Examples: https://github.com/SAP-samples/cf-mta-examples We think both App Manifest v2 and MultiApps Controller have pros and cons:
Our questions and concerns
|
Thank you for your thorough analysis of v2 manifests vs MultiApps. v2 manifests are intended to be a refresh for the existing v1 manifest interface and capabilities: an API-native way to apply bulk configuration. They are not intended as a replacement for MultiApps. We're not planning to introduce the advanced packaging and deployment configuration that is possible with MultiApps. Devs that want the features of MultiApps can and should continue to use them. I'm not familiar with the implementation of MultiApps, but maybe there are some opportunities for the MultiApps server to leverage v2 manifests under the hood. Regarding the questions/concerns, a lot of these details are specific and would need to be worked out during development. In general, I would expect the behavior to be similar to v1 manifests, when applicable. Don't take the following as definitive, but these are my first pass at answering the questions.
Bindings will behave the same as v1 manifests: no re-create. Same for keys. This could be a good value-add for MultiApps.
SIs are only updated if plan or parameters change.
Not currently. This is probably another good example of a MultiApps advanced feature.
No
Manifest schema version won't affect how deployments work. This will be the same as with v1 manifests (for better or worse).
Only what exists currently (via
Manifests only affect a single space. The only way they could figure out what is provided by manifests is by looking at audit events.
We are actively discussing this above, but we're leaning towards making pruning opt-in for each resource.
This will be the same as v1 manifests. You can write a manifest that can be applied to different spaces, but it's not guaranteed to work.
We don't have an opinion on this. Manifests bulk-apply configuration to a space. We leave it up to users and other tooling to be opinionated about how apps and spaces are managed.
This will be the same as with v1 manifests. It will be across multiple requests (manifest vs bits upload), but the bulk of the configuration is applied asynchronously.
We were only considering the
We will apply all configuration that is present in the manifest.
We hadn't considered registering space-scoped brokers, but that is a possible future extension. We wouldn't handle other broker registrations though, since they are a global resource.
This will be the same as v1 manifest. We won't be tracking the manifest post-application, so no.
There are already rolling deploys (& canaries) built in to CF. There may be additional deployment strategies in the future, but that wouldn't be related to the manifest schema.
This isn't defined right now. We'd need to figure out the proper order when implementing. Like in v1, users won't have fine-grained control over resource creation/deploy order. |
Thank you for answering all the questions! regarding: SIs are only updated if plan or parameters change. How will cases where a service broker adds additional “default” parameters to the creation parameters be handled? For example, some brokers return extra parameters after a service instance is created, making the comparison between the parameters in the manifest and the actual parameters always different. Will Cloud Foundry handle scenarios where certain service creation parameters cannot be provided during an update? For instance, what happens if the broker fails in such cases? |
I think I can also answer the last two question:
The Cloud Controller will try to update the service instance if a plan or parameters changed in the manifest definition. If there are no changes for a SI Cloud Controller won't try to do anything. Additionally, there is no way for the Cloud Controller to know that an added parameter to the manifest definition is a default one and also in this case a SI update will be triggered.
The RFC answers this question here: https://github.com/cloudfoundry/community/pull/1019/files#diff-adfdd3cef870d4a5d339bc9f879ad6f7cee5ef16931049dc6bb46973540c6cf9R169-R174. If a resource creation fails the deployment will fail but there won't be an automatic rollback for already created resources. |
@Gerg Going over the discussions it looks to me that most of them could be resolved. Would be great if you could resolve the discussion where we have an agreement and update the RFC if needed. Next TOC meeting on Tuesday we could start the Final Period for Comment if it looks good. |
multi-word keys. Schema version 2 keys should only use underscores, to match | ||
the v3 API convention. | ||
|
||
### Behavior |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question Any thoughts on whether v2 manifests mimic the behaviour of v1 where you may have to restart/restage the app in order for configuration to take effect (e.g. update env vars)? I can see it being annoying to apply a v2 manifest and then have to take an imperative action (restart) to actually see changes applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will probably remain the same. That gives users control over how they want to restart their app ("hard" restart vs restage vs deployment).
We can patch over that in clients, to give a more singular experience (e.g. cf push --manifest /path/to/manifest.yml --strategy=rolling
.
1. Update `POST /v3/spaces/:guid/actions/apply_manifest` to accept v2 manifests | ||
1. Update `POST /v3/spaces/:guid/manifest_diff` to accept v2 manifests | ||
|
||
#### CLI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought If there are multiple manifests applied to the same space and I just want to remove my stuff, should we support something like cf delete-manifest
? (Which would only delete resources by type/name that are defined in the manifest)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can call them "anti-manifests".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same manifest 😄 . Thinking about the parity to the k8s world of k apply / k delete
. Also, since if we're going to support pruning resources then the manifest piece will already have delete logic.
|
||
#### CLI | ||
|
||
1. Update `cf push` and `cf apply-manifest` to accept v2 manifests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought Maybe too detailed for this proposal, but it seems the existing cf apply-manifest
does display a diff but does theres no way to wait for user confirmation to apply changes - it just applies changes. In this manifest v2 world, if its possible some actions are destructive we will definitely need that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That may be considered a breaking change. For example, in a continuous delivery pipeline that starts hanging/failing because an app with a v2 manifest triggers an interactive prompt. We may need to reserve the prompt behavior (or v2 manifest support in general) for a new major version of the CLI.
- Top-level resources are no longer deleted by default - Based on PR feedback, deleting resource by default was too risky - Add section describing opt-in resource deletion (pruning)
We discussed this RFC during the TOC meeting on 17th of December and decided to start the FCP with the goal to accept this RFC during the next TOC meeting which is planed to be on 7th of January. |
- To match new named used in v1 manifests Co-authored-by: Maximilian Moehl <44866320+maxmoehl@users.noreply.github.com>
For easier viewing: https://github.com/Gerg/community/blob/rfc_v2-app-manifests/toc/rfc/rfc-draft-v2-app-manifests.md
cc @tcdowney