Skip to content

feat: unified orchestrator templates#604

Open
mo-getter wants to merge 6 commits into
youcef/service-orchestrator-chartfrom
feat/unified-orchestrator-templates
Open

feat: unified orchestrator templates#604
mo-getter wants to merge 6 commits into
youcef/service-orchestrator-chartfrom
feat/unified-orchestrator-templates

Conversation

@mo-getter

@mo-getter mo-getter commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Rationale

FiftyOne is adding service-based orchestrators (kubernetes-service environment: long-lived, on-demand service pods) alongside the existing job-based delegated operators, and deployments need to configure both. youcef/service-orchestrator-chart built the foundations for this — the service pod template, the orchestrator-seeding Job, and the builtin-services overrides — as a parallel configuration surface (its own ConfigMaps, values trees, and a hand-authored orchestrator registration list).

This PR lands that functionality unified under the existing delegatedOperatorJobTemplates pattern instead, so operators configure job- and service-based orchestrators one way, and the chart diff against main stays as small and reviewable as possible.

Review Priority

  • high
  • medium
  • low

Changes

  • delegatedOperatorJobTemplates.services replaces the standalone pod-template ConfigMap. Each entry renders a jinja2 Pod manifest into the same do-templates ConfigMap as jobs, merged over template with the same logic — and since that ConfigMap is already mounted at /tmp/do-targets, no new teams-api wiring is needed.
  • Registrations are derived, not hand-authored. template.registerOrchestrator (default true, overridable per entry) replaces the explicit seedOrchestrators.orchestrators list: instance_id = entry name, execution_tmpl_uri = the entry's file, merged worker image for jobs, available_operators pinned to run_service for services. This also retires the DEFAULT_WORKER_IMAGE image-sentinel, since helm renders the concrete merged image per entry.
  • The chart owns the pod shape. Service sizing and node targeting come from the values merge (like jobs); the broker's resource_requests vars are deliberately not consumed. Different sizes = different services entries.
  • Smaller interface. The seedOrchestrators values section is gone — the seeding Job hardcodes its mechanics and follows delegatedOperatorJobTemplates.template.image.

Upgrade note: deployments with existing jobs entries will seed registrations named after those entries on first upgrade (idempotent upserts after). Opt out with template.registerOrchestrator: false.

Checklist

  • This PR maintains parity between Docker Compose and Helm

Testing

  • New/extended terratest suites: service Pod rendering (merge semantics, broker vars, telemetry on/off, sizing ownership), seeding gating matrix and derived ORCHESTRATORS JSON, builtinServices ConfigMap, teams-api wiring.
  • Full helm unit suite and all pre-commit hooks pass; values.schema.json and README regenerated; full-chart helm template verified with jobs + services + builtinServices values.

Note

Ephem testing in progress.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • develop
  • release-*/

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: 20d3b98b-32dc-4a5d-afe8-e523abadb8f2

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/unified-orchestrator-templates

Comment @coderabbitai help to get the list of available commands.

mo-getter and others added 2 commits July 13, 2026 16:35
Force-skip helm-unit and helm-integration and let helm-pre-release
accept skipped results so every PR push still publishes the
fiftyone-teams-app sha chart for ephemeral environment testing.

DO NOT MERGE — revert before this branch lands in main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mo-getter mo-getter marked this pull request as ready for review July 14, 2026 00:53
@mo-getter mo-getter requested a review from a team as a code owner July 14, 2026 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant