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

Support CVAT 2.X deployment using helm #4448

Merged
merged 5 commits into from
Mar 27, 2022
Merged

Conversation

se-wo
Copy link
Contributor

@se-wo se-wo commented Mar 12, 2022

Motivation and context

The current charts provided in the helm-charts directory are broken. The latest docker images require that Open Policy Agent is available. OPA was added to the docker-compose files but not to the helm charts. This pull requests fixes this problem.
Moreover the templates were updated to support newer kubernetes versions. APIs like networking.k8s.io/v1beta1 and extensions/v1beta1 no longer exist in kubernetes versions 1.22 onwards. This pull request also adds support for networking.k8s.io/v1. Further, I fixed a small mistake where frontend templates used backend labels instead of their own.
Multiple issues like #4438 and #4192 are looking for a solution to CVAT 2.X support.

How has this been tested?

The helm charts were tested locally with minikube v1.25.2 running k8s v1.23.3. Charts were deployed using Helm 3.8

Checklist

License

  • 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.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

Previously those were erronously taken from backend.
Added templates for OPA and extended readme on how to deploy.
Moreover modernized the ingress chart to support k8s version 1.22+.
@se-wo se-wo requested a review from nmanovic as a code owner March 12, 2022 12:59
@nmanovic nmanovic requested a review from azhavoro March 12, 2022 16:24
@nmanovic
Copy link
Contributor

@se-wo , thanks for the contribution!

Copy link
Contributor

@azhavoro azhavoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@se-wo thank you for the your contribution,
Generally I'm OK with changes, but could you please fix Markdown linter warning and add minor note about archive?
Also could you please add a note to the changelog, I think it would be helpful to the community.

helm-chart/README.md Outdated Show resolved Hide resolved
@se-wo
Copy link
Contributor Author

se-wo commented Mar 16, 2022

@se-wo thank you for the your contribution, Generally I'm OK with changes, but could you please fix Markdown linter warning and add minor note about archive? Also could you please add a note to the changelog, I think it would be helpful to the community.

Can you give me a hint on what the linter issue is or what linter style is used? I am only able to see that the linter Github action failed. I fixed everything the default VS Code linter reported to me before opening this pull request.

@se-wo
Copy link
Contributor Author

se-wo commented Mar 16, 2022

@azhavoro added a changelog entry. Updated the readme with information about where to create the rules.tar.gz. Also added the shell command that I used to create the archive.

@@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.2.0
version: 0.3.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should match chart version with CVAT's release version?

@randombenj
Copy link

Thanks for the effort! I deployed my chart from this branch, and it works like a charm 🎉

@azhavoro
Copy link
Contributor

@se-wo LGTM, could you please fix linter warning, I've attached the report
remark_report (1).zip

Fix linter error due to markdown line longer than 120 characters
@se-wo
Copy link
Contributor Author

se-wo commented Mar 25, 2022

@azhavoro Linter error should be fixed. Was only a single line that exceeded 120 characters. Have split it into multiple lines.

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@se-wo , Thanks for the contribution!

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 this pull request may close these issues.

5 participants