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

Fixed solo clean command and made log level configurable #522

Merged
merged 4 commits into from Nov 9, 2017

Conversation

Projects
None yet
3 participants
@hoffi
Copy link
Contributor

hoffi commented Nov 8, 2017

The solo clean command did not work for me. It always generated this command:
sudo -p \'knife sudo password: \' -p \'knife sudo password: \' rm -rf

This is because KnifeSolo::SshCommand is calling #process_sudo which is replacing "sudo" with the sudo_command. But in this case sudo_command was already present in the command.
So i removed this from the Clean-Command and just used "sudo" like in the Cook-Command.

Also the log level for my Cook runs defaulted to debug and generated a lot of output. I added an option to change it and defaulted it to the Chef-Log-Level.

I also made enable_reporting configurable through the solo.rb file.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 8, 2017

Coverage Status

Coverage increased (+0.02%) to 89.166% when pulling 4a7b83d on Nix-wie-weg:correct-log-level into 98a4d7f on matschaffer:master.

@matschaffer

This comment has been minimized.

Copy link
Owner

matschaffer commented Nov 8, 2017

@hoffi thanks! Looks like this is causing a test failure as well.

  1) Failure:
SoloCleanTest#test_removes_with_custom_sudo [/home/travis/build/matschaffer/knife-solo/lib/chef/knife/solo_clean.rb:25]:
unexpected invocation: #<Chef::Knife::SoloClean:0x2306860>.run_command("sudo rm -rf ~/chef-solo")
unsatisfied expectations:
- expected exactly once, not yet invoked: #<Chef::Knife::SoloClean:0x2306860>.run_command("/usr/some/sudo rm -rf ~/chef-solo")
satisfied expectations:
- allowed any number of times, not yet invoked: #<Chef::Knife::UI:0x23055a0>.warn(any_parameters)
- allowed any number of times, invoked once: #<Chef::Knife::UI:0x23055a0>.msg(any_parameters)

Likely just a bad expectation but let's get it fixed up before merging.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 8, 2017

Coverage Status

Coverage increased (+1.004%) to 90.153% when pulling 50de524 on Nix-wie-weg:correct-log-level into 98a4d7f on matschaffer:master.

@hoffi

This comment has been minimized.

Copy link
Contributor Author

hoffi commented Nov 8, 2017

I fixed it and moved the tests for custom sudo commands to SshCommandTest

@matschaffer

This comment has been minimized.

Copy link
Owner

matschaffer commented Nov 9, 2017

Fantastic. Thanks!

@matschaffer matschaffer merged commit 02adb50 into matschaffer:master Nov 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hoffi

This comment has been minimized.

Copy link
Contributor Author

hoffi commented Nov 9, 2017

Do you plan a new release or pre-release of the gem including this fix anytime soon?

@matschaffer

This comment has been minimized.

Copy link
Owner

matschaffer commented Nov 9, 2017

@hoffi

This comment has been minimized.

Copy link
Contributor Author

hoffi commented Nov 9, 2017

A pre-release would be perfect. 👍

@matschaffer

This comment has been minimized.

Copy link
Owner

matschaffer commented Nov 10, 2017

Cool. Just pushed 0.7.0.pre2 - unit tests pass but I don't have anything convenient to cross check so let me know if you run into any trouble with it.

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.