search

cloudfoundry-incubator / spiff

declarative BOSH deployment manifest builder
Apache License 2.0
64 stars 44 forks source link

fix for defaulted static_ips based on expressions #70

Closed mandelsoft closed 8 years ago

mandelsoft commented 8 years ago

Yet another problem with static_ips in combination with the ||expression and expressions used for the implicitly used yaml elements.

static_ips as well as auto are not only using their arguments, but also dedicated parts of the yaml document (the static ip ranges for the network section and the job definition). If static_ips is used together with the || operator and an implicitly used value from the network or job definition is using a dynaml expression, the resulting value is always the default value, because the implementation does not distinuish between a final error situation and an intermediate problem because of an unresolved node. This becomes relevant with the new expression evaluation.

The calculation of the static ips must be delayed until the indirectly used elements are completely resolved, instead of delivering an error. Therefore the reference evaluation now uses reference expressions, instead of a direct lookup in the yaml document. This provides the opportunity to handle the unresolved case.

cfdreddbot commented 8 years ago

Hey mandelsoft!

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 commented 8 years ago

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/111897053.

zaksoup commented 8 years ago

@mandelsoft Thanks for making these changes!

I'm going to begin validating this PR against our current set of tests and let you know if I come across any issues. Nothing stood out to me at a quick review of your code changes.

Best, @zaksoup CF Release Integration

robdimsdale commented 8 years ago

This looks great, thanks.

Rob && @dsabeti