cloudfoundry / cli

The official command line client for Cloud Foundry
https://docs.cloudfoundry.org/cf-cli
Apache License 2.0
1.75k stars 926 forks source link

Enhance value substitution in manifests to support merging lists (of services) #1556

Open robachmann opened 5 years ago

robachmann commented 5 years ago

What's the user value of this feature request? App developers can follow the 'DRY' principle in software engineering: Don't repeat yourself. That makes application deployment to Cloud Foundry less error-prone and increases the over-all efficiency working with Cloud Foundry.

Who is the functionality for? Application developers pushing applications to Cloud Foundry using manifests - most likely in an automated CI/CD environment.

How often will this functionality be used by the user? With every push to Cloud Foundry.

Who else is affected by the change? Users of the deprecated manifest inheritance feature who are looking for an alternative.

Is your feature request related to a problem? Please describe. Since some versions of the Cloud Foundry CLI, users using the manifest inheritance feature get a deprecation warning during a push to Cloud Foundry, as announced in the blog in January 2018. While the suggested alternative value substitution covers most of the capabilities, the following feature does not seem to have a succeeding replacement:

In our context, to deploy around 120 micro-services based on Spring, we have some default values that are the same for each and every application. In order to avoid repeating that values, we created a so-called base-manifest with the following content:

domains:
  - domain.company.com
  - apps.internal
health-check-type: http
health-check-http-endpoint: /health
buildpack: java_buildpack
env:
  TZ: Europe/London
  SPRING_RABBITMQ_ADDRESSES: ${vcap.services.rabbitmq.credentials.uri}
  SPRING_ZIPKIN_BASE_URL: ${vcap.services.zipkin.credentials.uri}

services:
  - splunk
  - rabbitmq
  - zipkin

An example of a specific application's manifest looks like this:

inherit: base-manifest.yml
applications:
- name: app-test
  memory: 1G
  instances: 2
  env:
    SPRING_PROFILES_ACTIVE: test
  services:
    - mongodb

As you can see, an application developer just needs to add app-specific properties to the manifest, like memory and instances. If that application needs additional services, the developer adds those to the services list.

This works great and scales well with 100+ micro-services.

Migrating this concept to value substitution, a shared base-manifest would look like this:

domains:
  - domain.company.com
  - apps.internal
health-check-type: http
health-check-http-endpoint: /health
buildpack: java_buildpack
memory: ((memory))
instances: ((instances))
env:
  TZ: Europe/London
  SPRING_RABBITMQ_ADDRESSES: ${vcap.services.rabbitmq.credentials.uri}
  SPRING_ZIPKIN_BASE_URL: ${vcap.services.zipkin.credentials.uri}
  SPRING_PROFILES_ACTIVE: ((profile))
services:
  - splunk
  - rabbitmq
  - zipkin

... and using it would require an app-specific properties.yml which is applied with the --vars-file command:

memory: 1GB
instances: 2
profile: test

Now the problem is the remaining property for services: value substitution does not seem to support merging lists of default values and specific values. I don't see any possibility to use the three mentioned services and add the fourth on a per-app basis.

Describe the solution you'd like Since manifest inheritance does exactly what we expect (and like), the quickest solution would be to reconsider the deprecation of that feature and support it in upcoming releases of Cloud Foundry CLI. If manifest inheritance is really to be removed, an easy translation of the use-case is appreciated and almost necessary to follow the DRY principle. As I'm not that familiar with all capabilities of the YAML markup language, I can't exactly describe how I would expect the new syntax to look like. Easiest would be to provide a list of entries in the app specific properties.yml which are then merged with a place-holder in the base-manifest:

memory: 1GB
instances: 2
profile: test
services: 
  - mongodb

If the below described usage of multiple --vars-file would merge lists instead of replacing the values, that could also be an acceptable solution.

Describe alternatives you've considered I already tried using multiple --vars-files where I would expect values of duplicate keys of an arrays would get merged:

cf push -f manifest.yml --vars-file base-services.yml --vars-file specific-services.yml with

base-services.yml

services:
  - splunk
  - rabbitmq
  - zipkin
specific-services.yml

services:
  - mongodb

Unfortunately, the latter assignment of the variable always win.

Additional context Link to Slack conversation: https://cloudfoundry.slack.com/archives/C032824SM/p1548785899078700 Tested with cf version 6.42.0+0cba12168.2019-01-10

Thank you very much for considering this feature-request!

cf-gitbot commented 5 years ago

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

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

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

abbyachau commented 5 years ago

Manifests are now handled by the server (cc @ssisil @tcdowney CC API PM and Anchor).

tcdowney commented 5 years ago

@abbyachau

Mostly just thinking a bit out loud here...

This exact workflow isn't something we'd easily be able to support server-side. Server-side app manifests work well for the original use case of understanding a complete app manifest and taking actions based on it all at once in lieu of a client having to convert the manifest into many individual API requests.

They don't work so well for these cases where folks want to share configuration across apps (via inheritance or interpolation) or compose configuration together. The server-side manifest endpoints want the final manifest -- and if they ever become truly declarative this is important.

Since it's not quite declarative at this point (additive for many things, including services), one thing we could potentially do is have the shared manifest content applied individually from the app-specific manifest content in separate calls. Maybe there is a way to make that a decent workflow, but I'm not sure. I think adding concerns like YAML merging (and overrides in general) makes for a messy and overly-specific API.

In general though, I really see this as being similar to how the bosh cli works with the cf-deployment manifest. It supports variable interpolation client-side along with the ability to override and compose the manifest using ops-files. I wonder if something like ops-files is what we're missing for this use case?

Tagging @tjvman @Gerg @mkenyon for your thoughts as well.

abbyachau commented 5 years ago

Hi @robachmann thanks for creating this issue, apologies for the late response. @tcdowney thanks for your input. Just wanted to let you know we are tracking this issue but cannot provide a update. We'll revert as soon as we can. Thanks.

abbyachau commented 5 years ago

@robachmann many apologies for the delay. @ewrenn8 @selzoc @ssisil @acrmp trying to help with this use case. Off the top of my head, given server side manifests, do you think it would be feasible to have --vars-file-override and --vars-file-merge instead of --vars-file? --vars-file-override behaves like --vars-file currently does, overrides the manifest property whereas --vars-file-merge would merge the properties?

robachmann commented 5 years ago

Thanks for the feedback, @abbyachau. Good to see you have it on your radar.