New changes in Dockerfile break Dashboard on Staging

[RC-Lee]

What happened:
Dashboard is broken on staging.
(Clicking on the "i" on the hero banner)
https://dev.api.telescope.cdot.systems/v1/status/

What should have happened:
Should show the dashboard

How to reproduce it (as precise as possible):
Go here: https://dev.api.telescope.cdot.systems/v1/status/

Anything else we need to know?:
The docker logs show a healthcheck fail from the Status image. (Sometimes it happens to Posts too)

{"level":30,"time":1647656844771,"pid":1,"hostname":"62b3e97e79c2","msg":"Server started on port 1111"}
{"level":50,"time":1647656863276,"pid":1,"hostname":"62b3e97e79c2","msg":"healthcheck failed"

I went through the commit history to traceback the commit that may have caused this.
It seems like it was commit e1b73fd19100a9d91a458ac538b2f349a6744340 that had this problem
The commit before this is stable, and the dashboard works.

Scanning the changes in the commit, it seems like this line here is terminating the process as an error when healthcheck fails.

HEALTHCHECK --interval=30s --timeout=30s --start-period=5s --retries=3 \
	CMD wget --no-verbose --tries=1 --spider localhost:${PORT}/healthcheck || exit 1

I tested locally changing exit 1 to exit 0 and the dashboard showed.
This is only a temporary solution, but I think we should investigate more on healthcheck.

[cindyledev]

Would it make sense to rip out the HEALTHCHECKs temporarily until we can figure out to get them to work properly?

[RC-Lee]

Yes if it temporarily gets the dashboard working.
But we should continue to investigate.

[cindyledev]

My guess is staging is not happy with the localhost from this line

telescope/src/api/status/Dockerfile

Line 53 in 7944fa0

CMD wget --no-verbose --tries=1 --spider localhost:${PORT}/healthcheck || exit 1

[RC-Lee]

I'm just baffled cause it seems some other dockerfiles have this line as well but they don't seem to be affected.
Just Status and sometimes Posts

[humphd]

The problem here is that we're asking the HEALTHCHECK to look at ${PORT}, which will be 3000 on staging: https://github.com/Seneca-CDOT/telescope/blob/master/config/env.staging#L184. However, the status service uses STATUS_PORT=1111: https://github.com/Seneca-CDOT/telescope/blob/master/config/env.staging#L45.

We should be using STATUS_PORT vs. PORT in this file: https://github.com/Seneca-CDOT/telescope/blob/master/src/api/status/Dockerfile#L47

[Kevan-Y]

So PORT is overwriting our env PORT in dockerfile?

[manekenpix]

@humphd I'm checking all the dockerfiles, and they all use ${PORT}, but only status seems to be affected. Also, I'm surprised nobody saw this in local testing, because we're all testing every PR locally, right, RIGHT?

[Kevan-Y]

@manekenpix , should I fix all the other dockerfiles to use the right env variable in this issue?

[manekenpix]

@Kevan-Y I don't even know if that's the issue. All the other dockerfiles use the same var, but none of them are affected (maybe posts, but I'd have to confirm).

[Kevan-Y]

@manekenpix I don't think it's an issue of PORT maybe, on portainer I can see that the env PORT is correctly set to 1111

[manekenpix]

@Kevan-Y thanks for checking 👍

[humphd]

Regardless, PORT is wrong. All of the Dockerfiles use their own special {service-name}_PORT syntax, so this should get fixed.

[Kevan-Y]

Test scenario (each scenario doesn't contain any change from previous scenario):

1. removing || exit 1 in Dockerfile

   • http://localhost:1111/ able to access
   • http://localhost:1111/healthcheck showing 503 with {"status":"error"}
   • http://localhost/v1/status 404 page not found
   • http://localhost/v1/status/healthcheck 404 page not found

2. Changing || exit 1 to || exit 0 in Dockerfile

   • http://localhost:1111/ able to access
   • http://localhost:1111/healthcheck showing 503 with {"status":"error"}
   • http://localhost/v1/status able to access
   • http://localhost/v1/status/healthcheck showing 503 with {"status":"error"}

3. Comment HEALTHCHECK in Dockerfile

   • http://localhost:1111/ able to access
   • http://localhost:1111/healthcheck showing 503 with {"status":"error"}
   • http://localhost/v1/status able to access
   • http://localhost/v1/status/healthcheck showing 503 with {"status":"error"}

4. Comment healthCheck in server.js

   • http://localhost:1111/ able to access
   • http://localhost:1111/healthcheck showing 200 with {"status":"ok"}
   • http://localhost/v1/status able to access
   • http://localhost/v1/status/healthcheck showing 200 with {"status":"ok"}

I think in getPackage() function we need GITHUB_ACCESS_TOKEN env to be defined but in our case, it's not. So it fails in there for our healthcheck route.
Since the healthcheck route is failing and with the new Dockerfile change recently HEALTHCHECK is exit 1, so our status is not accessible and shows unhealthy in portainer.

telescope/src/api/status/src/server.js

Line 55 in 78bd995

const { version } = await getPackage('https://github.com/Seneca-CDOT/telescope/blob/master/');

[humphd]

Anything that gets awaited, needs a try/catch or we risk crashing (and here we are).

However, I think this getPackage call is not needed. Let's pass the value in as a build ARG to the Dockerfile.

In a GitHub Actions Workflow, you can get the following:

1. name of branch or tag: ${{ github.ref_name }}
2. git sha: ${{ github.sha }}

We can stamp these values into the status or even nginx images when we build them, and staging/production can simply read them when they run vs. needing to query anything.

I already do this for all the URLs we pass to next.js when we build the front-end, see https://github.com/Seneca-CDOT/telescope/blob/master/.github/workflows/docker-build-and-push.yml. We could add some more build-args to the appropriate container, and pass these values through. Then in the Dockerfile, we accept the ARG and set an ENV value so it will exist at runtime.

[Kevan-Y]

I was looking at the previous commit. But don't remember why we return version, sha, gitHubUrl.

[humphd]

It's so that the front-end could get it to display on the front landing page. But we don't need to do that in the status service, we can build that info into the nginx front-end container statically.

[manekenpix]

In a GitHub Actions Workflow, you can get the following:

name of branch or tag: ${{ github.ref_name }}
git sha: ${{ github.sha }}

We can stamp these values into the status or even nginx images when we build them, and staging/production can simply read them when they run vs. needing to query anything.

If we do this, we'll have undefined in development and we won't be able to display telescope's version and commit, are we ok with that?

[humphd]

I don't think it matters as much in development.

[Kevan-Y]

It's so that the front-end could get it to display on the front landing page. But we don't need to do that in the status service, we can build that info into the nginx front-end container statically.

I checked the code, in our front-end we don't call the status healthcheck to get that info.
https://github.com/Seneca-CDOT/telescope/blob/master/src/web/src/components/Banner.tsx#L145
https://github.com/Seneca-CDOT/telescope/blob/master/src/backend/web/routes/health.js

In development, when we run pnpm start for starting our backend, the front end will be able to get the version displayed.