Verify-Installation Command

[nahara7]

Addresses: #116

Intent

To simplify use of verify-installation command inside the container.

Approach

Verify Installation command was tricky to use in docker. This runs the command at startup and writes its output to a path referenced by the DIAGNOSTIC_DIR environment variable. Allows image tests to reference the log file. Additional environment variables added include:

DIAGNOSTIC_ENABLE : false by default. Runs verify installation command at startup

DIAGNOSTIC_ONLY : false by default. Only runs verify-installation command and then exits the script.

[colearendt]

This looks great! A couple of random ideas 😄

[colearendt]

Another idea I just had - when this "diagnostic report" ends, it will start the product as normal. Maybe we should have an environment variable that, if set, causes the script to exit 0 after the diagnostic report ends? i.e. DIAGNOSTIC_ONLY or something like that.

[nahara7]

Another idea I just had - when this "diagnostic report" ends, it will start the product as normal. Maybe we should have an environment variable that, if set, causes the script to exit 0 after the diagnostic report ends? i.e. DIAGNOSTIC_ONLY or something like that.

I like this ! Will do !

[colearendt]

Looking really good! A few more little tidbits 😄

[colearendt]

I'm curious - why the sleep?

[nahara7]

I originally had a wait command. It took quite some time to fetch the job output at the end of the line.
...Job has finished running Fetching job output...
The sleep command allowed me to view the file when I was sure the directory existed but not waiting as long. Is there a different way you think I should do ?

[bdeitte]

Looks good to me! My only suggestion would be that we document the environment variables somewhere in a README. But my guess is this is a more wide-ranging issue related to docs and not sure if it conflicts with any of @colearendt's doc work that I haven't gotten a chance to look through.

[colearendt]

Ok, had a long chat w/ the RSW team today. TL;DR; I think we need to:

• remove the & / backgrounding of the verify-installation command
• remove the sleep 5

What was happening with the hang you experienced was the product was stuck in a state where it could not start the job, but wasn't erroring and also wasn't timing out. The team is now aware and will hopefully improve the product to be a bit more clear in the long run, but backgrounding the process can actually hide some of these concerning issues, so I think foreground is the right call. If the product isn't working, we want to know about it! 😄

This is an example that reproduces the problem (using this branch / the public image after we merge):

docker run -v $(pwd)/server-pro/startup.sh:/usr/local/bin/startup.sh -e DIAGNOSTIC_ENABLE=true -e DIAGNOSTIC_ONLY=true -it --rm rstudio/rstudio-workbench:1.4.1717-3

And this example works as we expect (added --privileged) (I was mounting the startup script so I did not have to rebuild the image with each tweak)

docker run --privileged -v $(pwd)/server-pro/startup.sh:/usr/local/bin/startup.sh -e DIAGNOSTIC_ENABLE=true -e DIAGNOSTIC_ONLY=true -it --rm rstudio/rstudio-workbench:1.4.1717-3

I'm not super opinionated about docs, to be honest. I am AOK if we merge this and then follow up with docs once things have stabilized in the repo a bit. We've done a lot of work in this PR already 😄

I'll send y'all a link to the slack thread privately

[colearendt]

Woohoo! Looks awesome!