Add e2e tests for gdrive - #306
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe changes enable Google Drive e2e execution with LocalStack S3. They configure service-account secrets and pipeline stages, permit e2e operation without LDAP, and validate crawled files, permissions, converted documents, chunks, and destination embeddings. ChangesGoogle Drive pipeline end-to-end coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/e2e/gdrive_test.go (2)
505-570: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExtend garbage-collection assertions to metadata and permission sidecars.
The GC assessment verifies that
file2ID's primary object is removed (Lines 554-557) and that 2 data files remain (Lines 561-568), but it does not check whether the metadata sidecar (file2ID.json) or the permission file (permissions/file2ID.json) for the deleted source file are also removed. If production GC only deletes the primary object, an orphaned permission sidecar containing resolved UIDs/emails would remain in S3 indefinitely, undetected by this test.🔧 Proposed additional assertions
for _, obj := range output.Contents { key := aws.ToString(obj.Key) assert.NotContains(t, key, file2ID, "data.csv should be garbage collected from S3") + assert.NotContains(t, key, outputDir+"/permissions/"+file2ID+".json", + "data.csv permission sidecar should be garbage collected from S3") }🤖 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/gdrive_test.go` around lines 505 - 570, Extend the garbage-collection checks in the “garbage collection removes files deleted from source” assessment to assert that both the metadata sidecar for file2ID and the permissions/file2ID.json sidecar are absent from the listed S3 objects. Keep the existing primary-object and remaining-data-file assertions unchanged, and identify each sidecar by its expected key pattern.
234-264: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHarden the S3 bucket lifecycle against leftover state.
gdriveDataStorageBucketuses a fixed name, andCreateBucket(Line 260) usesrequire.NoError, which fails the test outright if the bucket already exists from a prior run whose teardown did not complete. The Teardown (Lines 572-590) ignores all errors fromDeleteObjectandDeleteBucketwith_, _ =, so a failed cleanup leaves the bucket present for the next run without surfacing a warning.Combine a failed teardown with a fixed bucket name, and the next test run fails at setup with a confusing "bucket already exists" error instead of a clear signal about the actual regression.
🔧 Proposed fix: tolerate a pre-existing bucket
_, err = s3Client.CreateBucket(ctx, &s3.CreateBucketInput{ Bucket: aws.String(gdriveDataStorageBucket), }) - require.NoError(t, err) + if err != nil { + var alreadyOwned *types.BucketAlreadyOwnedByYou + var alreadyExists *types.BucketAlreadyExists + require.True(t, errors.As(err, &alreadyOwned) || errors.As(err, &alreadyExists), "unexpected error creating bucket: %v", err) + t.Logf("bucket %s already exists, reusing", gdriveDataStorageBucket) + }Also applies to: 572-590
🤖 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/gdrive_test.go` around lines 234 - 264, Update the bucket setup around gdriveDataStorageBucket and CreateBucket to tolerate an already-existing bucket while still failing on other S3 errors. Preserve the existing bucket initialization for new runs, and surface unexpected creation failures clearly; do not treat all errors as successful setup.
🤖 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.
Nitpick comments:
In `@test/e2e/gdrive_test.go`:
- Around line 505-570: Extend the garbage-collection checks in the “garbage
collection removes files deleted from source” assessment to assert that both the
metadata sidecar for file2ID and the permissions/file2ID.json sidecar are absent
from the listed S3 objects. Keep the existing primary-object and
remaining-data-file assertions unchanged, and identify each sidecar by its
expected key pattern.
- Around line 234-264: Update the bucket setup around gdriveDataStorageBucket
and CreateBucket to tolerate an already-existing bucket while still failing on
other S3 errors. Preserve the existing bucket initialization for new runs, and
surface unexpected creation failures clearly; do not treat all errors as
successful setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Enterprise
Run ID: 116ec05f-b26f-4f50-9979-ab4ba63f0cd6
📒 Files selected for processing (1)
test/e2e/gdrive_test.go
46da3ac to
22f7c70
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/gdrive_test.go`:
- Around line 321-339: Update the cleanup flow after deleting the pipeline to
wait for the pipeline and its child resources to be fully absent before touching
the output bucket, using foreground deletion or an equivalent wait. In the
bucket cleanup block, handle and surface errors from ListObjectsV2, each
DeleteObject, and DeleteBucket instead of discarding them, while preserving the
existing object-removal sequence.
- Around line 110-114: Update the error handling in the GDrive pipeline
readiness check using WaitForResourceReady to call t.Fatal(err) instead of
t.Error(err), ensuring the setup stops immediately when the pipeline is not
ready.
In `@test/e2e/main_test.go`:
- Around line 156-163: Update the scale-down and scale-up command handling in
testSetup to return p.Err() immediately when either utils.RunCommandContext
invocation fails, instead of only logging and continuing. Preserve the existing
scale command order so setup stops before proceeding after an unsuccessful
operator restart.
🪄 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: defaults
Review profile: CHILL
Plan: Enterprise
Run ID: 76e5dcce-14d2-4001-80db-b1a7bb507d34
📒 Files selected for processing (7)
internal/controller/controllerconfig_controller.gointernal/controller/sourcecrawler_controller.gopkg/gdrive/permissions.gotest/e2e/gdrive_test.gotest/e2e/main_test.gotest/resources/unstructured/unstructured-secret.yamltest/utils/utils_function.go
| if err := operatorUtils.WaitForResourceReady(ctx, v1alpha1.UnstructuredDataPipelineCondition, | ||
| "unstructureddatapipelines.operator.dataverse.redhat.com", pipelineName, testNamespace); err != nil { | ||
| t.Error(err) | ||
| } | ||
| t.Log("GDrive pipeline CR is healthy") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="test/e2e/gdrive_test.go"
printf '%s\n' '--- candidate file outline ---'
ast-grep outline "$file" --lang go || true
printf '%s\n' '--- relevant test section ---'
sed -n '70,145p' "$file"
printf '%s\n' '--- readiness helper usages in this test ---'
rg -n -C 4 'WaitForResourceReady|t\.(Error|Fatal)\(' "$file"Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 4624
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- complete test structure ---'
sed -n '1,230p' test/e2e/gdrive_test.go
printf '%s\n' '--- feature setup and assessment definitions ---'
rg -n -C 5 'feature\.Setup|feature\.Assess|type Feature|func .*Setup|Assess\(' test internal . \
-g '*.go' \
-g '!vendor' | head -n 300Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 31641
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- e2e-framework dependency version ---'
rg -n 'sigs\.k8s\.io/e2e-framework' go.mod go.sum
printf '%s\n' '--- local dependency source, if present ---'
find "${GOMODCACHE:-/nonexistent}" -path '*sigs.k8s.io*e2e-framework*' -type f \
\( -name '*.go' -o -name 'README*' \) -print 2>/dev/null | head -n 40
printf '%s\n' '--- test runner invocation ---'
sed -n '1,145p' test/e2e/main_test.go
rg -n -C 4 'feature\.Run|features\.New|Run\(' test/e2e -g '*.go'Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 6338
🌐 Web query:
sigs.k8s.io/e2e-framework v0.7.0 features.Setup Assess implementation setup failure t.Error
💡 Result:
In the sigs.k8s.io/e2e-framework, the Assess method is used within a feature definition to perform specific test assertions against the environment [1][2]. The function signature for an assessment is: func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context Within this assessment function, you use the standard Go *testing.T object to handle test failures [1][3][2]. To signal a failure within an Assess block, you should call methods on the *testing.T object, such as: 1. t.Error: Reports an error but allows the execution of the current assessment (and subsequent assessments in the feature) to continue [2]. 2. t.Fatal: Reports an error and stops the execution of the current test/assessment immediately [1][4]. 3. t.Fail: Marks the test as failed but continues execution. 4. t.FailNow: Marks the test as failed and stops execution immediately [3]. Using t.Fatal or t.FailNow is often preferred if the failure prevents subsequent steps or assessments from running correctly [3]. When using the --fail-fast flag with the e2e-framework, invoking t.Fail or t.FailNow ensures the framework properly stops execution of the remaining assessments for that feature and skips subsequent features [3]. Example implementation: f:= features.New("example feature"). Assess("check condition", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { if conditionNotMet { t.Error("condition not met") // Continues execution // OR t.Fatal("critical failure") // Stops execution } return ctx }).Feature
Citations:
- 1: https://pkg.go.dev/sigs.k8s.io/e2e-framework@v0.7.0
- 2: https://medium.com/programming-kubernetes/end-to-end-testing-of-kubernetes-resources-with-the-e2e-framework-ac52e7e58db8
- 3: https://github.com/kubernetes-sigs/e2e-framework/blob/main/examples/fail_fast/README.md
- 4: https://github.com/kubernetes-sigs/e2e-framework/blob/main/examples/real_cluster/README.md
🏁 Script executed:
#!/bin/bash
set -eu
url='https://raw.githubusercontent.com/kubernetes-sigs/e2e-framework/v0.7.0/pkg/features/feature.go'
curl -fsSL "$url" | sed -n '1,260p'Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 2927
🏁 Script executed:
#!/bin/bash
set -eu
for path in pkg/features/builder.go pkg/features/feature_builder.go pkg/types/feature.go pkg/types/step.go pkg/env/run.go pkg/env/environment.go; do
url="https://raw.githubusercontent.com/kubernetes-sigs/e2e-framework/v0.7.0/$path"
body="$(curl -fsSL "$url" 2>/dev/null || true)"
if [ -n "$body" ]; then
printf '\n--- %s ---\n' "$path"
printf '%s\n' "$body" | rg -n -C 8 'func .*Setup|func .*Assess|LevelSetup|LevelAssess|Steps\(\)|FailNow|Run' || true
fi
doneRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 1928
🏁 Script executed:
#!/bin/bash
set -eu
curl -fsSL 'https://api.github.com/repos/kubernetes-sigs/e2e-framework/git/trees/v0.7.0?recursive=1' |
jq -r '.tree[].path' |
rg '(^|/)(run|runner|environment|feature|step).*\.go$|pkg/types'Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 234
🌐 Web query:
site:github.com/kubernetes-sigs/e2e-framework/blob/v0.7.0/pkg "GetStepsByLevel" "LevelSetup"
💡 Result:
Found in pkg/features/feature.go for v0.7.0:
LevelSetupis defined as an alias oftypes.LevelSetup.GetStepsByLevelfilters steps whoseLevel()matches the requested level.- Relevant lines: 542–545 and 653–673. (github.com)
func GetStepsByLevel(steps []types.Step, l types.Level) []types.Step {
if steps == nil {
return nil
}
var result []Step
for _, s := range steps {
if s.Level() == l {
result = append(result, s)
}
}
return result
}Citations:
🌐 Web query:
kubernetes-sigs e2e-framework v0.7.0 Feature Steps LevelSetup LevelAssess testenv.Run source
💡 Result:
The kubernetes-sigs/e2e-framework (v0.7.0) manages test execution via a defined set of steps within features [1][2]. In the e2e-framework architecture, steps are defined as having a Level, which determines their execution phase [1]. These levels are: - Setup: Actions to prepare the environment or feature [1]. - Assessment: Actions to verify or assert on the system state [1]. - Teardown: Actions to clean up resources [1]. The features package allows builders to add these steps using methods like Setup(...), Assess(...), and Teardown(...) [1][3]. These methods accept a function that typically follows the signature func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context [4][5][3]. Regarding testenv.Run(m), this function is the standard entry point used within a Go TestMain(m *testing.M) function to execute the defined testing environment, including setup and teardown tasks configured at the environment level, and to trigger the execution of tests in the package [4][5][6]. It returns an integer status code that is typically passed to os.Exit() [4][3][6]. Note: The terms "LevelSetup" and "LevelAssess" as paired phrases are not native to the kubernetes-sigs/e2e-framework API. They appear to be confused with components from the "Level CI" accessibility testing tools (e.g., @level-ci/a11y-playwright), which utilize functions named levelSetup and levelAnalyze [7][8][9].
Citations:
- 1: https://github.com/kubernetes-sigs/e2e-framework/blob/main/docs/design/README.md
- 2: https://pkg.go.dev/sigs.k8s.io/e2e-framework@v0.7.0/pkg/features
- 3: https://github.com/kubernetes-sigs/e2e-framework/blob/main/examples/real_cluster/README.md
- 4: https://pkg.go.dev/sigs.k8s.io/e2e-framework@v0.7.0
- 5: https://github.com/kubernetes-sigs/e2e-framework/blob/main/README.md
- 6: https://github.com/kubernetes-sigs/e2e-framework/
- 7: https://docs.ci.levelaccess.net/e2e-test-frameworks/playwright/api
- 8: https://docs.ci.levelaccess.net/e2e-test-frameworks/playwright/getting-started
- 9: https://docs.ci.levelaccess.net/e2e-test-frameworks/cypress/api
🏁 Script executed:
#!/bin/bash
set -eu
for path in \
pkg/env/env.go \
pkg/env/environment.go \
pkg/env/runner.go \
pkg/env/run.go \
pkg/env/funcs.go \
pkg/types/types.go \
pkg/types/feature.go; do
url="https://raw.githubusercontent.com/kubernetes-sigs/e2e-framework/v0.7.0/$path"
if body="$(curl -fsSL "$url" 2>/dev/null)"; then
printf '\n--- %s ---\n' "$path"
printf '%s\n' "$body" | rg -n -C 12 'func .*Run|GetStepsByLevel|LevelSetup|LevelAssess|Step\(' || true
fi
doneRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 5957
🏁 Script executed:
#!/bin/bash
set -eu
url='https://raw.githubusercontent.com/kubernetes-sigs/e2e-framework/v0.7.0/pkg/env/env.go'
curl -fsSL "$url" | sed -n '430,550p'Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 4013
Stop after pipeline readiness failure.
If WaitForResourceReady fails, t.Error(err) records the failure and the setup callback continues. The feature then runs later assessments against an unhealthy pipeline. Replace t.Error(err) with t.Fatal(err).
🤖 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/gdrive_test.go` around lines 110 - 114, Update the error handling in
the GDrive pipeline readiness check using WaitForResourceReady to call
t.Fatal(err) instead of t.Error(err), ensuring the setup stops immediately when
the pipeline is not ready.
| if err := kubeClient.Resources(testNamespace).Delete(ctx, pipeline); err != nil { | ||
| t.Logf("failed to delete GDrive pipeline: %v", err) | ||
| } | ||
|
|
||
| // cleanup output bucket | ||
| output, _ := s3Client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{ | ||
| Bucket: aws.String(outputBucketName), | ||
| }) | ||
| if output != nil { | ||
| for _, obj := range output.Contents { | ||
| _, _ = s3Client.DeleteObject(ctx, &s3.DeleteObjectInput{ | ||
| Bucket: aws.String(outputBucketName), | ||
| Key: obj.Key, | ||
| }) | ||
| } | ||
| } | ||
| _, _ = s3Client.DeleteBucket(ctx, &s3.DeleteBucketInput{ | ||
| Bucket: aws.String(outputBucketName), | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file outline ---'
ast-grep outline test/e2e/gdrive_test.go
printf '%s\n' '--- target section ---'
sed -n '260,365p' test/e2e/gdrive_test.go
printf '%s\n' '--- cleanup and pipeline deletion references ---'
rg -n -C 4 'Delete\(ctx, pipeline\)|outputBucketName|Wait|pipeline deletion|DeleteBucket|ListObjectsV2' test/e2eRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 25124
🏁 Script executed:
#!/bin/bash
set -e
sed -n '260,365p' test/e2e/gdrive_test.go
rg -n -C 4 'Delete\(ctx, pipeline\)|outputBucketName|Wait|pipeline deletion|DeleteBucket|ListObjectsV2' test/e2eRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 24956
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- pipeline and stage deletion logic ---'
rg -n -C 5 'DeletionTimestamp|Finalizer|finaliz|Delete.*Stage|UnstructuredDataPipeline|GDrive' controllers api internal pkg test -g '*.go' -g '*.yaml' 2>/dev/null | head -n 500
printf '%s\n' '--- client and wait helper definitions/usages ---'
rg -n -C 4 'func .*WaitForResourceReady|WaitForResourceReady|type Client interface|func .* Delete\(' . -g '*.go' | head -n 500
printf '%s\n' '--- test framework teardown semantics ---'
rg -n -C 5 'func \(.*Teardown|Teardown\(|Feature\(\).*Teardown|type .*Teardown' . -g '*.go' | head -n 300Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 50403
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C 5 'DeletionTimestamp|Finalizer|finaliz|Delete.*Stage|UnstructuredDataPipeline|GDrive' controllers api internal pkg test -g '*.go' -g '*.yaml' 2>/dev/null | head -n 500
rg -n -C 4 'func .*WaitForResourceReady|WaitForResourceReady|type Client interface|func .* Delete\(' . -g '*.go' | head -n 500
rg -n -C 5 'func \(.*Teardown|Teardown\(|Feature\(\).*Teardown|type .*Teardown' . -g '*.go' | head -n 300Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 50403
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- pipeline reconciliation and orphan cleanup ---'
sed -n '70,225p' internal/controller/unstructureddatapipeline_controller.go
printf '%s\n' '--- child resource creation and ownership ---'
sed -n '224,356p' internal/controller/unstructureddatapipeline_controller.go
printf '%s\n' '--- child controller reconcile entry points ---'
for f in internal/controller/sourcecrawler_controller.go internal/controller/documentprocessor_controller.go internal/controller/chunksgenerator_controller.go internal/controller/vectorembeddingsgenerator_controller.go internal/controller/destinationsyncer_controller.go; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
sed -n '55,120p' "$f"
fi
done
printf '%s\n' '--- teardown implementation and dependency versions ---'
sed -n '225,255p' test/utils/utils_function.go
rg -n 'sigs.k8s.io/controller-runtime|sigs.k8s.io/e2e-framework' go.mod go.sum
printf '%s\n' '--- precise deletion/finalizer matches in controllers ---'
rg -n 'DeletionTimestamp|Finalizer|SetControllerReference|OwnerReferences|Delete\(ctx' internal/controller -g '*.go'Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 29534
🏁 Script executed:
#!/bin/bash
set -e
sed -n '70,225p' internal/controller/unstructureddatapipeline_controller.go
sed -n '224,356p' internal/controller/unstructureddatapipeline_controller.go
for f in internal/controller/sourcecrawler_controller.go internal/controller/documentprocessor_controller.go internal/controller/chunksgenerator_controller.go internal/controller/vectorembeddingsgenerator_controller.go internal/controller/destinationsyncer_controller.go; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
sed -n '55,120p' "$f"
fi
done
sed -n '225,255p' test/utils/utils_function.go
rg -n 'sigs.k8s.io/controller-runtime|sigs.k8s.io/e2e-framework' go.mod go.sum
rg -n 'DeletionTimestamp|Finalizer|SetControllerReference|OwnerReferences|Delete\(ctx' internal/controller -g '*.go'Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 29275
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- destination writes and completion status ---'
rg -n -C 6 'S3Destination|PutObject|Upload|Sync|UpdateStatus|StatusPatch' internal/controller/destinationsyncer_controller.go pkg -g '*.go' | head -n 300
printf '%s\n' '--- read-only structural verifier ---'
python3 - <<'PY'
from pathlib import Path
pipeline = Path("internal/controller/unstructureddatapipeline_controller.go").read_text()
gdrive = Path("test/e2e/gdrive_test.go").read_text()
children = [
Path("internal/controller/sourcecrawler_controller.go"),
Path("internal/controller/documentprocessor_controller.go"),
Path("internal/controller/chunksgenerator_controller.go"),
Path("internal/controller/vectorembeddingsgenerator_controller.go"),
Path("internal/controller/destinationsyncer_controller.go"),
]
checks = {
"pipeline has no deletion handler": "DeletionTimestamp" not in pipeline,
"pipeline creates owner references": pipeline.count("SetControllerReference(") == 5,
"gdrive teardown ignores list errors": "output, _ := s3Client.ListObjectsV2" in gdrive,
"gdrive teardown ignores object-delete errors": "_, _ = s3Client.DeleteObject" in gdrive,
"gdrive teardown ignores bucket-delete errors": "_, _ = s3Client.DeleteBucket" in gdrive,
}
for name, result in checks.items():
print(f"{'PASS' if result else 'FAIL'}: {name}")
for path in children:
text = path.read_text()
reconcile = text[text.find("func ("):text.find("func (", text.find("func (") + 1)]
print(f"{path}: deletion guard in first reconcile slice = {'DeletionTimestamp' in reconcile}")
PYRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 26101
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C 6 'S3Destination|PutObject|Upload|Sync|UpdateStatus|StatusPatch' internal/controller/destinationsyncer_controller.go pkg -g '*.go' | head -n 300
python3 - <<'PY'
from pathlib import Path
pipeline = Path("internal/controller/unstructureddatapipeline_controller.go").read_text()
gdrive = Path("test/e2e/gdrive_test.go").read_text()
children = [
Path("internal/controller/sourcecrawler_controller.go"),
Path("internal/controller/documentprocessor_controller.go"),
Path("internal/controller/chunksgenerator_controller.go"),
Path("internal/controller/vectorembeddingsgenerator_controller.go"),
Path("internal/controller/destinationsyncer_controller.go"),
]
checks = {
"pipeline has no deletion handler": "DeletionTimestamp" not in pipeline,
"pipeline creates owner references": pipeline.count("SetControllerReference(") == 5,
"gdrive teardown ignores list errors": "output, _ := s3Client.ListObjectsV2" in gdrive,
"gdrive teardown ignores object-delete errors": "_, _ = s3Client.DeleteObject" in gdrive,
"gdrive teardown ignores bucket-delete errors": "_, _ = s3Client.DeleteBucket" in gdrive,
}
for name, result in checks.items():
print(f"{'PASS' if result else 'FAIL'}: {name}")
for path in children:
text = path.read_text()
reconcile = text[text.find("func ("):text.find("func (", text.find("func (") + 1)]
print(f"{path}: deletion guard in first reconcile slice = {'DeletionTimestamp' in reconcile}")
PYRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 26018
Wait for pipeline and child CR deletion before cleaning the bucket. Use foreground deletion or wait until all resources are absent, then surface ListObjectsV2, DeleteObject, and DeleteBucket errors. Otherwise, a child stage can write after cleanup and leave gdrive-output-bucket for the next test.
🤖 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/gdrive_test.go` around lines 321 - 339, Update the cleanup flow
after deleting the pipeline to wait for the pipeline and its child resources to
be fully absent before touching the output bucket, using foreground deletion or
an equivalent wait. In the bucket cleanup block, handle and surface errors from
ListObjectsV2, each DeleteObject, and DeleteBucket instead of discarding them,
while preserving the existing object-removal sequence.
| scaleDown := fmt.Sprintf("kubectl scale deployment/%s -n %s --replicas=0", deploymentName, testNamespace) | ||
| if p := utils.RunCommandContext(ctx, scaleDown); p.Err() != nil { | ||
| log.Printf("Failed to scale down: %s", p.Err()) | ||
| } | ||
| scaleUp := fmt.Sprintf("kubectl scale deployment/%s -n %s --replicas=1", deploymentName, testNamespace) | ||
| if p := utils.RunCommandContext(ctx, scaleUp); p.Err() != nil { | ||
| log.Printf("Failed to scale up: %s", p.Err()) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# Map the test file and inspect the setup function around the scale commands.
ast-grep outline test/e2e/main_test.go
sed -n '100,190p' test/e2e/main_test.go
# Find the enclosing function's callers and the command-result API usage.
rg -n -C 3 'RunCommandContext|scaleDown|scaleUp|CURRENT_ENV|func .*Setup|func .*setup' test/e2eRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 11254
🏁 Script executed:
# Inspect the command-result implementation and the setup caller to confirm that
# returning p.Err() aborts test setup and that Err() reflects command failure.
rg -n -C 5 'func RunCommandContext|type .*Process|func \(.*\) Err\(\)|testSetup\(' .Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 50403
🏁 Script executed:
# Narrow the lookup to the vendored e2e-framework command implementation.
rg -n -C 8 'RunProcWithContext|type Proc|func \(.*Proc.*\) Err|Wait\(' vendor/sigs.k8s.io/e2e-framework/pkgRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 3291
🏁 Script executed:
# Resolve the imported exec package and inspect only its process methods.
sed -n '1,45p' vendor/sigs.k8s.io/e2e-framework/pkg/utils/command.go
rg -n -C 6 'package exec|type Proc|RunProcWithContext|func \(.*\) Err' vendor/sigs.k8s.io/e2e-framework vendor | head -200Repository: redhat-data-and-ai/unstructured-data-controller
Length of output: 17185
🏁 Script executed:
# Read-only verifier: check the two scale branches and the testSetup caller
# without executing repository code.
python3 - <<'PY'
from pathlib import Path
import re
path = Path("test/e2e/main_test.go")
text = path.read_text()
for name in ("scaleDown", "scaleUp"):
match = re.search(
rf'if p := utils\.RunCommandContext\(ctx, {name}\); p\.Err\(\) != nil \{{(?P<body>.*?)\n\t\t\}}',
text,
re.S,
)
if not match:
raise SystemExit(f"{name}: branch not found")
body = match.group("body")
print(f"{name}: returns p.Err() = {'return p.Err()' in body}")
print(f"{name}: logs failure = {'log.Printf' in body}")
caller = re.search(
r'if err := testSetup\(ctx, &runningProcesses, config\); err != nil \{(?P<body>.*?)\n\t\t\}',
text,
re.S,
)
if not caller:
raise SystemExit("testSetup caller not found")
print(f"testSetup caller terminates on error = {'log.Fatalf' in caller.group('body')}")
PYRepository: redhat-data-and-ai/unstructured-data-controller
Length of output: 357
Propagate scale command failures.
If either scale command fails, return p.Err() so testSetup stops before continuing with an unverified operator restart.
🤖 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/main_test.go` around lines 156 - 163, Update the scale-down and
scale-up command handling in testSetup to return p.Err() immediately when either
utils.RunCommandContext invocation fails, instead of only logging and
continuing. Preserve the existing scale command order so setup stops before
proceeding after an unsuccessful operator restart.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests