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.

Sign up for GitHub

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

Jump to bottom

Bootstrap: Fix for performing sudo operations once sudo password ente… #11654

Merged
merged 1 commit into from
Aug 2, 2021
Merged

Bootstrap: Fix for performing sudo operations once sudo password ente… #11654

merged 1 commit into from
Aug 2, 2021

Conversation

msys-sgarg
Copy link
Contributor

@msys-sgarg msys-sgarg commented Jun 3, 2021 •
edited

…red by user

Signed-off-by: smriti sgarg@msystechnologies.com

Description

knife bootstrap --use-sudo-password command if is executed for systems where sudo password is used for every operation. A default train error is shown to user as of now.

With current change we are handling Train::UserError in knife code and display a user friendly error message with instructions which will help user to perform knife bootstrap operation successfully.

Related Issue

#11359

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

@msys-sgarg msys-sgarg requested review from a team as code owners June 3, 2021 11:47
@msys-sgarg msys-sgarg force-pushed the smriti/11359_knife_bootstrap_sudo_password_broken branch from becaf3d to a33c95e Compare June 3, 2021 12:39
marcparadise
marcparadise reviewed Jun 3, 2021
View reviewed changes
knife/lib/chef/knife/bootstrap.rb Outdated
@@ -712,6 +713,9 @@ def do_connect(conn_options)
conn_options[:sudo_password] = sudo_password

retry
elsif e.message =~ /Sudo failed: \S+/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to handle this message in train
rather than doing ssh literal text parsing both here and hidden in the train lib - we could supply a reason of :sudo_access_failed, and handle that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will do that same. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcparadise I have raised this PR on train repo - inspec/train#690 with required changes. Please take a look

marcparadise
marcparadise requested changes Jun 3, 2021
View reviewed changes
Copy link
Member

@marcparadise marcparadise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment nline

@msys-sgarg msys-sgarg force-pushed the smriti/11359_knife_bootstrap_sudo_password_broken branch from a33c95e to 4ab6433 Compare June 7, 2021 12:29
@msys-sgarg msys-sgarg closed this Jun 7, 2021
@msys-sgarg msys-sgarg reopened this Jun 8, 2021
@msys-sgarg
Copy link
Contributor Author

I am keeping this PR open.. since once chef/train inspec/train#690 changes are merged. We will need to handle e.reason here in bootstrap.rb and output much more to the point error message

@tas50
Copy link
Contributor

tas50 commented Jul 26, 2021

@msys-sgarg have the train fixes necessary for this been merged?

@tas50 tas50 added the Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. label Jul 26, 2021
@msys-sgarg
Copy link
Contributor Author

@tas50 train PR is still pending for review. It has been updated as per review inspec/train#690 quite some time back

@tas50
Copy link
Contributor

tas50 commented Jul 27, 2021

The train changes are merged and pulled into chef/chef.

@msys-sgarg msys-sgarg force-pushed the smriti/11359_knife_bootstrap_sudo_password_broken branch from 4ab6433 to 5aff158 Compare July 28, 2021 16:21
@tas50 tas50 merged commit 170f6fb into chef:master Aug 2, 2021
@msys-sgarg msys-sgarg mentioned this pull request Aug 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lamont-granquist lamont-granquist lamont-granquist approved these changes

@marcparadise marcparadise marcparadise approved these changes

Assignees
No one assigned
Labels
Status: Waiting on Contributor A pull request that has unresolved requested actions from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@msys-sgarg @tas50 @lamont-granquist @marcparadise