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

Update helm #7894

Merged
merged 5 commits into from
May 16, 2024
Merged

Update helm #7894

merged 5 commits into from
May 16, 2024

Conversation

azhavoro
Copy link
Contributor

@azhavoro azhavoro commented May 15, 2024

Added ability to specify ServiceAccount for backend pods
Removed passing of DJANGO_MODWSGI_EXTRA_ARGS env variable to server pod
Do not set database host and port env variables if they are empty

Motivation and context

How has this been tested?

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

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.

Summary by CodeRabbit

  • New Features

    • Introduced the ability to specify a ServiceAccount for backend pods in Helm.
  • Improvements

    • Updated Helm chart version to 0.13.0.
    • Enhanced logic for setting environment variables based on external PostgreSQL configurations.
  • Removals

    • Removed DJANGO_MODWSGI_EXTRA_ARGS environment variable setting.
  • Documentation

    • Added a changelog entry detailing the new ServiceAccount feature for backend pods.

Removed passing of DJANGO_MODWSGI_EXTRA_ARGS env cariable to server pod
Do not set database host and port env variables if they are empty
Copy link
Contributor

coderabbitai bot commented May 15, 2024

Warning

Rate Limit Exceeded

@azhavoro has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 27 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between a5426af and 69d7683.

Walkthrough

This update to the Helm chart primarily focuses on enhancing the configuration of backend pods by introducing the ability to specify a ServiceAccount. Additionally, it updates the version number to 0.13.0, refines PostgreSQL environment variable settings, and removes unnecessary environment variables. These changes improve security and flexibility in deploying backend services.

Changes

File Path Change Summary
helm-chart/Chart.yaml Updated version from 0.12.1 to 0.13.0.
helm-chart/templates/_helpers.tpl Renamed service account name definition and adjusted PostgreSQL environment variable logic.
helm-chart/templates/.../initializer/job.yml Added serviceAccountName field under the spec section.
helm-chart/templates/.../server/deployment.yml Added serviceAccountName field and removed DJANGO_MODWSGI_EXTRA_ARGS environment variable.
helm-chart/templates/.../utils/deployment.yml Added serviceAccountName field under the spec section.
helm-chart/values.yaml Added serviceAccount section, removed DJANGO_MODWSGI_EXTRA_ARGS, and modified external PostgreSQL settings.
changelog.d/20240515_144240_andrey_helm_update.md Documented the new functionality for specifying a ServiceAccount for backend pods.

In the garden of code, a rabbit hopped,
Through fields of YAML, it never stopped.
With a tweak here and a version bump there,
Deployments became light as air.
Service accounts now in place,
Backend pods run with grace.
Hoppy coding, everywhere! 🌸🐇


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@azhavoro azhavoro requested a review from nmanovic as a code owner May 15, 2024 11:45
@azhavoro azhavoro requested a review from SpecLad May 15, 2024 11:45
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between f4ee3f9 and 1a5c647.
Files selected for processing (12)
  • helm-chart/Chart.yaml (1 hunks)
  • helm-chart/templates/_helpers.tpl (2 hunks)
  • helm-chart/templates/cvat_backend/initializer/job.yml (1 hunks)
  • helm-chart/templates/cvat_backend/server/deployment.yml (2 hunks)
  • helm-chart/templates/cvat_backend/utils/deployment.yml (1 hunks)
  • helm-chart/templates/cvat_backend/worker_analyticsreports/deployment.yml (1 hunks)
  • helm-chart/templates/cvat_backend/worker_annotation/deployment.yml (1 hunks)
  • helm-chart/templates/cvat_backend/worker_export/deployment.yml (1 hunks)
  • helm-chart/templates/cvat_backend/worker_import/deployment.yml (1 hunks)
  • helm-chart/templates/cvat_backend/worker_qualityreports/deployment.yml (1 hunks)
  • helm-chart/templates/cvat_backend/worker_webhooks/deployment.yml (1 hunks)
  • helm-chart/values.yaml (3 hunks)
Files skipped from review due to trivial changes (1)
  • helm-chart/Chart.yaml
Additional comments not posted (13)
helm-chart/templates/cvat_backend/initializer/job.yml (1)

40-40: Addition of serviceAccountName field looks good and aligns with the objective to specify a ServiceAccount for backend pods.

