CNTRLPLANE-3609: update TestEtcdDBScaling to also include the validation check for BackendQuotaGiB#1656
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
WalkthroughThe database scaling end-to-end test now verifies that backend quota values below and above the allowed range are rejected before continuing with the existing valid quota update and pod environment checks. ChangesBackend quota validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/scaling_dbsize.go`:
- Around line 42-56: Update the scaling test’s Kubernetes API calls around the
Etcd validation and valid-update flow to create one timeout context and reuse it
for every Get, Update, and List operation, including pod-listing calls. Replace
each context.Background() passed to these client methods while preserving the
existing assertions and test behavior.
- Around line 45-52: Update the invalid Etcd update assertions in the scaling
test to verify apierrors.IsInvalid(err) for both validation failures, rather
than only requiring a generic error. For the BackendQuotaGiB case, also inspect
the invalid-field cause and confirm it identifies the BackendQuotaGiB field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5f615bc8-8a61-452e-a762-ab714e3bed61
📒 Files selected for processing (1)
test/e2e/scaling_dbsize.go
| etcdCR, err := opClientSet.OperatorV1().Etcds().Get(context.Background(), etcdCRName, metav1.GetOptions{}) | ||
| require.NoError(t, err) | ||
| etcdCR.Spec.BackendQuotaGiB = invalidLowDBSizeGB | ||
| etcdCR, err = opClientSet.OperatorV1().Etcds().Update(context.Background(), etcdCR, metav1.UpdateOptions{}) | ||
| require.Error(t, err) | ||
|
|
||
| etcdCR, err = opClientSet.OperatorV1().Etcds().Get(context.Background(), etcdCRName, metav1.GetOptions{}) | ||
| require.NoError(t, err) | ||
| etcdCR.Spec.BackendQuotaGiB = invalidHighDBSizeGB | ||
| etcdCR, err = opClientSet.OperatorV1().Etcds().Update(context.Background(), etcdCR, metav1.UpdateOptions{}) | ||
| require.Error(t, err) | ||
|
|
||
| // valid update | ||
| etcdCR, err = opClientSet.OperatorV1().Etcds().Get(context.Background(), etcdCRName, metav1.GetOptions{}) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant file and surrounding lines
sed -n '1,160p' test/e2e/scaling_dbsize.goRepository: openshift/cluster-etcd-operator
Length of output: 3021
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map functions in the file for context
ast-grep outline test/e2e/scaling_dbsize.go --view expandedRepository: openshift/cluster-etcd-operator
Length of output: 573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all uses of context.Background in the e2e test file
rg -n "context\.Background\(\)|WithTimeout|WithCancel" test/e2e/scaling_dbsize.goRepository: openshift/cluster-etcd-operator
Length of output: 953
Bound all Kubernetes API calls in this test. The Get/Update/List calls use context.Background(), so a stalled API server can hang the e2e job until the outer runner kills it. Create one timeout context and reuse it for the validation, update, and pod-listing calls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e/scaling_dbsize.go` around lines 42 - 56, Update the scaling test’s
Kubernetes API calls around the Etcd validation and valid-update flow to create
one timeout context and reuse it for every Get, Update, and List operation,
including pod-listing calls. Replace each context.Background() passed to these
client methods while preserving the existing assertions and test behavior.
Source: Path instructions
| etcdCR, err = opClientSet.OperatorV1().Etcds().Update(context.Background(), etcdCR, metav1.UpdateOptions{}) | ||
| require.Error(t, err) | ||
|
|
||
| etcdCR, err = opClientSet.OperatorV1().Etcds().Get(context.Background(), etcdCRName, metav1.GetOptions{}) | ||
| require.NoError(t, err) | ||
| etcdCR.Spec.BackendQuotaGiB = invalidHighDBSizeGB | ||
| etcdCR, err = opClientSet.OperatorV1().Etcds().Update(context.Background(), etcdCR, metav1.UpdateOptions{}) | ||
| require.Error(t, err) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant test file with line numbers
sed -n '1,140p' test/e2e/scaling_dbsize.go | cat -n
# Find other uses of the same assertion style in nearby tests
rg -n "require\.(Error|True\(t, apierrors\.IsInvalid|NoError)" test/e2e -g '*.go'Repository: openshift/cluster-etcd-operator
Length of output: 7216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate validation for BackendQuotaGiB / Etcd CRD schema references
rg -n "BackendQuotaGiB|Etcds\(\)\.Update|validation.*BackendQuota|Invalid" \
test pkg config install manifests . -g '*.go' -g '*.yaml' -g '*.yml' -g '*.json'
# Map the API type/validation implementation files
fd -a '.*etcd.*\.go|.*validation.*\.go|.*types.*\.go' pkg api test | head -n 50Repository: openshift/cluster-etcd-operator
Length of output: 50388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the API type and any validation logic around BackendQuotaGiB
rg -n "BackendQuotaGiB|Validate.*Etcd|validation.*Etcd|+kubebuilder|openapi|minimum|max" \
pkg api test config install manifests . -g '*.go' -g '*.yaml' -g '*.yml' -g '*.json'
# Show the Etcd API type if present
fd -a 'etcd.*\.go' pkg api | sed -n '1,20p'Repository: openshift/cluster-etcd-operator
Length of output: 384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate repo-owned files that mention the field name
git ls-files | rg '(^|/)(api|pkg|config|manifests|test)/.*\.(go|ya?ml|json)$' | xargs rg -n -F "BackendQuotaGiB"
# Find the Etcd API type declaration with a small, focused search
git ls-files | rg '\.go$' | xargs rg -n -F "type EtcdSpec" -A 40 -B 10Repository: openshift/cluster-etcd-operator
Length of output: 1186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact file(s) containing the field and nearby context
files=$(git ls-files | rg '(^|/)(api|pkg)/.*\.go$' | xargs rg -l -F "BackendQuotaGiB" || true)
printf '%s\n' "$files" | sed '/^$/d'
for f in $files; do
echo "### $f"
sed -n '1,220p' "$f" | rg -n -C 4 "BackendQuotaGiB|type EtcdSpec|Validate|kubebuilder|OpenAPI|minimum|max"
doneRepository: openshift/cluster-etcd-operator
Length of output: 2854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the field in repo-owned Go files only, excluding vendor
rg -n -F "BackendQuotaGiB" -g '!vendor/**' -g '*.go' .
# Also search for generated schema markers around the field
rg -n -F "kubebuilder:validation" -g '!vendor/**' -g '*.go' .Repository: openshift/cluster-etcd-operator
Length of output: 956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the repo-owned definition(s) for BackendQuotaGiB and show tight context
rg -n -F "BackendQuotaGiB" -g '!vendor/**' -g '*.go' .
# Find the EtcdSpec type if present in non-vendor Go files
rg -n -F "type EtcdSpec" -g '!vendor/**' -g '*.go' . -A 60 -B 10Repository: openshift/cluster-etcd-operator
Length of output: 956
Assert the expected validation error here. BackendQuotaGiB is schema-limited to 8–16, so these updates should fail with apierrors.IsInvalid(err) rather than any generic error. Checking the BackendQuotaGiB field cause would make the test even tighter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e/scaling_dbsize.go` around lines 45 - 52, Update the invalid Etcd
update assertions in the scaling test to verify apierrors.IsInvalid(err) for
both validation failures, rather than only requiring a generic error. For the
BackendQuotaGiB case, also inspect the invalid-field cause and confirm it
identifies the BackendQuotaGiB field.
atiratree
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@atiratree: This pull request references CNTRLPLANE-3609 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/pipeline required |
|
Scheduling required tests: |
|
@atiratree: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
…
Summary by CodeRabbit