Add monitor flag to common DO service deployment

[CamronStaley]

Rationale

This new flag will spawn a monitoring process that will check the DO status and kill it if a user has manually failed the DO during execution.

Changes

Add "-m" to do

Checklist

• This PR maintains parity between Docker Compose and Helm

Testing

tested locally and in ephem in previous PRs

[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.

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

Walkthrough

Adds the "-m" flag to the delegated launch command in documentation and updates unit tests to expect the new flag in constructed container arguments across multiple scenarios. No logic or public API changes.

Changes

Cohort / File(s)	Summary
Docs: GPU workloads command update docker/docs/configuring-gpu-workloads.md	Updated the example command for teams-do-with-gpu to append the -m flag to fiftyone delegated launch.
Tests: delegated operator args tests/unit/helm/delegated-operator-instance-deployment_test.go	Adjusted expected container argument lists in multiple test cases to include the -m flag; no control-flow or logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at the dash and m,
A tiny flag, yet straight and trim.
Docs now echo tests in tune,
Our commands aligned by afternoon.
Hop-hop! The pipelines softly hum—
One small switch, and off we run. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly indicates that a monitor flag has been added to the DO service deployment, which directly summarizes the primary change made by this pull request.
Description Check	✅ Passed	The pull request description follows the repository template by providing a rationale section explaining the problem, a concise description of the changes, a completed checklist confirming Docker Compose and Helm parity, and details on how the update was tested.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

docker/docs/configuring-gpu-workloads.md (1)

70-70: Add a brief note explaining -m.

Consider adding a short sentence in this doc (or linking to the delegated operators doc) explaining what the monitor flag does and when to use it.

tests/unit/helm/delegated-operator-instance-deployment_test.go (1)

4929-4943: Reduce duplication for expected args.

Consider a small helper to construct the common expected args (append -m), passing name/description as params. This will harden against future flag additions.

Also applies to: 4952-4966, 4967-4980, 4990-5004, 5005-5018, 5027-5041, 5042-5055, 5065-5079, 5080-5093

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bcea672 and 08a19ca.

⛔ Files ignored due to path filters (2)

• docker/common-services.yaml is excluded by !**/*.yaml
• helm/fiftyone-teams-app/templates/delegated-operator-instance-deployment.yaml is excluded by !**/*.yaml

📒 Files selected for processing (2)

• docker/docs/configuring-gpu-workloads.md (1 hunks)
• tests/unit/helm/delegated-operator-instance-deployment_test.go (9 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). (7)

• GitHub Check: helm-integration / integration-helm
• GitHub Check: helm-unit / unit-helm
• GitHub Check: docker-unit / integration-compose-legacy
• GitHub Check: docker-unit / integration-compose-internal
• GitHub Check: docker-unit / unit-compose
• GitHub Check: docker-pulls
• GitHub Check: pre-commit

🔇 Additional comments (4)

docker/docs/configuring-gpu-workloads.md (2)

70-70: LGTM: command now includes -m.

Good parity with the Helm/tests change.

---

70-70: Consistency verified for delegated launch commands
All occurrences of fiftyone delegated launch -t remote include -m.

tests/unit/helm/delegated-operator-instance-deployment_test.go (2)

4940-4940: LGTM: -m added to all expected arg sequences.

Covers default and override scenarios; ordering is consistent.

Also applies to: 4963-4963, 4977-4977, 5001-5001, 5015-5015, 5038-5038, 5052-5052, 5076-5076, 5090-5090

---

4929-4943: Verify ‘-m’ is emitted in rendered Helm templates for delegated-operator instances
Run helm template with DO enabled values and inspect .spec.template.spec.containers[].args to ensure -m appears for every delegated-operator instance.

[CamronStaley]

These changes shouldn't go out until 2.12

[swheaton]

Unless I'm mistaken, this does not pass pre commit checks? The helm readme must be generated via .gotpl. I think if this was undrafted and the PR action run, it would not pass.

This leads to the problem I was talking about where it's easy to add them in docker but because helm is generated, you have to add the vars to values.yaml which gives them a value instead of leaving it up to api to decide the default. Meaning we'd have to specify default in 2 places