fix: flaky api integration tests

[kaixi-wang]

Rationale

• relying on sanic's motd is unreliable, so lets just use the debug messages in the api since logging level is set to debug

Review Priority

• high
• medium
• low

Changes

Checklist

• This PR maintains parity between Docker Compose and Helm

Testing

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a1aa076f-4afe-40a1-97d3-12bf1066fed1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR implements comprehensive telemetry sidecar integration for FiftyOne Enterprise across Docker Compose and Kubernetes/Helm deployments, replacing the legacy delegated-operator worker replica environment variable with Compose profile-based scaling, introducing new Helm template helpers for telemetry configuration, and adding extensive test coverage for telemetry functionality.

Changes

Telemetry Sidecar Deployment & Configuration

Layer / File(s)	Summary
Docker Compose Telemetry Documentation & Configuration docker/README.md, docker/docs/configuring-telemetry.md, docker/docs/configuring-delegated-operators.md, docker/docs/configuring-gpu-workloads.md, docker/docs/upgrading.md, docker/internal-auth/env.template, docker/legacy-auth/env.template, tests/fixtures/docker/integration_*.env	Docker documentation expanded to cover telemetry architecture, Compose profile-based worker slot scaling (replacing FIFTYONE_DELEGATED_OPERATOR_WORKER_REPLICAS), GPU enablement, opt-out procedures, and environment variable overrides; configuration templates updated with telemetry Redis and target-name matching options; test fixtures bumped to newer RC versions.
Kubernetes Orchestrator Telemetry Documentation docs/orchestrators/configuring-kubernetes-orchestrator.md	Kubernetes on-demand orchestrator documentation added with telemetry sidecar initContainer patterns, required pod spec settings (shared process namespace, security context, socket volumes), full production template examples, and cleanup guidance.
Helm Chart Documentation & Values Reference helm/docs/upgrading.md, helm/fiftyone-teams-app/README.md	Helm upgrade documentation details telemetry sidecars, resource impact, cluster requirements (PSA constraints, shareProcessNamespace), external Redis configuration, and disable options; chart README updated with explicit pod security context defaults and new telemetry configuration section.
Helm Telemetry Template Helpers & Definitions helm/fiftyone-teams-app/templates/_telemetry.tpl	New helper template file defining Redis service naming/URL resolution (external override + in-cluster FQDN), RBAC role/rolebinding naming and subject management, sidecar environment variable rendering (pod metadata, service naming, Redis wiring, database secrets), standard and native sidecar container templates with hardened security contexts (capability drops, privilege escalation disablement, conditional SYS_PTRACE for executors), and conditional Redis URL environment variable emission.
Helm Environment Variable Wiring for Services helm/fiftyone-teams-app/templates/_helpers.tpl, helm/fiftyone-teams-app/templates/_do_targets.tpl	Service and delegated-operator environment variable templates updated to inject FIFTYONE_TELEMETRY_REDIS_URL via new telemetry redis-url-env helper and TELEMETRY_SOCKET=/tmp/telemetry/agent.sock when telemetry is enabled.
Helm Chart Deployment Notes & User Warnings helm/fiftyone-teams-app/templates/NOTES.txt	Added conditional warning when telemetry is enabled without Redis persistence, informing users that emptyDir volumes lose telemetry data on restart.

Test Infrastructure, Helpers, and New Telemetry-Specific Tests

