cvat
cvat copied to clipboard
[GSoC2024] update helm chart: Add common sub-chart
Motivation and context
There has been multiple issue coming due to conflict in common chart version of Redis and Postgresql. Fixes #7449 Fixes #7507
How has this been tested?
- Apply Helm chart with analytics enabled (Result: helm chart working)
- Now, try to run helm chart again, with analytics disabled (Result: helm chart will get an error) Below is the error:
Error: template: cvat/charts/redis/templates/master/service.yaml:12:14: executing "cvat/charts/redis/templates/master/service.yaml" at <include "common.labels.standard" (dict "customLabels" .Values.commonLabels "context" $)>: error calling include: template: cvat/charts/postgresql/charts/common/templates/_labels.tpl:6:27: executing "common.labels.standard" at <include "common.names.name" .>: error calling include: template: cvat/charts/postgresql/charts/common/templates/_names.tpl:6:18: executing "common.names.name" at <.Chart.Name>: nil pointer evaluating interface {}.Name
helm.go:84: [debug] template: cvat/charts/redis/templates/master/service.yaml:12:14: executing "cvat/charts/redis/templates/master/service.yaml" at <include "common.labels.standard" (dict "customLabels" .Values.commonLabels "context" $)>: error calling include: template: cvat/charts/postgresql/charts/common/templates/_labels.tpl:6:27: executing "common.labels.standard" at <include "common.names.name" .>: error calling include: template: cvat/charts/postgresql/charts/common/templates/_names.tpl:6:18: executing "common.names.name" at <.Chart.Name>: nil pointer evaluating interface {}.Name
Checklist
- [x] I submit my changes into the
develop
branch - [ ] I have created a changelog fragment
- [x] I have updated the documentation accordingly
- [x] I have added tests to cover my changes
- [x] I have linked related issues (see GitHub docs)
- [x] I have increased versions of npm packages if it is necessary (cvat-canvas, cvat-core, cvat-data and cvat-ui)
License
- [x] I submit my code changes under the same MIT License that covers the project. Feel free to contact the maintainers if that's a concern.
@abhi-bhatra , could you please add a link into CHANGELOG.md? See instructions inside the file on how to do that.
@abhi-bhatra thanks for the contribution I have one question: Are you sure that the current versions of Redis, Postgresql and Clickhouse charts will be compatible with all future versions of common chart v2.x.x?
Hi @azhavoro Thanks for looking at the contribution. I am sure about the version compatibility. I have checked version of common chart in all these 3 charts, and used the static version which will not create an ambiguity of choosing a version by other charts.
dependencies:
- name: common
repository: https://charts.bitnami.com/bitnami
tags:
- bitnami-common
version: 2.x.x
But, still I will run few more test by changing versions of common chart and will see that if it is compatible with Redis, Postgresql and Clickhouse charts.
Codecov Report
Merging #7581 (6574bd5) into develop (f513aa1) will decrease coverage by
0.12%
. Report is 39 commits behind head on develop. The diff coverage isn/a
.
Additional details and impacted files
@@ Coverage Diff @@
## develop #7581 +/- ##
===========================================
- Coverage 83.47% 83.35% -0.12%
===========================================
Files 373 373
Lines 39739 39722 -17
Branches 3741 3747 +6
===========================================
- Hits 33171 33112 -59
- Misses 6568 6610 +42
Components | Coverage Δ | |
---|---|---|
cvat-ui | 79.20% <ø> (-0.09%) |
:arrow_down: |
cvat-server | 87.21% <100.00%> (-0.13%) |
:arrow_down: |
Wouldn't it be better to pin the common chart version (or at least a minor version) in case of possible backwards compatibility problems in the future?
Also, probably need to add a simple test to sure cvat can be deployed without analytics, which would certainly help to quickly detect possible problems in the future.
@abhi-bhatra , will you be able to complete the PR?
HI @nmanovic I am working on it, and will complete it by today EOD
@azhavoro I have pinned the common chart version, but I would like you to point me where to add test for helm charts ?
Should I create a bash script ?
@azhavoro I have pinned the common chart version, but I would like you to point me where to add test for helm charts ?
Should I create a bash script ?
I think it can be separate job in this workflow https://github.com/opencv/cvat/blob/develop/.github/workflows/helm.yml
Thanks @azhavoro I am adding test case in this workflow. Thanks for guiding me !
@azhavoro I have added a simple test case in the workflow, Can you review it ?
@abhi-bhatra Hello, the test example is not working, could you please fix it?
I can't merge this PR due to failed tests