-
Notifications
You must be signed in to change notification settings - Fork 188
Add support for running hooks with sudo #42
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
Conversation
FYI, not ready to merge, once I receive feedback on the approach I will clean up the code and fix / add the appropriate tests. cc @amartyag / @Suryanarayanan |
ping @amartyag @Suryanarayanan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't here be:
if(script.runas.nil? && !script.sudo.nil?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yubangxi that looks correct, I will verify and update.
Thanks for the PR. |
@yubangxi thanks for reviewing. If you are supportive of the approach I will clean it up and ensure all tests are passing. |
@brettweavnet-intuit I just did the review, the code change overall looks good to me. I will need to talk with the team about how we want to support this feature. I will post a update here soon. Thanks! |
@yubangxi Thanks, please let me know if you will support this feature. I will update the tests in preparation. |
@yubangxi I've updated the tests and fixed the bug. Please let me know once you have discussed the changes with the team. |
Add support for running hooks with sudo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses systemctl to run start, stop, restart commands. systemctl doesn't stop the agent for the first time when you issue the stop command. This seems to be happening only on RHEL boxes. Could you please address this issue before we could release the changes?
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is already merged, I will add a new PR with logic to check for RHEL 7. Will that suffice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it should be fine to do so. Thanks a lot!
Code Deploy,
This PR is meant to open a conversation around adding support for running hooks with sudo. We have a requirement to run commands as sudo to provide additional logs to our operation center. There is also a secondary to desire to run the code deploy agent as a non-root user (if possible).
You can add now add sudo to hooks to have them executed with sudo:
This has also been tested to allow for running the code deploy agent as a non root user (#5) on RHEL 7 via the following process:
Please provide feedback / guidance on the approach and appropriateness of this change.