Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

irvingpop
Copy link
Contributor

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
Copy link
Contributor

jerryaldrichiii commented Jan 4, 2017

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):

Irving Popovetsky added 2 commits March 6, 2017 09:11
… look up the delivery_knife_rb, but that doesn't exist the way it's being called here. Use the existing delivery_knife_rb DSL method to fix that

Signed-off-by: Irving Popovetsky <[email protected]>
Signed-off-by: Irving Popovetsky <[email protected]>
@irvingpop irvingpop force-pushed the irving/fix_deploy_helper_search branch from 47b7ad8 to 019157e Compare March 6, 2017 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants