Remove env, bindings, etc. not specified in app manifest

[aegershman]

What's the user value of this feature request?

As a user, when I perform cf push with a manifest, I'd like a way for the manifest to be the absolute truth for the application's configuration.

Who is the functionality for?
Humans, CI/CD systems, compliance and auditing folks, anyone wanting configuration as code stored in git, anyone and anything which performs a cf push with a manifest

How often will this functionality be used by the user?
Every cf push or apply-manifest

Who else is affected by the change?
Shouldn't cause a breaking change as long as it's behind a flag like --remove-undeclared or something to that effect.

Is your feature request related to a problem? Please describe.
I feel that since application manifests are declarative, they should be able to describe desired state, not just additive state.

Currently when performing a push or apply-manifest (or cf7 push, v3-zdt-push, etc.), options like env key/values, service bindings, metadata labels/annotations, etc., are additive & won't be removed from the app if they're removed from the manifest:

Apply changes specified in a manifest to the named apps and their underlying processes. These changes are additive and will not modify any unspecified properties or remove any existing environment variables, routes, or services.

This makes it harder to reason about the actual service bindings, labels, etc. that an app is using, since the only way to remove no-longer-desired config is to manually imperatively go remove it with cf unbind-service..., etc.

Describe the solution you'd like

cf push -f manifest.yml --remove-unknown

Basically, if a service binding, env value, metadata value, etc. doesn't exist in the manifest.yml during push, but does exist on the current revision/deployment of the app already deployed, then they should be removed from the app. The manifest should be able to be the absolute source of truth.

Describe alternatives you've considered

Pushing an entirely new instance of the app and using blue/green deployments is an option (e.g., what the autopilot plugin would do) because it creates an entirely new app ID, and thus all new service bindings, labels, env vars, etc... But it also blows away revision history, any metrics history, etc. It entirely defeats the purpose of being able to do revisions and zdt-push and such.

Thoughts?

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/167058413

The labels on this github issue will be updated when the story is started.

[aegershman]

Or better yet, kubectl apply uses --prune to do a symmetric diff, so that would be a good choice for the name of the flag

cf push -f manifest.yml --prune

[abbyachau]

cc @ssisil @selzoc

[aegershman]

Not to nag, but some comment updates:

• Not sure if this is the domain of the CLI or CAPI?

• I do hope this story around service bindings (+ env vars, labels, etc.) are taken into consideration as part of the ongoing discussions regarding v3 APIs && deployments, revisions, rollbacks, etc. I feel that smarter people than I would benefit from considering if metadata or env data or services bindings or routes(?), etc. should be associated with a revision, and, if they're removed/updated during a rolling update, whether they should be restored during rollbacks.

• I can foresee statements such as "a continuous delivery tool (spinnaker) can do this using the APIs, why not use that?" or "just use a bash script?"-- but in my view it's better to evaluate this use-case as part of the core API as a first-class citizen rather than putting the burden on other systems. If the workflow is encapsulated as part of the core API, it makes it easier to reason how other systems (Concourse tasks, Jenkins, raw CLI usage, etc.) could integrate/orchestrate it.

• Similar to how BOSH deployment manifests are treated as the source of truth, it makes sense that the manifest.yml for an application should represent... well, a declarative manifest(.yml 😉) which doesn't require imperative steps (or external systems like Spinnaker) to achieve the expressed state.

Thanks for your time & consideration

[selzoc]

Hi @aegershman

• this is the domain of CAPI as the manifest is now parsed server-side
• we are considering how service bindings will work with revisions, but have no concrete plans as of yet
• unfortunately the manifest implementation as-is is not declarative so that it works in a backwards-compatible manner with older manifests and clients

This is good feedback though, and we're trying to evolve our thinking in these areas

[aegershman]

Thanks for the response @selzoc

I understand the need for backwards compatibility. I definitely don't pretend to know the complexities and nuance of the architecture; perhaps this could be opt-in with a --prune / request param? I guess what I'm saying is I hope backwards compatibility in usage wouldn't be the only reason for manifests being additive-only. But honestly as long as the use-case is aware of & being considered during the v3 discussions, I'm a happy camper.

My fear is these awesome new features like rollingUpdate are implemented & solve many problems, but inadvertently open up a new class of issues which aren't manifested (hah) until after core design is solidified. Then it puts the onus on users to retro-fill the problem with other tools ("why don't you just use Spinnaker? or this wonky bash script?"). Apologies, don't mean to ramble, just sharing.

Anyhow--

Would you like me to move this issue to a different repository? Or leave it here? (Or leave it here and close it?)

Thanks again 👍

[aegershman]

I hope this will still be considered one day. Don't get me wrong, revisions are great for rollingDeployments, and useful for the case of keeping apps up and available to roll back on in the event of a failed rolling update (via cancel deployment).

It's just that pruning envvars, service bindings, routes, etc. are all more common concerns than needing to do revision rollbacks.

Don't mean to belabor this or whine, I'm just posting an update on this. In my team's experience from using v3 apis and cf7-cli in prod, having the manifest be the true source of truth for config reconciliation would be, as they say, "pretty neat"

👍 thanks for folks' time

[Kiemes]

@pivotal-mikegresham could you as CAPI PM please kindly have a look at this issue and let us know what we should do? Can you take this request into our team or should we create a new issue in https://github.com/cloudfoundry/capi-release? Thx
Btw, I could not find Laura Clampett in Github as she seems to be your co-PM according to your Slack channel description.

[Kiemes]

Any updates on this?
We stumbled upon this again as not only env vars are problematic but also the buildpack.
Pushing the same manifest but having different app histories will result in different outcome.
Example:

1. You push a manifest with a buildpack specified, it will use the buildpack
2. Remove the buildpack from the manifest and push again, it will still use the formerly specified buildpack and not choose the buildpack from the predefined list of available buildpacks (cf buildpacks).
   Whereas if you would just push the same manifest without the buildpack right in the beginning (or after deleting the app), it would choose the buildpack from the available buildpacks.

For all people I know this is a real issue as the expectation is that the manifest is a description of the desired state which is not fulfilled. I will gently ping people in the capi slack channel.

[Gerg]

I'm going to transfer this issue to cloud controller, since it is now responsible for applying manifests.

We definitely recognize that it would be much easier to understand if manifests were applied declaratively. We are also in the early stages of designing a v2 schema for manifests, which might be a good opportunity to introduce declarative manifests.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/174980210

The labels on this github issue will be updated when the story is started.

[lbeaufort]

Hi there, thanks for considering this feature request!

We moved to rolling deployments as an alternative to using the deprecated autopilot plugin and the add-only functionality of rolling deployments with manifest files wasn't immediately clear to me.

Adding an option to prune unused env vars, routes, etc. would be helpful for keeping our deployments fully automated. It might also be helpful to note this additive-only functionality somewhere in the rolling deployment documentation - if you agree, I'd be happy to put in an issue or PR for the documentation here.

Thanks again!