feat: Unify wait-ready with check.sh; implement mongo wait #1199
Conversation
- Move MySQL readiness checks from wait-ready.sh to existing check.sh, and call check.sh for any service we want to wait for - Add Mongo readiness check to check.sh, and use it from provisioning - Give cms a separate check from lms - Limit docker log output to 30 lines when check fails
|
Hmm, provisioning tests are failing at the wait-ready step... :-/ |
Apparently if you just use `$cmd`, it gets tokenized by spaces rather than
having quotes and backslashes and such interpreted as you might expect in
the shell. Since the mysql check commands are the first ones that have
spaces in the command, I need to change this to `bash -c "$cmd"`.
Apparently I had only tested the script with lms and such.
(The correct way to build up a command and run it is instead with arrays,
such as `"${cmd[@]}"`.)
|
Figured out what was wrong. See commit message for details. |
|
Executing an array would be the "clean" way of doing it, and it's what I would normally do. It has the advantage that it can only represent an executable (the first array element) and its arguments (the rest of the array), and doesn't support redirection, pipelines, etc. -- the reduced functionality means it can be easier to reason about in some cases. But... passing an array to a function, which is what we would need to do here, is more complicated and annoying, so just passing a precomposed string to a bash subprocess is the more expedient option. I think it will be clear to a reader what's going on (possibly more clear than before, even) so I don't think it needs a comment about alternative options. |
|
Credentials dev.check failed with |
Ok. I'm not sure if you also plan to drop it from the squashed commit message, and if it was just for the review, but it feels disconnected as-written. So, if you don't think it makes sense in a code comment, it probably doesn't make sense in the commit comment either. |
|
Yeah, I'm not planning on including it in the squash commit. Still need to figure out what's going with credentials. Seems like it should be unrelated, but it's failing in the check phase. |
|
OK, it passed that time? So I suspect that this is just an existing race condition with I'll fix that race condition in a separate PR. |
- Move MySQL readiness checks from wait-ready.sh to existing check.sh, and call check.sh for any service we want to wait for - Add Mongo readiness check to check.sh, and use it from provisioning - Give cms a separate check from lms - Change how check commands are executed; now that some of the commands have a space in them, we have to ensure that quoted or escaped spaces are interpreted properly. Using a bash -c invocation fixes this. - Limit docker log output to 30 lines when check fails
I've completed each of the following or determined they are not applicable: