App env vars and service binding changes aren't reverted for failed rolling deployment

[beyhan]

Issue

A rolling update which is canceled because of an application issue (e.g. failed start) will revert to the latest known droplet version but any changes to environment variables or service bindings will not be reverted.

Steps to Reproduce

1. Deploy a slightly modified version of the spring-music app which introduces a version and a new env var called TEST by following the instructions available on the readme.
2. Verify that https://<apphost>/appinfo shows {"profiles":["http2"],"services":[],"version":"1.0.0"} and https://<apphost>/envVarTest shows false
3. Create a service instance from any type with the name test
4. Deploy with cf push --strategy rolling another version of the spring-music app which can't start and introduces changes to the app version, the value of the TEST env var and a binding to the test service instance.
5. The cf push --strategy rolling will fail and with cf droplets <my-app> you can check that the latest droplet isn't the current one, but cf env <my-app> will show the updated version of the env var and that the app is bound to the test service.
6. Restart the app with cf restart <my-app> Verify that https://<apphost>/appinfo shows {"profiles":["http2"],"services":["test"],"version":"1.0.0"} and https://<apphost>/envVarTest shows true.

Expected result

The env var and service binding changes are not applied for failed rolling deployments.

Current result

Currently the env var and service binding changes are applied to the application.

Possible Fix

One option could be to use the Cloud Foundry API app revisions to revert to the last known good version in such a situation.

Additional Context

Create an issue on the docs side to document the current behaviour.

[Gerg]

To me, this is expected behavior. cf push is not atomic, regardless of whether you use --strategy=rolling or not. Failed pushes can result in partially-applied configuration.

If you want to roll back configuration for an application, that's what revisions are supposed to do: https://docs.cloudfoundry.org/devguide/revisions.html

[beyhan]

Thank you @Gerg for the feedack. I agree that cf push is fine because in this scenario you will have an application in state crashed and manual interaction is required anyway. The promise behind cf push ... --strategy=rolling is to have zero downtime updates and people are building automations around it. That is why avoiding the need of manual interaction is important. The use of the revisions APIs for failed deployments could be automated but it looks to me as a reasonable improvement for the rolling updates. This is the feedback I get from CF users I talk to. Some of them had production impact because they didn't know about the limits of Cancel a deployment. The use case of this issue wasn't even documented there and I proposed a fix so that we documented it at least.

[Rand0mF]

I agree with beyhan. For a normal cf push that's fine, as it will not have an old version of the app running on failure. A rolling push should be atomic.

Giving an example:
Apps can be inspected with cf app <app> and seeing an app which is perfectly stable running, however, since some time ago, another user (or automation) failed to apply a new version, it is now not possible to restart the existing app anymore, reason unknown for the current user (or automation). The old version of the app is not designed to work with the new env values from the new app version.

The configuration (env) should be bound to the version it is deployed with. It is counter-intuitive for an end-user to have a rolling deployment, which spins up a new version of the app but then applies the configuration to the old version of the app also.

Rolling updates should be atomic, new configuration should only be applied for a new version, the old version should continue to run with the old configuration.