Layer / File(s)	Summary
Test Helper Functions & Common Infrastructure tests/unit/helm/yaml_helpers.go, tests/unit/helm/common_test.go, utils/bump-fixtures-helm.sh	Added YAML parsing helpers (splitYAMLDocs, disableTelemetry) for Helm test scenarios; build tag additions for telemetry test variants; script updated to bump telemetry sidecar image tag in Helm fixtures.
Helm Unit Tests - Telemetry Redis Configuration tests/unit/helm/telemetry-redis_test.go	New comprehensive test suite verifying Redis Helm template rendering (enabled by default, disabled when telemetry.enabled=false, skipped when external URL is set), URL wiring across API/delegated-operator deployments and job ConfigMaps, deployment metadata/image/args, service/PVC metadata, and persistence volume modes (emptyDir, existingClaim, PVC).
Helm Unit Tests - Telemetry RBAC Configuration tests/unit/helm/telemetry-rolebinding_test.go	New test suite asserting telemetry RBAC Role/RoleBinding rendering, including pod log access (get on pods and pods/log), default/custom subject configuration, roleRef accuracy, and label verification.
Helm Unit Tests - Telemetry Sidecar Injection tests/unit/helm/telemetry-sidecar_test.go	New test suite verifying sidecar injection across workload templates, including shareProcessNamespace=true when telemetry enabled, security context with capability drops and no privilege escalation, and conditional SYS_PTRACE for executor vs. service sidecars.
Helm Unit Tests - API, App, Plugins Deployments Updated tests/unit/helm/api-deployment_test.go, tests/unit/helm/api-role_test.go, tests/unit/helm/app-deployment_test.go, tests/unit/helm/plugins-deployment_test.go	Updated to render Helm charts with disableTelemetry by default, added explicit telemetry on/off test cases, updated pod security context defaults to concrete values (fsGroup/runAsGroup/runAsUser=1000, runAsNonRoot=true), and verified RBAC rules include pods/log access.
Helm Unit Tests - Delegated Operator Deployments & Jobs Updated tests/unit/helm/delegated-operator-instance-deployment_test.go, tests/unit/helm/delegated-operator-job-configmap_test.go	Applied disableTelemetry helper throughout test cases, updated pod security context expectations, and added TestTelemetrySocketInjection verifying correct telemetry socket volume/mount injection without duplication.
Helm Unit Tests - Teams App Deployment Updated tests/unit/helm/teams-app-deployment_test.go	All Helm template rendering calls updated to use disableTelemetry transformation on test values.
Docker Compose Unit Tests - Service Configuration Validation tests/unit/compose/docker-compose-internal-auth_test.go, tests/unit/compose/docker-compose-legacy-auth_test.go	Extended to support Compose profiles for delegated-operator slot activation and GPU variants; updated expectations for telemetry service names, images, environment variables (FIFTYONE_TELEMETRY_REDIS_URL, TELEMETRY_SOCKET), volumes (telemetry-redis-data, telemetry-socket), and resource configuration across all compose scenarios.
Integration Tests & Utility Script Updates tests/integration/compose/docker-compose-*.go, utils/validate-docker-pulls.sh	Integration test fixtures bumped to newer RC versions with delegated-operator worker replica env var removed; integration tests updated to expect new service readiness log markers (Successfully connected to MongoDB vs. Starting worker); Docker pull validation script expanded to include redis and telemetry sidecar images.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• voxel51/fiftyone-teams-app-deploy#571: Both PRs revolve around the same telemetry-sidecar integration: updating delegated-operator worker slot scaling to use Compose profiles, injecting FIFTYONE_TELEMETRY_REDIS_URL/TELEMETRY_SOCKET via Helm/Compose templates, and updating the related Docker/Helm telemetry docs/tests accordingly.

Suggested labels

priority-low

Suggested reviewers

• kevin-dimichel
• mo-getter

Poem

🐰 A telemetry sidecar hops into view,
Metrics flowing freely in Redis—how true!
Pod logs and processes now share a namespace,
Leaving no operational trace out of place. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description includes the rationale section explaining why the changes are made, but the Changes section is left empty and testing section is incomplete. Required checklist items are present but not all mandatory template sections are adequately filled.	Complete the Changes section describing what was modified (test expectations updated), and provide explicit testing steps showing how the flaky tests were verified to be more reliable.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: flaky api integration tests' is specific and directly describes the main purpose of the PR: fixing flaky API integration tests by updating test assertions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch kacey/fix-flaky-api-tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (4)

tests/unit/helm/telemetry-rolebinding_test.go (1)

