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

Make detection of currently installed Chef version more robust #61

Merged

Conversation

4 participants
@ampedandwired
Copy link
Contributor

commented Jan 14, 2014

In some situations the result of running 'chef-solo -v' using
@machine.communicate.sudo includes more than just the command output
(eg: command prompts, session setup commands).

This change finds the chef version in the resultant output using a
regular expression to handle situations like this.

@tmatilai

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2014

👍 as I just recently debugged this for someone but then forgot to make a PR. =)
The triggering thing was config.ssh.pty = true. I'm surprised if that doesn't break a lot more things too all over Vagrant and other plugins.

But I have a feeling that you should not break from the block as it seemed to leave all the unconsumed lines for the next Communicator caller.

@schisamo

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2014

@ampedandwired Thx for this fix! Can you please rebase on the latest master and remove the break as @tmatilai suggested?

Make detection of currently installed Chef version more robust
In some situations the result of running 'chef-solo -v' using
@machine.communicate.sudo includes more than just the command output
(eg: command prompts, session setup commands).

This change finds the chef version in the resultant output using a
regular expression to handle situations like this.
@ampedandwired

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2014

I've rebased and removed the break statement. We are also using config.ssh.pty so this does indeed seem to be the cause.

@ampedandwired

This comment has been minimized.

Copy link
Contributor Author

commented Feb 3, 2014

Any updates on this pull request?

schisamo added a commit that referenced this pull request Feb 4, 2014

Merge pull request #61 from ampedandwired/chef_install_idempotency
Make detection of currently installed Chef version more robust

@schisamo schisamo merged commit 332ff23 into chef-boneyard:master Feb 4, 2014

1 check passed

default The Travis CI build passed
Details
@schisamo

This comment has been minimized.

Copy link
Contributor

commented Feb 4, 2014

@ampedandwired sorry for the delay! working on one more fix before I release a new version of the gem.

@tas50 tas50 added the enhancement label Aug 31, 2016

@tas50 tas50 added Type: Enhancement and removed enhancement labels Jul 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.