chef-boneyard / delivery-truck

DEPRECATED: Delivery build cb for pipelines
Apache License 2.0
36 stars 52 forks source link

Fix a situation where deploys would fail if the workspace is in a non-standard location #32

Open irvingpop opened 7 years ago

irvingpop commented 7 years ago

The DeliverySugar::ChefServer class expects a node object to exist in order for it to look up the delivery_knife_rb ( here ) , but that doesn't exist the way it's being called from DeliveryTruck::Helpers::Deploy.

This PR calls the same delivery_knife_rb method from DeliverySugar::DSL but in a way that it can succeed by calling it from the DSL method and then passing it to the non-DSL method.

As a point of discussion:

  1. The rescue in DeliverySugar::DSL.delivery_workspace ( link ) seems like a bad idea, because it's masking a problem where the node object doesn't exist, causing the change method to blow up. That should be revisited, I suspect that this is a problem for any customer that uses a non-standard workspace location.

  2. It's not clear to me why the DeliveryTruck::Helpers::DSL.delivery_chef_server_search method needs to alias DeliveryTruck::Helpers::Deploy.delivery_chef_server_search ( link ) - it would be helpful if someone with some history could explain that.

jerryaldrichiii commented 7 years ago

Yup, there is a rescue here that will always trigger in the Deploy phase. This causes errors in Deploy for instances where the default workspace path is not used (e.g Windows Build Nodes).

I agree that the issue is caused by calling DeliverySugar::ChefServer.new.with_server_config from within DeliveryTruck::Helpers::Deploy without passing the node object thus causing change to fail to initialize (and triggering the rescue)

Here is the code path that leads to an error (based on my understanding):