Closed dinhxuanvu closed 8 years ago
dinhxuanvu
commented
8 years ago @tiwillia @jwhonce [test] and ready for review. This is the least invasive fix that I can think of at the moment to avoid regression issues or unexpected consequences down the road. Thanks!
dinhxuanvu
commented
8 years ago re-[test] @jwhonce As we discussed last Friday, I have changed the code to explicitly stop the cartridge instead of restarting it.
dinhxuanvu
commented
8 years ago @jwhonce A slight change of code :), as Tim suggested to skip the start cartridge entirely which I thought so too but I didn't do so as I wasn't sure if it has any consequences for that. Turns out after looking at the code again, the start gear in post-receive does call start cartridge as well. So, it doesn't miss anything if we skip the second start cartridge if the cartridge is already started according to the state value.
dinhxuanvu
commented
8 years ago re-[test]
dinhxuanvu
commented
8 years ago re-[test]
dinhxuanvu
commented
8 years ago re-[test]
dinhxuanvu
commented
8 years ago @jwhonce Would like to have your review on this PR before merging :). I know you are a not big fan of restart approach :D but so far this is only solution that is not causing issues with Jenkins test + addon cartridges. I have tried to stop+start and skipping start entirely and both causes failures on Jenkins and on my manual testing as well, especially when adding secondary cartridge (such as mysql) to gear. The restart approach, on the other hand, is working well for Jenkins + my manual test as it uses restart method in control script which is to me at least is safer as it's written for that purposes.
jwhonce
commented
8 years ago @dinhxuanvu If we're going with restart then I believe you should add something to the oo_user_guide.adoc in the action_hooks section about this side case. At least then we can point to the documentation. Thanks
openshift-bot
commented
8 years ago Evaluated for online test up to ad66e2cba684c832cead4a0ae26bc3864f1935b3
dinhxuanvu
commented
8 years ago @tiwillia Ready for review/merge. Thanks :)
openshift-bot
commented
8 years ago Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9182/)
tiwillia
commented
8 years ago LGTM [merge]
openshift-bot
commented
8 years ago Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6701/) (Image: devenv_5763)
openshift-bot
commented
8 years ago Evaluated for online merge up to ad66e2cba684c832cead4a0ae26bc3864f1935b3
When quickstart is deployed with rhc --from-code, there is a start process (from control script) called during post_receive step which starts the gear and apache processes. However, in the next step, the cartridge is started as well. As a result, there are two consecutive start processes leading to the second one fails due to the fact apache (httpd) process is already started and currently in use.
This commit adds a check of current state of the gear before starting the cartridge. If the gear is already started, the restart cartridge process is triggered instead. Otherwise, the start process is striggered as expected. This fix only change the behavior initial build of cartridge during git deployment and it's operating within the local gear. Therefore, it doesn't impact any other gears that are running in the same node.
Bug 1296943 Link https://bugzilla.redhat.com/show_bug.cgi?id=1296943
Signed-off-by: Vu Dinh vdinh@redhat.com