-
Notifications
You must be signed in to change notification settings - Fork 188
Update to include RHEL 7 logic #49
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
I don't think this solves the underlying issue. The 'functions' file also exists in RHEL7 so this change doesn't fix the problem. I think I figured out what is the problem here. The custom commands 'start-no-update' and 'start-with-update' don't seem to work well with systemctl. The rpm installation uses 'start-no-update' command. This doesn't activate the agent's service unit in systemctl. $systemctl list-units --type service --all | grep codedeploy I can fix the rpm to call 'start' instead of 'start-no-update'. The bigger issue here is whether to have a feature that is supported only on a subset of officially supported OSes. This makes it harder to maintain the code. We would ideally want to support running the agent as a different user in all of the supported linux OSes. This means we might have to use a different way to run as arbitrary user in Ubuntu systems say something like 'start-stop-daemon'. It would be great if you could make this change generic and would work on all supported OSes. I'll also bring this to team's attention and see what they think. Thanks, |
@Suryanarayanan my apologies, you are correct. I quickly threw this together using docker and didn't see /etc/init.d/functions in my docker image and made an incorrect assumption. We are using RHEL 6 which is why I focused there. I will look into why this is not starting properly on RHEL 7 based on the information you have provided and try and devise a solution which includes Ubuntu as well. Are there other OS's which need to be supported? It would be helpful to see your build scripts which produce your RPMs. It would assist in trying to come up with a solutions which are not OS specific. |
@ccloes-intuit currently we support AmazonLinux, RHEL and Ubuntu on the linux side of things. We need to fix two things here:
Thanks again for your effort and contribution, |
This reverts commit a2c77ce.
@Suryanarayanan ... closing this PR to open a new one. |
@Suryanarayanan Please review for fix on RHEL 7