Infrastructure: Add pull request deployment using Helm
#9618
Conversation
Signed-off-by: Colin Wilk <[email protected]>
WalkthroughThe pull request introduces several changes to the Helm chart for the Artemis application. A new entry, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Application
User->>Helm: Deploy Artemis Chart
Helm->>Application: Check service type (Ingress, NodePort, LoadBalancer, ClusterIP)
Application-->>Helm: Return service details
Helm-->>User: Provide access instructions based on service type
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (35)
.github/workflows/helm_pr_deployment.ymlis excluded by!**/*.yml.github/workflows/helm_pr_deployment_delete.ymlis excluded by!**/*.ymlhelm/artemis/Chart.lockis excluded by!**/*.lock,!**/*.lockhelm/artemis/Chart.yamlis excluded by!**/*.yamlhelm/artemis/templates/autoscaler/horizontal-pod-autoscaler.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/activemq-broker-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/artemis-ci-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/artemis-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/artemis-mysql-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/artemis-usermanagement-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/artemis-vcs-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/jhipster-registry-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/configmaps/jhipster-registry-env-configmap.ymlis excluded by!**/*.ymlhelm/artemis/templates/deployments/_artemis-deployment.ymlis excluded by!**/*.ymlhelm/artemis/templates/deployments/activemq-broker.ymlis excluded by!**/*.ymlhelm/artemis/templates/deployments/artemis-deployment-profile.yamlis excluded by!**/*.yamlhelm/artemis/templates/deployments/artemis-deployment.yamlis excluded by!**/*.yamlhelm/artemis/templates/ingresses/ingress.yamlis excluded by!**/*.yamlhelm/artemis/templates/monitors/podmonitor.ymlis excluded by!**/*.ymlhelm/artemis/templates/pvc/artemis-mysql.ymlis excluded by!**/*.ymlhelm/artemis/templates/pvc/artemis.ymlis excluded by!**/*.ymlhelm/artemis/templates/secrets/activemq-broker-secrets.ymlis excluded by!**/*.ymlhelm/artemis/templates/secrets/artemis-secrets.ymlis excluded by!**/*.ymlhelm/artemis/templates/secrets/artemis-usermanagement-secrets.ymlis excluded by!**/*.ymlhelm/artemis/templates/secrets/jhipster-registry-secrets.ymlis excluded by!**/*.ymlhelm/artemis/templates/services/activemq-broker-service.ymlis excluded by!**/*.ymlhelm/artemis/templates/services/artemis-service-profile.yamlis excluded by!**/*.yamlhelm/artemis/templates/services/artemis-service.yamlis excluded by!**/*.yamlhelm/artemis/templates/services/jhipster-registry-service.yamlis excluded by!**/*.yamlhelm/artemis/templates/services/mysql-service.yamlis excluded by!**/*.yamlhelm/artemis/templates/statefulsets/artemis-mysql.ymlis excluded by!**/*.ymlhelm/artemis/templates/statefulsets/artemis-statefulset.yamlis excluded by!**/*.yamlhelm/artemis/templates/statefulsets/jhipster-registry.ymlis excluded by!**/*.ymlhelm/artemis/templates/tests/test-connection.yamlis excluded by!**/*.yamlhelm/artemis/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (3)
- helm/.gitignore (1 hunks)
- helm/artemis/templates/NOTES.txt (1 hunks)
- helm/artemis/templates/_helpers.tpl (1 hunks)
🧰 Additional context used
🪛 LanguageTool
helm/artemis/templates/NOTES.txt
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ...ervices {{ include "artemis.fullname" . }}) export NODE_IP=$(kubectl get nodes ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... svc -w {{ include "artemis.fullname" . }}' export SERVICE_IP=$(kubectl get svc...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...pace }} {{ include "artemis.fullname" . }} --template "{{"{{ range (index .status...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ...tes.io/name={{ include "artemis.name" . }},app.kubernetes.io/instance={{ .Release...(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (6)
helm/artemis/templates/_helpers.tpl (6)
1-6: LGTM: Name template follows Helm best practicesThe implementation correctly handles name overrides and follows Kubernetes naming conventions.
8-24: LGTM: Fullname template handles all edge casesThe implementation properly handles all name override scenarios and follows Kubernetes naming conventions.
26-31: LGTM: Chart name template handles version formatting correctlyThe implementation properly sanitizes version strings and follows naming conventions.
45-51: LGTM: Selector labels follow Kubernetes best practicesThe implementation includes the minimum required labels for reliable pod selection.
53-62: LGTM: Service account naming logic is correctThe implementation properly handles service account creation and naming scenarios.
1-82: Verify template usage in other Helm resourcesThe helper templates look good overall. Let's verify their usage across the Helm chart.
✅ Verification successful
Let me gather more specific information about the template usage to verify their correctness.
Helper templates are correctly used across the Helm chart
The verification shows that all helper templates defined in
_helpers.tplare properly used across the Helm chart resources:
artemis.nameis used for app name labels and pod selectorsartemis.fullnameis consistently used for resource naming across deployments, services, and other resourcesartemis.labelsis properly indented and used for metadata labelsartemis.selectorLabelsis correctly applied in pod templates and service selectorsartemis.springprofilesis properly used in configmaps and statefulsets for Spring profile configurationThe templates follow Helm best practices with proper indentation and consistent usage patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of template functions across the Helm chart echo "Checking template function usage..." rg -l "include \"artemis\." helm/artemis/templates/ echo "Checking for potential template syntax errors..." rg -l "{{-?\s*include\s+[\"']artemis\.[^\"']+[\"']\s*\." helm/artemis/templates/ | \ while read -r file; do echo "Analyzing $file..." cat "$file" | grep -n "{{-\?\s*include\s\+\"artemis\." doneLength of output: 1157
Script:
#!/bin/bash # Check specific template function usage patterns echo "Checking artemis.name usage..." rg "include \"artemis.name\"" helm/artemis/templates/ -A 1 echo -e "\nChecking artemis.fullname usage..." rg "include \"artemis.fullname\"" helm/artemis/templates/ -A 1 echo -e "\nChecking artemis.labels usage..." rg "include \"artemis.labels\"" helm/artemis/templates/ -A 1 echo -e "\nChecking artemis.selectorLabels usage..." rg "include \"artemis.selectorLabels\"" helm/artemis/templates/ -A 1 echo -e "\nChecking artemis.serviceAccountName usage..." rg "include \"artemis.serviceAccountName\"" helm/artemis/templates/ -A 1 echo -e "\nChecking artemis.springprofiles usage..." rg "include \"artemis.springprofiles\"" helm/artemis/templates/ -A 1Length of output: 8263
|
🚀 Deployed PR #9618 to https://pr9618.artemis-k8s.ase.cit.tum.de ⏳ It might take up to 10 minutes to fully start up. |
|
🚀 Deployed PR #9618 to https://pr9618.artemis-k8s.ase.cit.tum.de ⏳ It might take up to 10 minutes to fully start up. |
kliwniloc
force-pushed
the
feature/helm-pr-deployment
branch
from
3e4799a
to
23d1004
Compare
Bump versions for Chart and appVersion. Signed-off-by: Colin Wilk <[email protected]>
Add url for k8s env Signed-off-by: Colin Wilk <[email protected]>
|
🚀 Deployed PR #9618 to https://pr9618.artemis-k8s.ase.cit.tum.de ⏳ It might take up to 10 minutes to fully start up. |
|
🚀 Deployed PR #9618 to https://pr9618.artemis-k8s.ase.cit.tum.de ⏳ It might take up to 10 minutes to fully start up. |
Add condition to check if delpoy:k8s tag exists for the PR Signed-off-by: Colin Wilk <[email protected]>
|
🚀 Deployed PR #9618 to https://pr9618.artemis-k8s.ase.cit.tum.de ⏳ It might take up to 10 minutes to fully start up. |
Please add it to the documentation inside the repo -> https://docs.artemis.cit.tum.de/dev/testservers.html File path: |
Signed-off-by: Colin Wilk <[email protected]>
Set cancel-in-progress to true so that in scenarios where commits are pushed fast or multiple tags are added at once we only run the most recent pipeline. Signed-off-by: Colin Wilk <[email protected]>
There was a problem hiding this comment.
Nice change! I think this will really make the test flow much smoother and would also allow to have new DBs for every test 😍
RE: Automatically deploy to K8s and not wait for admin (mtze) approval. I think in order to really profit from this enhancement, the student must be trusted enough to deploy to K8s at their own will. Potentially, this opens up the option for deployers/students to run any pod with any functionality on the cluster by modifying the helm charts. In the system, any abuse can be detected using logs and analytics, tracked to the student, and disciplinary actions can take place. Thus, I see no problem here.
When adding multiple labels at the same time, the job is currently executing multiple times. After removing the barrier of manual approval, this would re-deploy the commit multiple times drastically increasing setup time. Please make sure that the job only successfully executed exactly once 👍
|
Thanks for the Feedback!
This should be fixed with the cancel-in-progress setting in f4da4ed Regarding the Approvals: Anybody with admin access can change this in the Environments - this does not require any changes to my pr. |
|
🚀 Deployed PR #9618 to https://pr9618.artemis-k8s.ase.cit.tum.de ⏳ It might take up to 10 minutes to fully start up. |
|
When a developer or tester wants to verify that the changes work on a newly set up environment, this is a great addition. However, I see a few challenges / limitations:
Please mention those limitations as part of the PR and the documentation so everyone has a good understanding of the advantages and limitations of the new deployment option! |
Checklist
General
Server
None
Client
None
Changes affecting Programming Exercises
None
Description
This Pull Request merges the Helm chart from https://github.com/ls1intum/artemis-helm with my changes proposed in ls1intum/artemis-helm#18 and adds GitHub action workflows to continuously deploy pull requests to Kubernetes.
Problem
Artemis incorporates Continuous Delivery in its development workflow to perform
testing and Quality Assurance on code.
In the existing architecture, the administrators maintain over 20 virtual
machines with different version control systems and continuous integration systems for Artemis test
servers to deploy the pull requests on.
scaling those instances is a manual process limiting the number of available
test instances.
Often, the instances are not redeployed after every code review, which is
prone to configuration drift over time, leading to broken test servers and deployment related errors during
pull request deployment review.
Motivation
The advancements in modern software deployment practices, epitomized by adopting
containerization and orchestration technologies such as Kubernetes, underscore a
pivotal shift towards more efficient, reliable, and scalable application
management.
The deployment and continuous development of complex
platforms like Artemis encapsulates the pressing need for deployment
architectures that not only scale but also simplify operational complexities.
Leveraging Kubernetes to scale and manage Artemis' pull request
deployments transforms Artemis's quality assurance workflow.
This approach mitigates the configuration drift and enhances the efficiency and
reliability of testing environments.
Developers can assure higher quality standards and faster iteration cycles by
enabling on-demand, scalable test instances.
This is critical in an academic setting where the platform's stability and
functionality directly affect the learning outcomes and overall user experience.
Changes to the Helm Chart:
Dependencies Updated:
prometheusdependency inChart.lock.artemisVersionto7.5.0invalues.yaml.Template Updates:
_helpers.tpl.artemis-ci-configmap.ymlwith new continuous integration parameters.artemis-configmap.ymlto include JWT for registry security.artemis-vcs-configmap.ymlto support a more generic version control configurations.Deployment Changes
activemq-broker,artemis-mysql.yml, andjhipster-registryconfigurable via values.yaml (Introduced resource requests and limits forartemis,database,registry, andbroker.)_artemis-deployment.ymlandartemis-statefulset.yaml.These changes incorporate configuration values of newer Artemis releases coming from the artemis ansible collection https://github.com/ls1intum/artemis-ansible-collection.
Testing
Check this PR deployment.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
NOTES.txtfile with detailed instructions for accessing the Artemis application based on service type.Chores
.gitignorefile to exclude files and directories namedcharts.