Manifest generation should merge extral properties from collocated jobs

[Kaixiang]

When we are trying to add a collocated job into diego.

we found the properties of the collocated job will be ignored even if we put it into property-overrides.yml.
we need to add a 'merge' into the template so it could take the property for collocated job.

[cfdreddbot]

Hey Kaixiang!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

[cfdreddbot]

Hey Kaixiang!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/103241362.

[Amit-PivotalLabs]

Property overrides are meant to override job spec defaults for only a very specific list of diego properties, not permissively merge in any provided properties. Those sorts of merges allow people to start to use the stubs and manifest generation tools in unmaintainable ways, and also commits the Diego team to maintaining a contract they have no control over.

In general, tools like the scripts/generate-deployment-manifest are meant as a convenience, and are not guaranteed to support 100% of all possible BOSH deployments of diego. spiff templates are inherently incapable of supporting arbitrary colocation strategies, arbitrary numbers of AZs/clusters, arbitrarily large instance scaling for things that need static IPs, etc.

Would the properties you're trying to add be easy to add after the fact, e.g.

./scripts/generate-deployment-manifest ...stubs... > temp_manifest.yml
ruby -r yaml -e \
  'puts YAML.dump(YAML.load_file("temp_manifest.yml").tap { |m| m["properties"].merge! YAML.load_file("additional_job_properties.yml") })' \
  > final_manifest.yml

[emalm]

Hi, @Kaixiang,

As @Amit-PivotalLabs mentioned, the spiff-based manifest-generation in diego-release doesn't allow arbitrary changes to the manifest. In particular, it takes a fixed sequence of stub files as its arguments, instead of the arbitrary sequence of stubs that the manifest-generation script in cf-release does, and extracts a specific set of values from each stub, instead of simply collapsing them via a spiff merge. So even if the <<: (( merge )) operator is present in the properties section, there's no reasonable way to get additional properties to that merge site via the script. It might be possible to have a post-processing stage that takes the Diego manifest and merges in additional properties, though.

Thanks,
Eric