64-88: ⚡ Quick win

Align string matching patterns for consistency and robustness.

The extractRole helper (line 67) requires newlines around kind: Role ("\nkind: Role\n"), while extractRoleBinding (line 80) matches kind: RoleBinding without newlines. This inconsistency could allow extractRoleBinding to match the string in unexpected contexts (e.g., field values or annotations), though this is unlikely given the templates being rendered.

For consistency and defensive coding, align both helpers to use the same pattern.

♻️ Proposed fix to align the patterns

 func (s *telemetryRoleBindingTemplateTest) extractRoleBinding(output string) (rbacv1.RoleBinding, bool) {
 	for _, doc := range splitYAMLDocs(output) {
-		if !strings.Contains(doc, "kind: RoleBinding") {
+		if !strings.Contains(doc, "\nkind: RoleBinding\n") {
 			continue
 		}
 		var rb rbacv1.RoleBinding

🤖 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 `@tests/unit/helm/telemetry-rolebinding_test.go` around lines 64 - 88, The two
helpers are inconsistent: extractRole checks for "\nkind: Role\n" but
extractRoleBinding checks for "kind: RoleBinding" without newlines; change the
pattern in extractRoleBinding to use the same newline-delimited check (e.g.
"\nkind: RoleBinding\n") so both extractRole and extractRoleBinding use the same
defensive matching before calling splitYAMLDocs and helm.UnmarshalK8SYaml to
parse the document.

tests/unit/helm/api-deployment_test.go (1)

2403-2406: ⚡ Quick win

Add explicit nil guards before dereferencing pod security context pointers.

These assertions can panic before producing a clear test failure message. Add NotNil checks for podSecurityContext and each pointer field before dereferencing.

Proposed test hardening

 			func(podSecurityContext *corev1.PodSecurityContext) {
+				s.Require().NotNil(podSecurityContext)
+				s.Require().NotNil(podSecurityContext.FSGroup)
+				s.Require().NotNil(podSecurityContext.RunAsGroup)
+				s.Require().NotNil(podSecurityContext.RunAsNonRoot)
+				s.Require().NotNil(podSecurityContext.RunAsUser)
 				s.Equal(int64(1000), *podSecurityContext.FSGroup, "fsGroup should be 1000 (image UID)")
 				s.Equal(int64(1000), *podSecurityContext.RunAsGroup, "runAsGroup should be 1000")
 				s.True(*podSecurityContext.RunAsNonRoot, "runAsNonRoot should be true")
 				s.Equal(int64(1000), *podSecurityContext.RunAsUser, "runAsUser should be 1000")

🤖 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 `@tests/unit/helm/api-deployment_test.go` around lines 2403 - 2406, The test
currently dereferences podSecurityContext and its pointer fields (FSGroup,
RunAsGroup, RunAsNonRoot, RunAsUser) directly which can panic; add explicit
NotNil assertions before each dereference: assert NotNil(t, podSecurityContext)
then NotNil for podSecurityContext.FSGroup, podSecurityContext.RunAsGroup,
podSecurityContext.RunAsNonRoot and podSecurityContext.RunAsUser, and only after
those NotNil checks perform the existing Equal/True assertions so failures
produce clear test messages instead of panics.

tests/unit/helm/app-deployment_test.go (1)

2094-2097: ⚡ Quick win

Guard pointer fields before dereference in pod security context assertions.

Adding explicit NotNil assertions will produce clearer failures than a panic when rendered defaults change.

Proposed test hardening

 			func(podSecurityContext *corev1.PodSecurityContext) {
+				s.Require().NotNil(podSecurityContext)
+				s.Require().NotNil(podSecurityContext.FSGroup)
+				s.Require().NotNil(podSecurityContext.RunAsGroup)
+				s.Require().NotNil(podSecurityContext.RunAsNonRoot)
+				s.Require().NotNil(podSecurityContext.RunAsUser)
 				s.Equal(int64(1000), *podSecurityContext.FSGroup, "fsGroup should be 1000 (image UID)")
 				s.Equal(int64(1000), *podSecurityContext.RunAsGroup, "runAsGroup should be 1000")
 				s.True(*podSecurityContext.RunAsNonRoot, "runAsNonRoot should be true")
 				s.Equal(int64(1000), *podSecurityContext.RunAsUser, "runAsUser should be 1000")

🤖 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 `@tests/unit/helm/app-deployment_test.go` around lines 2094 - 2097, The test
dereferences podSecurityContext pointer fields directly (FSGroup, RunAsGroup,
RunAsNonRoot, RunAsUser) which can panic if defaults change; add explicit NotNil
(or Require().NotNil) assertions for each field (podSecurityContext.FSGroup,
podSecurityContext.RunAsGroup, podSecurityContext.RunAsNonRoot,
podSecurityContext.RunAsUser) immediately before the s.Equal/s.True assertions
so failures show clear messages instead of panics when those pointers are nil.

tests/unit/helm/plugins-deployment_test.go (1)

2525-2528: ⚡ Quick win

Add nil checks before dereferencing pod security context fields.

This prevents panic-style failures and keeps assertion output actionable when defaults change.

Proposed test hardening

 			func(podSecurityContext *corev1.PodSecurityContext) {
+				s.Require().NotNil(podSecurityContext)
+				s.Require().NotNil(podSecurityContext.FSGroup)
+				s.Require().NotNil(podSecurityContext.RunAsGroup)
+				s.Require().NotNil(podSecurityContext.RunAsNonRoot)
+				s.Require().NotNil(podSecurityContext.RunAsUser)
 				s.Equal(int64(1000), *podSecurityContext.FSGroup, "fsGroup should be 1000 (image UID)")
 				s.Equal(int64(1000), *podSecurityContext.RunAsGroup, "runAsGroup should be 1000")
 				s.True(*podSecurityContext.RunAsNonRoot, "runAsNonRoot should be true")
 				s.Equal(int64(1000), *podSecurityContext.RunAsUser, "runAsUser should be 1000")

🤖 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 `@tests/unit/helm/plugins-deployment_test.go` around lines 2525 - 2528, Add nil
checks before dereferencing podSecurityContext and its pointer fields in the
test: assert podSecurityContext is not nil (e.g., s.NotNil(podSecurityContext))
and assert each pointer field is not nil (FSGroup, RunAsGroup, RunAsNonRoot,
RunAsUser) before using *podSecurityContext.FSGroup,
*podSecurityContext.RunAsGroup, *podSecurityContext.RunAsNonRoot, and
*podSecurityContext.RunAsUser; after those NotNil checks, keep the existing
s.Equal/s.True assertions to compare the dereferenced values.

🤖 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 `@docker/docs/configuring-gpu-workloads.md`:
- Around line 62-66: The docs use two different GPU worker service names —
`teams-do-with-gpu` (current line) and `teams-do-gpu` elsewhere — so pick one
canonical service name (recommend `teams-do-gpu`) and update this sentence to
use that name, and also update the matching telemetry sidecar name to the
consistent form (`teams-do-gpu-telemetry`) and any references to the example
compose file (`compose.delegated-operators.gpu.yaml`) to ensure all occurrences
(service, telemetry sidecar, and examples) match the chosen identifier.
- Around line 54-59: The example uses an invalid profile name `do-1`; update the
docs to use the correct CPU worker profile (e.g., `do`) so the GPU overlay
`compose.delegated-operators.gpu.yaml` is activated with the CPU profile: change
the example `COMPOSE_PROFILES=do-1,gpu` to `COMPOSE_PROFILES=do,gpu` (or to
whichever documented CPU profile name is used in your repo) and ensure the text
mentions activating the GPU worker and its telemetry sidecar together.

In `@docker/docs/upgrading.md`:
- Around line 143-144: The Markdown line uses nested bold markers around
`teams-do` and `SYS_PTRACE` creating malformed emphasis; update the line so only
one emphasis/span is used (e.g., remove the outer ** around `teams-do` or
convert to plain text) so it reads cleanly like: `teams-do` requires the
`SYS_PTRACE` capability to allow the telemetry agent to observe the target
process, ensuring correct rendering of the `teams-do` and `SYS_PTRACE` tokens.

In `@tests/unit/helm/delegated-operator-job-configmap_test.go`:
- Around line 747-751: Test currently indexes
job.Spec.Template.Spec.Containers[0] which is fragile; instead locate the target
container by identity and assert its mounts. Update the assertion to find the
container in job.Spec.Template.Spec.Containers whose .Name (or another stable
identifier used elsewhere in the test) matches the expected container, then call
countMountsByName on that container's VolumeMounts and compare to
testCase.expectMnt using socketName and countMountsByName; ensure you still
require the container exists before asserting.

In `@utils/validate-docker-pulls.sh`:
- Around line 17-18: The script currently appends :${expected_tag} to every
voxel51/* image even when the image already includes an explicit tag; update the
image-handling logic (where the script inspects each image string, e.g., the
variable holding the image name) so it only appends :${expected_tag} when the
image does not already contain a tag (detect tag presence with a regex or shell
test that checks for a colon after the repository/name, e.g. match /:[^/]+$/ or
use parameter expansion to see if the image contains ':' after the last '/'),
and apply the same fix to the duplicated logic around the block noted as lines
107-114 so tagged voxel51/...:... entries are left unchanged.

---

Nitpick comments:
In `@tests/unit/helm/api-deployment_test.go`:
- Around line 2403-2406: The test currently dereferences podSecurityContext and
its pointer fields (FSGroup, RunAsGroup, RunAsNonRoot, RunAsUser) directly which
can panic; add explicit NotNil assertions before each dereference: assert
NotNil(t, podSecurityContext) then NotNil for podSecurityContext.FSGroup,
podSecurityContext.RunAsGroup, podSecurityContext.RunAsNonRoot and
podSecurityContext.RunAsUser, and only after those NotNil checks perform the
existing Equal/True assertions so failures produce clear test messages instead
of panics.

In `@tests/unit/helm/app-deployment_test.go`:
- Around line 2094-2097: The test dereferences podSecurityContext pointer fields
directly (FSGroup, RunAsGroup, RunAsNonRoot, RunAsUser) which can panic if
defaults change; add explicit NotNil (or Require().NotNil) assertions for each
field (podSecurityContext.FSGroup, podSecurityContext.RunAsGroup,
podSecurityContext.RunAsNonRoot, podSecurityContext.RunAsUser) immediately
before the s.Equal/s.True assertions so failures show clear messages instead of
panics when those pointers are nil.

In `@tests/unit/helm/plugins-deployment_test.go`:
- Around line 2525-2528: Add nil checks before dereferencing podSecurityContext
and its pointer fields in the test: assert podSecurityContext is not nil (e.g.,
s.NotNil(podSecurityContext)) and assert each pointer field is not nil (FSGroup,
RunAsGroup, RunAsNonRoot, RunAsUser) before using *podSecurityContext.FSGroup,
*podSecurityContext.RunAsGroup, *podSecurityContext.RunAsNonRoot, and
*podSecurityContext.RunAsUser; after those NotNil checks, keep the existing
s.Equal/s.True assertions to compare the dereferenced values.

In `@tests/unit/helm/telemetry-rolebinding_test.go`:
- Around line 64-88: The two helpers are inconsistent: extractRole checks for
"\nkind: Role\n" but extractRoleBinding checks for "kind: RoleBinding" without
newlines; change the pattern in extractRoleBinding to use the same
newline-delimited check (e.g. "\nkind: RoleBinding\n") so both extractRole and
extractRoleBinding use the same defensive matching before calling splitYAMLDocs
and helm.UnmarshalK8SYaml to parse the document.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a87fb8c8-64fa-4381-a30e-d32752808b34

📥 Commits

Reviewing files that changed from the base of the PR and between 195cff9 and 308b825.

⛔ Files ignored due to path filters (32)

• docker/common-services.yaml is excluded by !**/*.yaml
• docker/internal-auth/compose.dedicated-plugins.yaml is excluded by !**/*.yaml
• docker/internal-auth/compose.delegated-operators.gpu.yaml is excluded by !**/*.yaml
• docker/internal-auth/compose.delegated-operators.yaml is excluded by !**/*.yaml
• docker/internal-auth/compose.plugins.yaml is excluded by !**/*.yaml
• docker/internal-auth/compose.yaml is excluded by !**/*.yaml
• docker/legacy-auth/compose.dedicated-plugins.yaml is excluded by !**/*.yaml
• docker/legacy-auth/compose.delegated-operators.gpu.yaml is excluded by !**/*.yaml
• docker/legacy-auth/compose.delegated-operators.yaml is excluded by !**/*.yaml
• docker/legacy-auth/compose.plugins.yaml is excluded by !**/*.yaml
• docker/legacy-auth/compose.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/api-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/api-role.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/app-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/delegated-operator-instance-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/delegated-operator-job-configmap.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/plugins-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/telemetry-redis-deployment.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/telemetry-redis-pvc.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/telemetry-redis-service.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/telemetry-role.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/telemetry-rolebinding.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/values.schema.json is excluded by !**/*.json
• helm/fiftyone-teams-app/values.yaml is excluded by !**/*.yaml
• tests/fixtures/docker/compose.override.mongodb.yaml is excluded by !**/*.yaml
• tests/fixtures/docker/compose.override.mongodb_do.yaml is excluded by !**/*.yaml
• tests/fixtures/docker/compose.override.mongodb_plugins.yaml is excluded by !**/*.yaml
• tests/fixtures/helm/integration_values.yaml is excluded by !**/*.yaml
• tests/unit/helm/test_data/delegated-operator-job-configmap_test/expected-cpu-default-override-template-values.yaml is excluded by !**/*.yaml
• tests/unit/helm/test_data/delegated-operator-job-configmap_test/expected-cpu-default.yaml is excluded by !**/*.yaml
• tests/unit/helm/test_data/delegated-operator-job-configmap_test/expected-override-example-cascading-template.yaml is excluded by !**/*.yaml
• tests/unit/helm/test_data/delegated-operator-job-configmap_test/expected-override-example-template.yaml is excluded by !**/*.yaml

📒 Files selected for processing (34)

• docker/README.md
• docker/docs/configuring-delegated-operators.md
• docker/docs/configuring-gpu-workloads.md
• docker/docs/configuring-telemetry.md
• docker/docs/upgrading.md
• docker/internal-auth/env.template
• docker/legacy-auth/env.template
• docs/orchestrators/configuring-kubernetes-orchestrator.md
• helm/docs/upgrading.md
• helm/fiftyone-teams-app/README.md
• helm/fiftyone-teams-app/templates/NOTES.txt
• helm/fiftyone-teams-app/templates/_do_targets.tpl
• helm/fiftyone-teams-app/templates/_helpers.tpl
• helm/fiftyone-teams-app/templates/_telemetry.tpl
• tests/fixtures/docker/integration_internal_auth.env
• tests/fixtures/docker/integration_legacy_auth.env
• tests/integration/compose/docker-compose-internal-auth_test.go
• tests/integration/compose/docker-compose-legacy-auth_test.go
• tests/unit/compose/docker-compose-internal-auth_test.go
• tests/unit/compose/docker-compose-legacy-auth_test.go
• tests/unit/helm/api-deployment_test.go
• tests/unit/helm/api-role_test.go
• tests/unit/helm/app-deployment_test.go
• tests/unit/helm/common_test.go
• tests/unit/helm/delegated-operator-instance-deployment_test.go
• tests/unit/helm/delegated-operator-job-configmap_test.go
• tests/unit/helm/plugins-deployment_test.go
• tests/unit/helm/teams-app-deployment_test.go
• tests/unit/helm/telemetry-redis_test.go
• tests/unit/helm/telemetry-rolebinding_test.go
• tests/unit/helm/telemetry-sidecar_test.go
• tests/unit/helm/yaml_helpers.go
• utils/bump-fixtures-helm.sh
• utils/validate-docker-pulls.sh

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix invalid profile example (do-1) in GPU instructions.

Line 57 uses COMPOSE_PROFILES=do-1,gpu, but slot 1 is the default and no do-1 profile is documented elsewhere. This can prevent the intended services from starting as described.

Suggested doc fix

- worker profile (e.g. `COMPOSE_PROFILES=do-1,gpu`); see
+ worker/default slot plus GPU profile (e.g. `COMPOSE_PROFILES=gpu`); see

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	For most deployments, prefer the shipped GPU overlay
	`compose.delegated-operators.gpu.yaml`, which already includes the GPU
	worker and its paired telemetry sidecar. Activate it alongside the CPU
	worker profile (e.g. `COMPOSE_PROFILES=do-1,gpu`); see
	[Configuring Telemetry](./configuring-telemetry.md#scaling-teams-do-with-telemetry)
	for the full profile reference.
	For most deployments, prefer the shipped GPU overlay
	`compose.delegated-operators.gpu.yaml`, which already includes the GPU
	worker and its paired telemetry sidecar. Activate it alongside the CPU
	worker/default slot plus GPU profile (e.g. `COMPOSE_PROFILES=gpu`); see
	[Configuring Telemetry](./configuring-telemetry.md#scaling-teams-do-with-telemetry)
	for the full profile reference.

🤖 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 `@docker/docs/configuring-gpu-workloads.md` around lines 54 - 59, The example
uses an invalid profile name `do-1`; update the docs to use the correct CPU
worker profile (e.g., `do`) so the GPU overlay
`compose.delegated-operators.gpu.yaml` is activated with the CPU profile: change
the example `COMPOSE_PROFILES=do-1,gpu` to `COMPOSE_PROFILES=do,gpu` (or to
whichever documented CPU profile name is used in your repo) and ensure the text
mentions activating the GPU worker and its telemetry sidecar together.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use one GPU worker service name consistently across docs.

Line 62 says teams-do-with-gpu, while other telemetry docs describe teams-do-gpu. Unifying the name avoids copy/paste and troubleshooting confusion.

🤖 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 `@docker/docs/configuring-gpu-workloads.md` around lines 62 - 66, The docs use
two different GPU worker service names — `teams-do-with-gpu` (current line) and
`teams-do-gpu` elsewhere — so pick one canonical service name (recommend
`teams-do-gpu`) and update this sentence to use that name, and also update the
matching telemetry sidecar name to the consistent form
(`teams-do-gpu-telemetry`) and any references to the example compose file
(`compose.delegated-operators.gpu.yaml`) to ensure all occurrences (service,
telemetry sidecar, and examples) match the chosen identifier.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix malformed markdown emphasis around SYS_PTRACE.

Line 143 has nested ** around `teams-do` and `SYS_PTRACE`, which renders awkwardly. Use plain text or a single emphasis span.

Suggested doc fix

-1. **`teams-do` requires the **`SYS_PTRACE`** capability to allow the
+1. **`teams-do` requires the `SYS_PTRACE` capability** to allow the

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	1. **`teams-do` requires the **`SYS_PTRACE`** capability to allow the
	telemetry agent to observe the target process.
	1. **`teams-do` requires the `SYS_PTRACE` capability** to allow the
	telemetry agent to observe the target process.

🤖 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 `@docker/docs/upgrading.md` around lines 143 - 144, The Markdown line uses
nested bold markers around `teams-do` and `SYS_PTRACE` creating malformed
emphasis; update the line so only one emphasis/span is used (e.g., remove the
outer ** around `teams-do` or convert to plain text) so it reads cleanly like:
`teams-do` requires the `SYS_PTRACE` capability to allow the telemetry agent to
observe the target process, ensuring correct rendering of the `teams-do` and
`SYS_PTRACE` tokens.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid positional container indexing in telemetry mount assertion.

Line 750 assumes the main workload is always Containers[0]; that makes this test fragile if container order changes. Select the target container by identity before asserting mount counts.

Suggested patch

 func (s *doK8sConfigMapTemplateTest) TestTelemetrySocketInjection() {
@@
 	countMountsByName := func(mounts []corev1.VolumeMount, name string) int {
@@
 	}
+	findMainContainer := func(containers []corev1.Container) *corev1.Container {
+		for i := range containers {
+			// Delegated-operator job container uses the fiftyone command.
+			if len(containers[i].Command) > 0 && containers[i].Command[0] == "fiftyone" {
+				return &containers[i]
+			}
+		}
+		return nil
+	}
@@
-			s.Require().NotEmpty(job.Spec.Template.Spec.Containers, "expected at least one container")
+			main := findMainContainer(job.Spec.Template.Spec.Containers)
+			s.Require().NotNil(main, "expected delegated-operator container")
 			s.Equal(
 				testCase.expectMnt,
-				countMountsByName(job.Spec.Template.Spec.Containers[0].VolumeMounts, socketName),
+				countMountsByName(main.VolumeMounts, socketName),
 				"telemetry-socket volumeMount count mismatch",
 			)

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	s.Require().NotEmpty(job.Spec.Template.Spec.Containers, "expected at least one container")
	s.Equal(
	testCase.expectMnt,
	countMountsByName(job.Spec.Template.Spec.Containers[0].VolumeMounts, socketName),
	"telemetry-socket volumeMount count mismatch",
	main := findMainContainer(job.Spec.Template.Spec.Containers)
	s.Require().NotNil(main, "expected delegated-operator container")
	s.Equal(
	testCase.expectMnt,
	countMountsByName(main.VolumeMounts, socketName),
	"telemetry-socket volumeMount count mismatch",

🤖 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 `@tests/unit/helm/delegated-operator-job-configmap_test.go` around lines 747 -
751, Test currently indexes job.Spec.Template.Spec.Containers[0] which is
fragile; instead locate the target container by identity and assert its mounts.
Update the assertion to find the container in job.Spec.Template.Spec.Containers
whose .Name (or another stable identifier used elsewhere in the test) matches
the expected container, then call countMountsByName on that container's
VolumeMounts and compare to testCase.expectMnt using socketName and
countMountsByName; ensure you still require the container exists before
asserting.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Match tag-append behavior to the documented rule.

The new comment says explicitly tagged images are used as-is, but current logic still appends :${expected_tag} to every voxel51/* image. If a tagged voxel51/...:... entry is added later, this will generate an invalid double-tag image string.

Proposed fix

 for img in "${EXPECTED_IMAGES[@]}"; do
-  if [[ ${img} =~ "voxel51/" ]]; then
+  if [[ ${img} == voxel51/* && ${img} != *:* ]]; then
     # Only add a tag for our organizations images, not
     # publicly available ones our chart may reference
     img_with_tag="${img}:${expected_tag}"
   else
     img_with_tag="${img}"
   fi

Also applies to: 107-114

🤖 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 `@utils/validate-docker-pulls.sh` around lines 17 - 18, The script currently
appends :${expected_tag} to every voxel51/* image even when the image already
includes an explicit tag; update the image-handling logic (where the script
inspects each image string, e.g., the variable holding the image name) so it
only appends :${expected_tag} when the image does not already contain a tag
(detect tag presence with a regex or shell test that checks for a colon after
the repository/name, e.g. match /:[^/]+$/ or use parameter expansion to see if
the image contains ':' after the last '/'), and apply the same fix to the
duplicated logic around the block noted as lines 107-114 so tagged
voxel51/...:... entries are left unchanged.