Resolve docker image build issue in example server

[rvenables]

Fixes #35 by switching the npm build command in the Dockerfile from npm build to npm run build. Additionally, these changes make future related issues more visible by adding the docker image build to a new example server docker-test script and updating the related automated GitHub workflow to run the new test script.

Impact of Docker Image Build on Test Run Time Adding the docker image build to the (after adjustment) docker-test script for the opr-example-server will increase the total time required to run tests for the project in github actions. All else being equal, slower test times and more related compute are obviously less desirable for many reasons including reduced productivity and - at scale - even environmental impact [1]. Running the project locally, I estimate the approximate incremental impact to be an additional 30s-1m for first-run test executions, and <5s for subsequent test executions utilizing the (docker) cache. Running in GitHub actions, I estimate a similar impact to the first-run execution (30s-1m). These estimates are based on a few quick local benchmarks on an m2 MacBook Air and in GitHub actions.

Before...

✅ After Changes - Local Docker Build

✅ After Changes - Check Against Running Server in Container

✅ After Changes - Personal Repository GitHub Action Failure on (Simulated) Docker Failure (10/31)

✅ After Changes - Personal Repository GitHub Action Success on Successful Build (10/31)

• [x] Manual check against running application successful
• [x] No updates to documentation required

[1] The environmental impact here appears to be very low, even with many contributors. Considered annually, 1,000 (non-cached) image builds per day 0.00016kWh per 1 minute of compute per cache miss 365 days in a year * EPA conversion factor = 0.026 metric tons of Carbon Dioxide (CO2) equivalent. That’s roughly equivalent to driving a single gasoline powered car 60 miles.
 Note: this conversion was roughly calculated and not independently verified.

See https://www.epa.gov/energy/greenhouse-gases-equivalencies-calculator-calculations-and-references for kWh to CO2 conversion factor and https://www.epa.gov/energy/greenhouse-gas-equivalencies-calculator for equivalence examples.

On a related note, GitHub claims to be carbon neutral (See https://github.blog/2021-04-22-environmental-sustainability-github/) and that appears to include the compute required for actions.

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[rvenables]

I see the CLA issue - caused by a commit on a new laptop. I'll take a look in a few minutes.

[rvenables]

CLA issue resolved via rebase on my repository to update email address.

[ryckmanat]

Thanks for this, Rob - and great catch. No idea how this error got in there.

I saw the comment from @johndayrichter on the issue, and maybe I should put this there... but, personally, I don't love adding docker builds to the general tests. The headache is that would add docker as a requirement for everyone, and the core code doesn't require it: Docker is only used in the examples.

Maybe rather than adding docker builds to our main tests, we could have a separate test call, specific to the examples folder, that would build the containers. That way, a developer not building in there wouldn't need to worry about having docker available. Maybe there's a way we could require the docker build tests if and only if a PR makes a change to something under examples/ ?

[rvenables]

@ryckmanat Great feedback - thank you. Personally, I don't have a strong opinion on this one. I think your argument about Docker not being a core requirement is fair. It's clearly possible to run/use OPR without any intention of using Docker. While docker is increasingly becoming a mainstream tool, it seems plausible that some developers won't have it installed or want to use it.

@johndayrichter / @ryckmanat: In line @ryckmanat's suggestion, what do you think about adding a new target "test-docker" that just runs the docker build for now in the example server project? Then, adding that target as a last step to the workflow for now. Correspondingly, developers that run the workspace test target locally will not exercise the docker build but any changes that cause the docker image build to fail will be apparent on pull request / push.

[johndayrichter]

@ryckmanat Great feedback - thank you. Personally, I don't have a strong opinion on this one. I think your argument about Docker not being a core requirement is fair. It's clearly possible to run/use OPR without any intention of using Docker. While docker is increasingly becoming a mainstream tool, it seems plausible that some developers won't have it installed or want to use it.

I hadn't considered the point Mike is making here - it's probably particularly salient for Mike and I actually, because some Google hardware configurations don't allow docker to be installed at all.

I think we do need Docker tests, because it's so easy to introduce mistakes that break Docker builds but not dev builds. However...

@johndayrichter / @ryckmanat: In line @ryckmanat's suggestion, what do you think about adding a new target "test-docker" that just runs the docker build for now in the example server project? Then, adding that target as a last step to the workflow for now. Correspondingly, developers that run the workspace test target locally will not exercise the docker build but any changes that cause the docker image build to fail will be apparent on pull request / push.

I think this is an excellent suggestion. Rob, would you be willing to change the test rule you added to a test-docker rule, revert the "test" rule to the original no-op, and add test-docker run to the github action?

Thanks so much, everyone!

[rvenables]

Sounds great. I'll update this PR with those small adjustments later today.