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

- Set hook to run before provisioner so that provisioners that need to c... #20

Closed
wants to merge 1 commit into from

Conversation

b2jrock
Copy link
Contributor

@b2jrock b2jrock commented Jun 21, 2013

...ommunicate to other hosts in the configuration can run.

  • Don't prevent hook from running when guest is already up. The operation should be idempotent. This was also interfering with setting hostname before the provisioner.

Use case for this is:

  1. Bringing up a vagrant puppet master
  2. Bring up a machine that's communicating with the puppet master by hostname during the provisioning stage. If the hostname isn't set, the action will hang or fail.

…o communicate to other hosts in the configuration can run.

- Don't prevent hook from running when guest is already up. The operation should be idempotent. This was also interfering with setting hostname before the provisioner.
@b2jrock
Copy link
Contributor Author

b2jrock commented Jul 11, 2013

Hello... any thoughts on this?

@smdahlen
Copy link
Contributor

I should get a chance to look at this and other pull requests early next week. I've been on vacation and am now catching up on work.

@b2jrock
Copy link
Contributor Author

b2jrock commented Jul 11, 2013

Understandable. Thank you!
On Jul 11, 2013 11:18 AM, "Shawn Dahlen" [email protected] wrote:

I should get a chance to look at this and other pull requests early next
week. I've been on vacation and am now catching up on work.


Reply to this email directly or view it on GitHubhttps://github.com//pull/20#issuecomment-20831228
.

@b2jrock
Copy link
Contributor Author

b2jrock commented Jul 29, 2013

Anything more I can provide to get this in? I'd really like to have my team using the 'real' version, I think this is the last hang-up.

@smdahlen
Copy link
Contributor

@b2jrock Sorry, been pulled in other directions lately. I looked at the PR briefly but I'm not particularly comfortable with updating the hosts file every time the up command is called. Many vagrant plugins check state before executing an action and I wanted to be consistent (while reducing excess cycles).

With that said, I want to support your use case and I have not had time to think about it. Could you clarify the use case a bit further and perhaps look at an alternative approach that ensured that the hosts file was only updated when a new machine is brought online? I'll look at this myself tomorrow and will consider the approach you provided. We should be able to close tomorrow afternoon.

@b2jrock
Copy link
Contributor Author

b2jrock commented Jul 30, 2013

I agree that it's a bit heavy-handed. I also found that it doesn't play
nicely with multiple Vagrantfiles on the same machine, something I think is
a less common use case. I intended to refactor for that issue later, opting
to go for functional first. However, it seems the 2 issues are related, and
could probably be knocked out by the same changes.

On Mon, Jul 29, 2013 at 2:28 PM, Shawn Dahlen [email protected]:

@b2jrock https://github.com/b2jrock Sorry, been pulled in other
directions lately. I looked at the PR briefly but I'm not particularly
comfortable with updating the hosts file every time the up command is
called. Many vagrant plugins check state before executing an action and I
wanted to be consistent (while reducing excess cycles).

With that said, I want to support your use case and I have had time to
think about it. Could you clarify the use case a bit further and perhaps
look at an alternative approach that ensured that the hosts file was only
updated when a new machine is brought online? I'll look at this myself
tomorrow and will consider the approach you provided. We should be able to
close tomorrow afternoon.


Reply to this email directly or view it on GitHubhttps://github.com//pull/20#issuecomment-21753078
.

@smdahlen
Copy link
Contributor

@b2jrock It doesn't look like I will have time in the near future to look at this. If you can find a solution that doesn't require the update of the hosts file every time a command is run, I'd be happy to merge. I'm going to close for now.

@smdahlen smdahlen closed this Aug 15, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants