cloudfoundry-attic / cf-deployment-transition

This repository is deprecated - no longer accepting PR's or Issues
Apache License 2.0
9 stars 5 forks source link

error generating manifest for unresolved nodes with extract-vars-store-from-manifests.sh script #7

Closed phong2tran closed 6 years ago

phong2tran commented 6 years ago

We're in process of testing cf-deployment migration from cf-release and ran into this error when using extract-vars-store-from-manifests.sh script:

$ ./extract-vars-store-from-manifests.sh --ca-keys ../ca-keys.yml --cf-manifest ../manifests/cf-287.yml --diego-manifest ../manifests/cf-diego-1.35.0.yml /tmp/tmp.4hzDL1MW6Y 2018/02/27 21:48:51 error generating manifest: unresolved nodes: (( merge )) in /tmp/tmp.z3Nb90iMIU jobs.[0].properties (jobs.database_z1.properties) 2018/02/27 21:48:51 error generating manifest: unresolved nodes: (( merge )) in /tmp/tmp.z3Nb90iMIU jobs.[0].properties (jobs.database_z1.properties)

If we added the nil option in the template (https://github.com/cloudfoundry/cf-deployment-transition/blob/master/templates/vars-pre-processing-template.yml#L179-L181) as below,

jobs:

then the error went away.

What exact properties are expected for database_z1 job if required? If not, is adding "|| nil" option for ((merge)) expression expected?

cf-gitbot commented 6 years ago

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

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

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

dsabeti commented 6 years ago

Hi @phong2tran, our environments typically have a whole lot of properties in the database_z1 instance group. Many of them are not actually set (they merely have empty values), but they definitely tend to have loggregator.ca_cert and metron_agent.zone. Does your database_z1 instance group not have any properties configured for them?

I'm going to rope in @ematpl from the Diego team to see if he has any guidance about which properties must be configured. If it turns out that these properties aren't actually required, we can add a || nil to that part of the template, I suppose.

dsabeti commented 6 years ago

Ok @phong2tran. After digging in, it looks like we mostly need to include that for operators who have deployed locket as part of their Diego deployment. We should probably remove the requirement that the properties be set.

dsabeti commented 6 years ago

Ok @phong2tran, try the latest commit: https://github.com/cloudfoundry/cf-deployment-transition/commit/a38b2b112d584f60d38f02e008eed1340db69676

phong2tran commented 6 years ago

Great, thanks for the fix @dsabeti!