[preview-install] Add user-friendly output #10695
Conversation
Pothulapati
force-pushed
the
tar/preview-output
branch
3 times, most recently
from
d122a6b to
c0aa15d
Compare
|
started the job as gitpod-build-tar-preview-output.4 because the annotations in the pull request description changed |
github-actions
bot
added
the
team: delivery
Pothulapati
force-pushed
the
tar/preview-output
branch
from
c0aa15d to
b7594f4
Compare
|
started the job as gitpod-build-tar-preview-output.6 because the annotations in the pull request description changed |
Pothulapati
force-pushed
the
tar/preview-output
branch
from
b7594f4 to
a6b322e
Compare
| for f in /var/lib/rancher/k3s/server/manifests/gitpod/*PersistentVolumeClaim*.yaml; do yq e -i '.spec.storageClassName="local-path"' "$f"; done | ||
| # Set `volumeClassTemplate` so that each replica creates its own PVC | ||
| yq eval-all -i ". as \$item ireduce ({}; . *+ \$item)" /var/lib/rancher/k3s/server/manifests/gitpod/*_StatefulSet_messagebus.yaml /app/manifests/messagebus.yaml |
There was a problem hiding this comment.
Hmm, Guessing we might not actually need this, will try and revert back!
| @@ -5,6 +5,10 @@ | |||
|
|
|||
| set -ex | |||
|
|
|||
| if [ "$1" != "logging" ]; then | |||
| $0 logging 2>&1 | /prettylog | |||
There was a problem hiding this comment.
If I'm reading this correctly, we're re-launching this script in order to redirect all logs to /prettylog, correct?
If we are relaunching the script with the intent described above, we can use exec to redirect stdout/stderr, with something like this:
exec > >(/prettylog) 2>&1
This way we don't need to relaunch the script; we can launch /prettylog with process substitution and redirect stderr/stdout with exec.
This is another approach to the problem, but isn't necessarily better or worse. If you like the above approach we can adopt it; if what you have is more readable then let's keep it as-is. 😄
There was a problem hiding this comment.
Following up on this, we're also relying on the set -e to exit this script upon error, and we're relying on /entrypoint.sh and prettylog to exit with a non-zero exit value. When the user hits ctrl-c to kill the container, the forked processes exit non-zero then the behavior of set -e in the parent process kicks in and terminates the script.
If the set -e is removed for whatever reason, then when the forked processes exit we'll proceed to run this script again, which isn't intended. I think this code will be a little bit more correct to have an exit after $0 logging; that way changes in exit codes or other behaviors won't accidentally run the script twice.
There was a problem hiding this comment.
This way we don't need to relaunch the script; we can launch /prettylog with process substitution and redirect stderr/stdout with exec.
I tried doing this but I get an error stating that In POSIX sh, process substition is undefined which makes sense as its a sh script, and not bash. This is important to work with the tini in the leeway.Dockerfile. (I tried replacing it with bash but couldn't get far)
There was a problem hiding this comment.
Ach, Bourne shell strikes again! Thanks for investigating this!
Part of #9075 This PR adds user friendly output to the preview install docker container by adding a new program prettylog that looks at the output of the entry-point.sh and just adds status updates and spinners. This feels easier instead of adding the same status updates in bash. The status updates are also kept as simple as possible and any additional instructions (URL, and certs) will be added to the documentation instead of here. Feel free to post here if you think it would be better to have them here in the script. Signed-off-by: Tarun Pothulapati <[email protected]>
Pothulapati
force-pushed
the
tar/preview-output
branch
from
a6b322e to
ed10d38
Compare
Description
This PR adds user friendly output to the
previewinstalldocker container by adding a new program
prettylogthatlooks at the output of the
entry-point.shand just adds statusupdates and spinners.
This feels easier instead of adding the same status updates in
bash.The status updates are also kept as simple as possible and
any additional instructions (URL, and certs) will be added
to the documentation instead of here. Feel free to post here
if you think it would be better to have them here in the
script.
Signed-off-by: Tarun Pothulapati [email protected]
Related Issue(s)
Part of #9075
How to test
Run
docker run --privileged --name gitpod --rm -it -v /tmp/gitpod:/var/gitpod eu.gcr.io/gitpod-core-dev/build/preview-install:tar-preview-output.5Release Notes
Documentation