Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces Kubernetes deployment support for TeSS by adding new manifests, documentation and updating configuration files, as well as adjusting contributor listings and ignore files.
- Added new Kubernetes manifests for deploying TeSS components (app, db, redis, sidekiq, backup, and setup job)
- Updated environment configuration and database setups to include DB_PORT and other necessary changes
- Updated documentation and contributor information to reflect new deployment workflows and team contributions
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| k8s/sidekiq-deployment.yaml | Added Sidekiq deployment manifest |
| k8s/redis-service.yaml | Added Redis service manifest |
| k8s/redis-persistentvolumeclaim.yaml | Added PVC for Redis |
| k8s/redis-deployment.yaml | Added Redis deployment manifest |
| k8s/logs-persistentvolumeclaim.yaml | Added PVC for logs |
| k8s/db-service.yaml | Added Service manifest for DB |
| k8s/db-persistentvolumeclaim.yaml | Added PVC for DB |
| k8s/db-deployment.yaml | Added DB deployment manifest |
| k8s/backup-persistenvolumeclaim.yaml | Added PVC for backups (file name appears misspelled) |
| k8s/backup-cronjob.yaml | Added cronjob manifest for backups |
| k8s/app-setup-job.yaml | Added job manifest for app setup |
| k8s/app-service.yaml | Added app service manifest |
| k8s/app-route.yaml | Added route manifest for app (openshift) |
| k8s/app-deployment.yaml | Added app deployment manifest |
| env.sample | Updated sample env file to include DB_PORT |
| docs/kubernetes.md | Added documentation for Kubernetes deployment |
| config/database.yml | Updated database config to fetch DB_PORT |
| Dockerfile-k8 | Added Dockerfile for Kubernetes builds |
| CONTRIBUTORS.md | Updated contributor listing and ordering |
| .dockerignore | Updated dockerignore file with .DS_Store |
Comments suppressed due to low confidence (1)
docs/kubernetes.md:7
- The word 'aswell' should be corrected to 'as well' for clarity and correctness.
Any modification of this `k8s/` directory must be updated in the [TeSS Helm repo](https://github.com/ElixirTeSS/TeSS-Helm) aswell. Thank you 🙏
k8s/backup-persistenvolumeclaim.yaml
Outdated
| @@ -0,0 +1,10 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
The file name 'backup-persistenvolumeclaim.yaml' appears to have a typo. Consider renaming it to 'backup-persistentvolumeclaim.yaml' for clarity and consistency.
k8s/app-route.yaml
Outdated
| to: | ||
| kind: Service | ||
| name: app-service | ||
| weight: null |
There was a problem hiding this comment.
The 'weight: null' field might be unnecessary; consider removing it or assigning a numeric value to improve clarity.
| weight: null |
env.sample
Outdated
|
|
||
| # Database (db is the docker container name) | ||
| DB_HOST=db | ||
| DB_PORT= |
There was a problem hiding this comment.
An empty DB_PORT in env.sample may lead to deployment issues; consider providing a default value (e.g., DB_PORT=5432) consistent with the documentation.
| DB_PORT= | |
| DB_PORT=5432 |
| * Finn Bacall <finn.bacall@manchester.ac.uk> | ||
| * Servilio Afre Puentes <afrepues@sharcnet.ca> | ||
| * Aitor Apaolaza | ||
| * Finn Bacall <finn.bacall@manchester.ac.uk> |
There was a problem hiding this comment.
There appear to be duplicate contributor entries. Consider deduplicating to maintain a clear and accurate contributor list.
There was a problem hiding this comment.
This is wrong, there are no duplicates in CONTRIBUTORS.md
|
After discussions with @fbacall, we decided to only have a Helm chart deployment through a separate Helm chart repository. This is avoiding maintenance of a Before approving the PR I'd suggest to :
|
Dockerfile-k8
Outdated
There was a problem hiding this comment.
Is it possible to avoid duplicating the existing Dockerfile?
For example, this dockerfile is out of sync with the main Dockerfile (different ruby version): https://github.com/ElixirTeSS/TeSS/blob/master/Dockerfile#L1
Could we inject the necessary K8 steps in there somehow?
…narqube style issues
Dockerfile
Outdated
| SHELL [ "/bin/bash", "-c" ] | ||
| ENTRYPOINT bundle exec rails server -b 0.0.0.0 |
There was a problem hiding this comment.
This change (and the one at the end of the file) seems to prevent running an alternative command in the container, for example to run tests: https://github.com/ElixirTeSS/TeSS/actions/runs/15994949196/job/45116138794
was there a specific reason for it?
There was a problem hiding this comment.
It was for linting reasons (ref), we can skip it.
Summary of changes
Concerns #1112
Additions:
Dockerfile-k8,docs/kubernetes.mdand every Kubernetes manifests (i.e., files) underk8s/Modifications:
DB_PORTin mydb-deployment.yamlKubernetes manifest, I added the mention of PORT everywhere that I needed.env.sampleconfig/database.yml.gitignore: addedapp-secrets*which are Kubernetes manifests created by the users and act like thetess.ymlandsecrets.ymlfiles, but for Kubernetes.dockerignore: added.DS_Store, a MacOS folder/Finder config fileCONTRIBUTORS.md: added Kenneth and listed in alphabetical orderMotivation and context
I needed to deploy TeSS using OpenShift/Kubernetes in my institution, this is why I share the knowledge here!
Screenshots
Checklist
to license it to the TeSS codebase under the
BSD license.