Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Helm] multiples fixes needed #6043

Closed
Keramblock opened this issue Apr 19, 2023 · 2 comments · Fixed by #6137
Closed

[Helm] multiples fixes needed #6043

Keramblock opened this issue Apr 19, 2023 · 2 comments · Fixed by #6137

Comments

@Keramblock
Copy link
Contributor

Keramblock commented Apr 19, 2023

Hi, I cannot create another PR right now, or update current one, but I decided at least to write down my chart concerns in case someone else could fix that:

  1. Treafik is hardcoded to values right now and that is bad, because it is not standard package and people could use other software as ingress(nginx/istio/ambassador/etc)
  2. Previously there was a way to specify psql db creds via secret, that was for some reason removed in new versions. That is important because some people(including me) using Terraform to create db in cloud and provirde k8s secret to pass creds to cvat
  3. Right now I see, that there is 5-6 deployments of cvat deployed by default - are you sure, that I need all of them just to start cvat on my cluster? My guess that at least part of them should be opt-in and switched off by default. I see that compose with https uses only one!
  4. Right now there is only one PVC for all of the deployments, I think that will not work from the box for almost anybody, who tried to use this chart
@Keramblock
Copy link
Contributor Author

I tried to fix at least some of them in now outdated PR, mb you find it useful: #5522

@Keramblock
Copy link
Contributor Author

bsekachev pushed a commit that referenced this issue Jul 25, 2023
<!-- Raise an issue to propose your change
(https://github.com/opencv/cvat/issues).
It helps to avoid duplication of efforts from multiple independent
contributors.
Discuss your ideas with maintainers to be sure that changes will be
approved and merged.
Read the [Contribution
guide](https://opencv.github.io/cvat/docs/contributing/). -->

<!-- Provide a general summary of your changes in the Title above -->

### Motivation and context
<!-- Why is this change required? What problem does it solve? If it
fixes an open
issue, please link to the issue here. Describe your changes in detail,
add
screenshots. -->
Right now helm chart is broken and not usable at least in my
environment, I trying to fix it to make it work
content: 

1. Moved test-related values to another values.file to separate it from
default config
2. fixed issue with multiple caches in same RWX volume, which prevents
db migration to start
3. Removed hardcoded mandatory traefik ingress usage
4. Added confugurable default storage option to chart

### How has this been tested?
<!-- Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc. -->
We test it on our AKS with RWX volume

### Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply.
If an item isn't applicable for some reason, then ~~explicitly
strikethrough~~ the whole
line. If you don't do that, GitHub will show incorrect progress for the
pull request.
If you're unsure about any of these, don't hesitate to ask. We're here
to help! -->
- [x] I submit my changes into the `develop` branch
- [x] I have added a description of my changes into the
[CHANGELOG](https://github.com/opencv/cvat/blob/develop/CHANGELOG.md)
file
- [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](

https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))
- [x] I have increased versions of npm packages if it is necessary

([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning),

[cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning),

[cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning)
and

[cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning))

### License

- [x] I submit _my code changes_ under the same [MIT License](
https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the
project.
  Feel free to contact the maintainers if that's a concern.
  
closes #6043 
closes #6096 
closes #5733

---------

Co-authored-by: Michael Kirpichev <[email protected]>
Co-authored-by: Nikita Manovich <[email protected]>
Co-authored-by: Andrey Zhavoronkov <[email protected]>
mikhail-treskin pushed a commit to retailnext/cvat that referenced this issue Oct 25, 2023
<!-- Raise an issue to propose your change
(https://github.com/opencv/cvat/issues).
It helps to avoid duplication of efforts from multiple independent
contributors.
Discuss your ideas with maintainers to be sure that changes will be
approved and merged.
Read the [Contribution
guide](https://opencv.github.io/cvat/docs/contributing/). -->

<!-- Provide a general summary of your changes in the Title above -->

### Motivation and context
<!-- Why is this change required? What problem does it solve? If it
fixes an open
issue, please link to the issue here. Describe your changes in detail,
add
screenshots. -->
Right now helm chart is broken and not usable at least in my
environment, I trying to fix it to make it work
content: 

1. Moved test-related values to another values.file to separate it from
default config
2. fixed issue with multiple caches in same RWX volume, which prevents
db migration to start
3. Removed hardcoded mandatory traefik ingress usage
4. Added confugurable default storage option to chart

### How has this been tested?
<!-- Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc. -->
We test it on our AKS with RWX volume

### Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply.
If an item isn't applicable for some reason, then ~~explicitly
strikethrough~~ the whole
line. If you don't do that, GitHub will show incorrect progress for the
pull request.
If you're unsure about any of these, don't hesitate to ask. We're here
to help! -->
- [x] I submit my changes into the `develop` branch
- [x] I have added a description of my changes into the
[CHANGELOG](https://github.com/opencv/cvat/blob/develop/CHANGELOG.md)
file
- [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](

https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))
- [x] I have increased versions of npm packages if it is necessary

([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning),

[cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning),

[cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning)
and

[cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning))

### License

- [x] I submit _my code changes_ under the same [MIT License](
https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the
project.
  Feel free to contact the maintainers if that's a concern.
  
closes cvat-ai#6043 
closes cvat-ai#6096 
closes cvat-ai#5733

---------

Co-authored-by: Michael Kirpichev <[email protected]>
Co-authored-by: Nikita Manovich <[email protected]>
Co-authored-by: Andrey Zhavoronkov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant