test(e2e): bump sandbox tenant storage quota 100Gi -> 200Gi - #3206
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates an e2e bats test for tenant creation in isolated mode, increasing the tenant's requested storage quota from 100Gi to 200Gi and updating the corresponding ResourceQuota assertion, with added comments explaining the rationale. ChangesTenant Storage Quota Test Update
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested labels: `kind/bug` Suggested reviewers: `kvaps`, `lllamnyp` 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
The E2E sandbox tenant (tenant-test) provisions back-to-back tenant
Kubernetes control planes via kubernetes-latest.bats and
kubernetes-previous.bats. Each control plane brings up two worker VMs
whose 20Gi root disks are populated by CDI, and CDI's second-phase
importer allocates a scratch PVC of the same size before the qcow2
unpack. Peak per-tenant footprint is therefore ~82Gi during import
(2 × 20Gi target + 2 × 21Gi scratch), collapsing to 40Gi at rest.
kubernetes-latest teardown deletes the tenant Kubernetes CR and waits
via cozy_wait_tenant_drained for the KubeVirt VMs, VMIs and namespace
PVCs to disappear. The API-level PVC delete returns quickly, but the
LINSTOR CSI detach + DRBD resource release on the underlying nodes
lags past the 300s drain budget. During that window, the deleted
PVCs still count against ResourceQuota's requests.storage because
the storage-provisioner has not finished releasing the volume — so
kubernetes-previous starts its own CDI import with 40Gi of stale
kubernetes-latest quota still charged, and CDI's scratch PVC create
for the second worker (used=80Gi + scratch=21Gi = 101Gi) is
rejected by the tenant-quota admission check at 100Gi. The second
worker's DataVolume never leaves ImportInProgress, the tenant node
never joins, and the 12-minute node-join deadline in
run-kubernetes.sh trips kubernetes-previous.bats.
Raise the sandbox tenant-quota storage ceiling to 200Gi so the
overlap between one test's slow storage cleanup and the next test's
CDI import fits inside the quota. The failing-run diagnostic is
cozyreport artifact from run 28730757213:
cdi-deployment log:
scratch PVC API create errored: … forbidden: exceeded quota:
tenant-quota, requested: requests.storage=22763536384,
used: requests.storage=85899345920,
limited: requests.storage=100Gi
This is a test-infrastructure fix; the 100Gi default in the tenant
chart itself is unchanged. The assertion below the CR update is
lifted from 100Gi to 200Gi in lockstep.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
fe4ff21 to
3ab9f95
Compare
Summary of ChangesHello, 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 addresses a race condition in the E2E test suite where overlapping tenant lifecycles cause storage quota violations. By doubling the available storage for the test tenant, the system can now accommodate the transient peak usage during CDI imports when previous test resources have not yet been fully released by the storage provider. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request increases the storage resource quota from 100Gi to 200Gi in the E2E installation test suite (hack/e2e-install-cozystack.bats) to prevent resource quota issues during back-to-back tenant Kubernetes tests. The review feedback points out that the PR title uses an invalid scope (e2e) according to the repository style guide, and suggests using a valid scope such as tests or hack.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| cpu: "60" | ||
| memory: "128Gi" | ||
| storage: "100Gi" | ||
| # 200Gi so back-to-back tenant Kubernetes tests |
There was a problem hiding this comment.
The PR title uses the scope e2e, which is not listed as a valid scope in the repository style guide. Please consider updating the PR title and commit message to use a valid scope such as tests or hack to comply with the Conventional Commits requirements.
References
- Each commit must follow Conventional Commits format with valid scopes (e.g.,
tests,hack,ci,api,docs). (link)
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — minimal, test-only quota bump that unblocks a real back-to-back tenant-test flake, with production defaults untouched.
Business context: raise the E2E sandbox tenant's storage quota so the kubernetes-latest → kubernetes-previous back-to-back tests don't trip the tenant ResourceQuota during CDI import while the prior test's DRBD/LINSTOR detach is still releasing storage.
Non-blocking notes
- The bump absorbs a transient quota-accounting overlap (deleted PVCs still counted while LINSTOR/DRBD detach lags past
cozy_wait_tenant_drained) with headroom rather than closing it — fine for e2e stability, and the ~78Gi margin covers the documented worst-case overlap. If detach lag ever grows past that, the flake returns; a more durable (but more coupled) alternative would be to make the drain wait block untilquota.status.used.requests.storageactually decays. Not needed now. - Minor description nit: "The 100Gi default in
packages/apps/tenantis unchanged" — the app'sresourceQuotasdefault is actually{}(empty); the 100Gi lived only in this fixture. The intent (no product default touched) is correct. - The title scope
e2eis valid percontributing.mdand the labeler (maps toarea/testing); no rename needed.
What this PR does
Raises the E2E sandbox tenant's
resourceQuotas.storagefrom 100Gi to 200Gi so back-to-back tenant Kubernetes tests (kubernetes-latest.bats->kubernetes-previous.bats) do not collide with tenant quota during the second cluster's CDI import.The 100Gi default in
packages/apps/tenantis unchanged — this touches only the E2E test tenant CR inhack/e2e-install-cozystack.bats.Root cause
Both kubernetes-*.bats tests provision a 2-worker tenant control plane whose worker root disks are populated by CDI. CDI's second-phase importer allocates a scratch PVC of the same 20Gi as the target disk before the qcow2 unpack, so per-tenant peak footprint is:
collapsing to 40Gi at rest.
kubernetes-latest'scozy_cleanupdeletes the Kubernetes CR and blocks incozy_wait_tenant_drained(300s) waiting for KubeVirt VMs / VMIs / namespace PVCs to disappear. The API-level PVC delete returns quickly, but the LINSTOR CSI detach + DRBD resource release on the underlying nodes lags past that budget. During the overlap window the deleted PVCs still count againstResourceQuota.status.used.requests.storagebecause the storage provisioner has not finished releasing the volume.kubernetes-previousthen starts its own CDI import with 40Gi of stalekubernetes-latestquota still charged, and CDI's scratch PVC create for the second worker (used 80Gi + scratch 21Gi = 101Gi) is rejected by the tenant-quota admission check at 100Gi. The second worker's DataVolume stalls inImportInProgress, the tenant node never joins, and the 12-minute node-join deadline inrun-kubernetes.shfailskubernetes-previous.bats.Evidence
cozyreport.tgzfrom the failing E2E run of PR #3044 (run 28730757213):The tenant-quota
status.used.requests.storagein the snapshot decays back to 40Gi by end-of-run — confirming the leak is a transient window during the second test's start-up, not a permanent leak.Relation to prior work
Overlaps in scope with #2947 (
ci(e2e): bump tenant storage quota 100Gi -> 200Gi), which bundles this same bump alongside three other e2e stabilization changes (crust-gather tag, harbor BucketClaim timeout, cilium IP-conflict healer). #2947 has been stale since 2026-06-17 with the maintainer suggesting it be closed. This PR isolates just the quota-bump so it can land on its own scope; the other three changes are orthogonal and can ship separately if #2947 stays dormant.Release note
Summary by CodeRabbit