helm-chart/templates/cvat_backend/worker_webhooks/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the objective to specify a ServiceAccount for backend pods.

helm-chart/templates/cvat_backend/worker_qualityreports/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the objective to specify a ServiceAccount for backend pods.

helm-chart/templates/cvat_backend/worker_analyticsreports/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the objective to specify a ServiceAccount for backend pods.

helm-chart/templates/_helpers.tpl (2)

54-57: Renaming the service account name definition to cvat.backend.serviceAccountName is clear and aligns with the updated structure.


97-104: The addition of conditional blocks for CVAT_POSTGRES_HOST and CVAT_POSTGRES_PORT environment variables ensures that they are only set if the external PostgreSQL host and port are provided. This is a good practice to avoid setting empty environment variables.

helm-chart/templates/cvat_backend/utils/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the objective to specify a ServiceAccount for backend pods.

helm-chart/templates/cvat_backend/server/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the PR objectives.

helm-chart/templates/cvat_backend/worker_export/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the PR objectives.

helm-chart/templates/cvat_backend/worker_import/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the PR objectives.

helm-chart/templates/cvat_backend/worker_annotation/deployment.yml (1)

48-48: Addition of serviceAccountName field looks good and aligns with the PR objectives.

helm-chart/values.yaml (2)

20-24: Addition of serviceAccount section looks good and aligns with the PR objectives.


282-285: Modification to allow empty values for host and port in the external section under postgresql looks good and aligns with the PR objectives.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 1a5c647 and 323ccc0.
Files selected for processing (1)
  • changelog.d/20240515_144240_andrey_helm_update.md (1 hunks)

Comment on lines 1 to 4
### Added <!-- pick one -->

- [Helm] Ability to specify ServiceAccount for backend pods
(<https://github.com/cvat-ai/cvat/pull/7894>)
Copy link
Contributor

Choose a reason for hiding this comment

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

Include all significant changes in the changelog entry.

The current changelog entry is missing information about the removal of the DJANGO_MODWSGI_EXTRA_ARGS environment variable and the conditional setting of database host and port environment variables. Consider updating the entry to reflect these changes for completeness.

### Added <!-- pick one -->

- [Helm] Ability to specify ServiceAccount for backend pods
  (<https://github.com/cvat-ai/cvat/pull/7894>)
- [Helm] Removed `DJANGO_MODWSGI_EXTRA_ARGS` environment variable from server pod
- [Helm] Conditional setting of database host and port environment variables

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
### Added <!-- pick one -->
- [Helm] Ability to specify ServiceAccount for backend pods
(<https://github.com/cvat-ai/cvat/pull/7894>)
### Added <!-- pick one -->
- [Helm] Ability to specify ServiceAccount for backend pods
(<https://github.com/cvat-ai/cvat/pull/7894>)
- [Helm] Removed `DJANGO_MODWSGI_EXTRA_ARGS` environment variable from server pod
- [Helm] Conditional setting of database host and port environment variables

helm-chart/templates/_helpers.tpl Outdated Show resolved Hide resolved
helm-chart/values.yaml Outdated Show resolved Hide resolved
azhavoro and others added 2 commits May 15, 2024 16:57
Co-authored-by: Roman Donchenko <roman@cvat.ai>
Co-authored-by: Roman Donchenko <roman@cvat.ai>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 323ccc0 and c649659.
Files selected for processing (1)
  • helm-chart/templates/_helpers.tpl (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • helm-chart/templates/_helpers.tpl

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between c649659 and a5426af.
Files selected for processing (1)
  • helm-chart/values.yaml (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • helm-chart/values.yaml

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.33%. Comparing base (f4ee3f9) to head (69d7683).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7894      +/-   ##
===========================================
- Coverage    83.33%   83.33%   -0.01%     
===========================================
  Files          380      380              
  Lines        40022    40022              
  Branches      3766     3766              
===========================================
- Hits         33353    33352       -1     
- Misses        6669     6670       +1     
Components Coverage Δ
cvat-ui 79.19% <ø> (ø)
cvat-server 87.14% <ø> (-0.01%) ⬇️

@SpecLad SpecLad merged commit 49ef2a3 into develop May 16, 2024
32 checks passed
@SpecLad SpecLad deleted the az/helm_update branch May 16, 2024 09:26
@cvat-bot cvat-bot bot mentioned this pull request May 21, 2024
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.

None yet

3 participants