Repository navigation
add resource quota for testing ns - #1382
Conversation
Signed-off-by: IvanHunters <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdated the e2e install script to patch the root Tenant enabling host/ingress/monitoring/seaweedfs and to replace an empty resourceQuotas map with explicit cpu/memory/storage quotas for a test tenant; adjusted cozy-lib template resource emission to drop limits for ephemeral-storage and other non-CPU/memory resources and to return a raw mapping (no top-level resourceQuotas key). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant BATS as e2e-install-cozystack.bats
participant KCTL as kubectl
participant API as Kubernetes API
participant Tenant as Tenant CR
Dev->>BATS: run e2e script
BATS->>KCTL: patch root Tenant spec (enable host/ingress/monitoring/seaweedfs)
KCTL->>API: PATCH /apis/.../tenants/root
API-->>KCTL: 200 OK
BATS->>KCTL: patch test Tenant resourceQuotas (cpu/memory/storage)
KCTL->>API: PATCH /apis/.../tenants/test
API-->>KCTL: 200 OK
BATS->>API: wait for applications ready
API-->>BATS: Ready status
BATS-->>Dev: e2e complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello IvanHunters, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the end-to-end testing environment by implementing resource quotas for the dedicated testing namespace. This change aims to simulate a more constrained environment for tests, ensuring that applications within the test tenant adhere to predefined resource limits for CPU, memory, and storage.
Highlights
- Resource Quota Configuration: This pull request introduces specific resource quotas (CPU, memory, and storage) for the
tenant-testnamespace within the e2e installation script. This ensures that the testing environment operates with defined resource limits. - E2E Test Script Update: The
hack/e2e-install-cozystack.batsscript has been updated to include these resource quota definitions, which are applied when configuring thetenant-testtenant.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | Gemini (@gemini-code-assist) | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds resource quotas for the testing namespace in the e2e tests, which is a valuable addition for ensuring resource constraints are tested. The implementation is straightforward, but there is a minor style issue with YAML indentation that should be addressed for better code readability and maintainability.
| resourceQuotas: | ||
| cpu: "4" | ||
| memory: "4Gi" | ||
| storage: "100Gi" |
There was a problem hiding this comment.
The indentation for the resourceQuotas properties is unconventional. While a single space indentation might be parsed correctly by kubectl, it's against common YAML style guides which recommend two-space indentation for readability and consistency. This makes the file harder to read and maintain. Please use a two-space indent for the properties under resourceQuotas.
resourceQuotas:
cpu: "4"
memory: "4Gi"
storage: "100Gi"
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
hack/e2e-install-cozystack.bats (2)
126-131: Root-tenant patch is fine; prefer fully qualified resource for clarityUsing the group-qualified resource avoids ambiguity if another CRD named “tenants” exists.
- kubectl patch tenants/root -n tenant-root --type merge -p '{"spec":{"host":"example.org","ingress":true,"monitoring":true,"etcd":true,"isolated":true, "seaweedfs": true}}' + kubectl patch tenants.apps.cozystack.io/root -n tenant-root --type merge -p '{"spec":{"host":"example.org","ingress":true,"monitoring":true,"etcd":true,"isolated":true,"seaweedfs":true}}'
192-195: Normalize YAML indentation under resourceQuotasChildren of resourceQuotas are indented by 1 space; the file otherwise uses 2-space steps. Standardize for readability.
resourceQuotas: - cpu: "4" - memory: "4Gi" - storage: "100Gi" + cpu: "4" + memory: "4Gi" + storage: "100Gi"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
hack/e2e-install-cozystack.bats(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
hack/e2e-install-cozystack.bats (1)
192-200: Verify Tenant CRD schema and resource quotas
- Manually confirm the Tenant CRD (
tenants.apps.cozystack.io, version v1alpha1) definesspec.resourceQuotaswithcpu,memory, andstorageunder its OpenAPI schema.- Apply the e2e install and ensure a ResourceQuota in the
tenant-testnamespace is created with hard limits set tocpu: "4",memory: "4Gi", andstorage: "100Gi".
Signed-off-by: IvanHunters <[email protected]>
Signed-off-by: IvanHunters <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/library/cozy-lib/templates/_resources.tpl (2)
49-53: Bug: unit parsing uses root context ($) instead of the input value ($value).This breaks suffix detection (and can cause type errors) for quantities like "256Mi" or "500m".
Apply this fix:
- {{- if hasSuffix $k $ -}} - {{- $value = trimSuffix $k $ -}} + {{- if hasSuffix $k $value -}} + {{- $value = trimSuffix $k $value -}} {{- $unit = $v -}} {{- end -}}
107-113: Add missing limits assignments for ephemeral-storage and other resources
In thecozy-lib.resources.sanitizetemplate, theephemeral-storageand default branches set onlyrequests, leavinglimitsunset (lines 107–113). This diverges from the documented contract—non-CPU/memory resources must emit identicallimitsandrequests. Restore symmetry by adding$_ := set $output.limitsin both branches:else if eq $k "ephemeral-storage" }} {{- $_ := set $output.requests $k ($ephemeralStorageRequestF64 | int | toString) }} + {{- $_ := set $output.limits $k ($v | toString) }} else }} - {{- $_ := set $output.requests $k $v }} + {{- $_ := set $output.requests $k ($v | toString) }} + {{- $_ := set $output.limits $k ($v | toString) }} end }}
🧹 Nitpick comments (1)
packages/library/cozy-lib/templates/_resources.tpl (1)
58-86: Update the sanitize() docblock to match behavior.The comment says “All resources are returned with equal requests and limits, except cpu…”, but the implementation currently omits limits for ephemeral-storage and others.
Propose clarifying the contract (either restore limits per previous comment or document the new requests-only behavior with examples).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/library/cozy-lib/templates/_resources.tpl(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
| {{- $out | toYaml }} | ||
| {{- end }} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Breaking change: flatten() output shape dropped the resourceQuotas wrapper.
This will break any consumers expecting keys under resourceQuotas.*.
-
Please confirm all call sites/tests were updated. Run:
-
Consider a short deprecation period (feature flag or alias helper) and add a release note highlighting the YAML shape change.
🏁 Script executed:
#!/bin/bash
# Find usages relying on the old wrapper and flatten invocations
rg -nP -C2 'resourceQuotas\.'
rg -nP -C2 'include\s+"cozy-lib\.resources\.flatten"'Length of output: 1778
Breaking change: flatten() now drops the resourceQuotas wrapper
The only consumer of this helper is in packages/apps/tenant/templates/quota.yaml:9. After this change it will emit the quota fields unwrapped (no top-level resourceQuotas:), so that template will break. Update it to re-introduce the wrapper (or adjust its structure), and verify any tests for cozy-lib.resources.flatten have been updated. Add a release note for the YAML shape change and consider a deprecation alias or feature flag for backward compatibility.
🤖 Prompt for AI Agents
In packages/library/cozy-lib/templates/_resources.tpl around lines 183 to 184,
flatten() has been changed to emit quota fields without the top-level
resourceQuotas: wrapper which will break
packages/apps/tenant/templates/quota.yaml (line 9) and tests. Update
packages/apps/tenant/templates/quota.yaml to re-introduce the wrapper by
wrapping the flattened output under resourceQuotas: (or adjust the template
structure to match the new flat shape), update or add unit tests for
cozy-lib.resources.flatten to reflect the new output shape (or add tests for a
deprecation alias that preserves the old wrapped output), add a release note
describing the YAML shape change, and optionally implement a short-term
deprecation alias or feature flag in the library to emit the old wrapped
structure for backward compatibility.
Signed-off-by: IvanHunters <[email protected]>
Andrei Kvapil (kvaps)
added
the
kind/backport
|
Successfully created backport PR for |
# Description Backport of #1382 to `release-0.35`.
2 participants
What this PR does
Release note
Summary by CodeRabbit
Tests
Refactor