Add workflow label support - #1194
Draft
jiaenren wants to merge 22 commits into
Draft
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1194 +/- ##
==========================================
+ Coverage 66.76% 67.00% +0.23%
==========================================
Files 202 203 +1
Lines 26050 26109 +59
Branches 3938 3952 +14
==========================================
+ Hits 17393 17494 +101
+ Misses 7917 7854 -63
- Partials 740 761 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
📖 Docs preview: https://d3in15bfzp49i0.cloudfront.net/1194/index.html |
cypres
reviewed
Jul 16, 2026
jiaenren
commented
Jul 16, 2026
| override_pod_template = copy.deepcopy(task_platform.parsed_pod_template) | ||
| substitute_pod_template_tokens(override_pod_template, jinja_variables) | ||
| pod = apply_pod_template(pod, override_pod_template) | ||
| pod = apply_workflow_labels(pod, workflow_labels) |
Collaborator
Author
There was a problem hiding this comment.
Swap with apply_pod_template so user labels don't override pod template.
cypres
reviewed
Jul 16, 2026
| override_pod_template = copy.deepcopy(task_platform.parsed_pod_template) | ||
| substitute_pod_template_tokens(override_pod_template, jinja_variables) | ||
| pod = apply_pod_template(pod, override_pod_template) | ||
| pod = apply_workflow_labels(pod, workflow_labels) |
Member
There was a problem hiding this comment.
Maybe swap the order, so any OSMO set labels are always applied and the user can't override any of our labels?
jiaenren
force-pushed
the
jiaenr/osmo-6501-workflow-labels
branch
from
July 23, 2026 22:26
e4fa258 to
6279cb8
Compare
jiaenren
force-pushed
the
jiaenr/osmo-6501-workflow-labels
branch
from
July 29, 2026 22:14
bcca93c to
b05b72f
Compare
3 tasks
Add one prefixed attribute per curated policy key to osmo_tasks_count, clamped to the allow-list plus <missing>/<other> sentinels so series stay bounded in every enforcement mode. Fix the gauge's counting defect by selecting COUNT(*) instead of emitting presence rows. Pin the OSS Workflow Resources dashboard as deployment-neutral (no PPP binding) with a structural test; PPP panels ship via internal provisioning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the unreachable JSON-text parsing branch (psycopg2 decodes JSONB to dicts), hoist loop-invariant attribute names, consolidate the triplicated patch scaffolding in the metrics tests, loosen change-detector assertions, and replace the dashboard title pinning with a deployment-neutrality scan. Re-home the osmo_tasks_count label-dimension reference docs from the CLI PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Correct the aggregation comment (clamping can collapse distinct rows today), explain the sentinel angle-bracket choice at both the code and docs sites, refresh the stale get_recent_tasks docstring, and note the Grafana schema quirk behind the query helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the NVIDIA-internal 'PPP' example key with the generic 'project' in the metrics test, dashboard-neutrality test, and the metrics docs, to match merged #1220. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Workflow list gains a Labels column and label/no-label filter chips wired to the list API; the detail page shows the stored labels and the recomputed warn-mode policy warnings; the resubmit panel carries labels forward and lets users edit them; the new-workflow submit page reads labels from the spec editor with an optional override editor. Adds a shared workflow-labels formatting module, adapter shims for the new response fields, and unit plus Playwright coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Compute the detail-page label entries once per render, replace a warnings useMemo with a stable module constant, type ResubmitParams priority to drop a cast, and remove two shim tests that exercised generated URL builders already covered end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move the module constant below imports, refresh stale JSDoc, document the shared label helpers and their Python cap mirror, unify warning toast order and list keys across the two submit surfaces, align copy with the app's terminology, and trim a duplicated e2e assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The UI slice owns the single regeneration of the OpenAPI-derived artifacts (openapi.json + generated client + mocks) for the whole label stack — the backend slices (#1220 and the B4-B8 PRs) deliberately carry none, so this avoids spreading generator drift across the stack. Regenerated from the full server surface (config policy, admission, list filters). Also replace the NVIDIA-internal 'PPP' example key with the generic 'project' in the UI tests and e2e journeys. Verified: tsc, eslint, vitest (1017), and production build all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run Prettier on the files touched by the PPP->project rename so format:check (part of ui-build's validate:coverage) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The label editor's other controls use the 'workflow label' phrasing
('Workflow label key/value N', 'Remove workflow label N'), but the add
button's accessible name was just 'Add label' — inconsistent, and the
resubmit/submit e2e journeys query 'Add workflow label', so the resubmit
test could never find the button. Align the button's aria-label and
visible text with the convention.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regenerate openapi.json + generated client/mocks from the current server (B5 admission incl. the help_text->assert_message rename, B6 filters, all now merged). Picks up an assert_message schema field the previously-frozen contract was missing. Verified: type-check, lint, format, vitest (1027), build all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Warnings panel is no longer framed as label-policy-specific: the `warnings` field is generic, so drop the 'Workflow label policy' sub-heading and rename the region to 'Workflow warnings' (cypres: there are other possible warnings than label policies). - Alias the orval-generated `SubmitWorkflowApiPoolPoolNameWorkflowPostParams` to `SubmitWorkflowParams` in the adapter (matching the existing `as WorkflowListEntry` pattern) and use it in actions.ts. Verified: type-check, lint, format, vitest (1027) pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds test/scenarios/workflow_labels.py, the KIND-runnable subset of the label feature verified through the OSMO API (the only channel a sandboxed OETF test has): - policy gate off/warn/enforce via validation-only submits (no rows, nothing schedules), asserting warn surfaces warnings and enforce rejects with 400 and leaves no row; - label-syntax rejection (nested value, invalid key, empty value); - list filtering by label, glob, and no_label; - one real end-to-end labeled workflow that runs to COMPLETED, whose labels survive submit -> persistence -> API surface + list filtering. Tagged `kind` so it runs in the `oetf:deploy_and_run --env kind` gate, which builds the whole stack locally and deploys it. Serial + exclusive because the policy tests mutate labels_config; each restores the baseline in tearDown, and mutation tests skip in ConfigMap mode. The heavier lifecycle and ConfigMap-fixture matrix runs on dev/Orin, not here. Based on B9 (#1227), the current stack tip, so the KIND gate validates the full label stack end to end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tagging a scenario `kind` is necessary but not sufficient — the KIND gate runs an explicit, cost-narrowed target-pattern allowlist (smoke + templates + mount-validation). Add //test/scenarios:workflow-labels so the label policy gate, filters, and one real labeled end-to-end run are covered on every full-stack KIND deployment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pass label/policy submit kwargs explicitly instead of **splatting a dict[str, str] into typed parameters, and narrow the rejection path with a single _expect_rejected helper (assertIsNotNone does not narrow for mypy; the try/except + self.fail idiom does). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
oetf_scenario_test runs the file as __main__ (main = src). Without an `if __name__ == "__main__": unittest.main()` block the py_test imported the module and exited 0 without executing any test method — a hollow pass. Add the standard entry point (matching every other scenario).
Three fixes found by actually running the tests on the KIND gate: - _set_policy compares only key/enforcement/allow_list; the LabelPolicy model carries extra fields (assert_message) that default in on read-back, so an exact-dict compare was wrong. - list-filter assertions use the workflow id returned by submit (which carries the "-<job>" suffix the list echoes), not the base name. - the end-to-end test asserts the label round-trip (submit -> persist -> workflow API + list filter) instead of run-to-completion: the PR-gate KIND does not run workflows to completion (only validation scenarios are gated), and pod stamping is covered by unit tests.
The label scenario is green (Ran 6 tests, OK); restore the upload step's if: failure() guard now that the diagnostic served its purpose. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jiaenren
force-pushed
the
jiaenr/osmo-6501-workflow-labels
branch
from
August 3, 2026 19:07
b05b72f to
15abaa8
Compare
…abels Operators can now set labels_config.pod_label_prefix (empty by default) to namespace user workflow labels on pods without users typing the prefix on every spec or query. The prefix is prepended to each label key only when the labels are stamped onto pod metadata; the workflow row, list filters, CLI, UI, and service metrics keep using the bare keys, so the submit/query UX is unchanged. The prefix is an opaque string, not assumed to be a DNS prefix: the key and prefix are merged first, then the merged key is validated as a Kubernetes label key at submission (including validation-only submits), so a user key that would form an invalid merged key (e.g. a key that already carries its own prefix) is rejected with a message naming the key, the prefix, and the resulting key. OSS default is empty to keep the deployment neutral; an operator sets the prefix (e.g. "osmo.nvidia.com/") in its own labels_config. System osmo.* labels and env-var derivation are unaffected. - LabelsConfig.pod_label_prefix field + light sanity validator (no whitespace, <=253 chars) - validation.apply_pod_label_prefix / validate_prefixed_workflow_label_keys - submission gate validates merged keys; stamping applies the prefix at both pod-build sites - regenerated openapi.json, generated.ts, generated-mocks.ts - unit tests for the transform, the schema field, and the submission gate Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apply_pod_label_prefix now reads workflow_config.labels_config.pod_label_prefix at both pod-build sites; two existing tests mocked workflow_config with a bare Mock, so the auto-mocked prefix was prepended to every label key. Point the create-group test at the configured context and set an empty prefix on the retry test's config mock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep the OSS repo deployment-neutral: the pod_label_prefix unit tests used a deployment-specific domain as the example prefix; switch to example.com. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue - None
Tracking: OSMO-6501
Purpose
OSMO needs immutable workflow metadata that survives admission, PostgreSQL persistence, restart/resubmit, and task-Pod creation. Existing mutable tags do not provide that contract or the Kubernetes labels needed for resource-attribution joins.
This draft integration PR adds generic workflow labels, with
PPPas the first configured policy and reporting use case. Defaults remain inert: labels are optional and no key is required until an administrator configures a policy.Current integration contents
labels, capped at 16 entries with reserved-key protection.off,warn, andenforcepolicy underlabels_config.policy[].COMPLETED.workflows.labels JSONB, a genericjsonb_opsGIN index, and schema coverage across pgroll, in-code bootstrap, and the test database.$indexentries retain strategic merge behavior.Split review stack
PR #1194 remains the full integration/prototype branch. Reviewable slices are based on their real prerequisites, not on this monolith:
23640ca5eb6b0012fdc3d3e9af94d03ca4e7b8c57fda6The internal B2 companion branch is
jiaenr/osmo-6501-b2-label-schemaatfb43dd12e, based onfeature/PROJ-147-operator-redesign. The verified internal B6 PPP-index commit ise1a2207269; it remains local pending explicit authorization to publish to the internal remote.Current integration head:
5bee16915.Migration behavior
jsonb_opsGIN index exists. Missing or invalid active 008 state rolls back and restarts.Current-head verification
Targeted verification through
5bee16915://deployments/charts/service/migrations:test_run_migrations//deployments/charts/service/migrations:test_run_migrations-pylint//src/utils/connectors/tests:test_workflow_label_schema//src/utils/connectors/tests:test_workflow_config//src/utils/connectors:connectors-pylint78c8e9073: 3/3 targets passed.//src/utils/job/tests:test_task_pure//src/utils/job/tests:test_task_pure-pylint//src/utils/job:job-pylint51526e180: 2/2 targets passed uncached.//test/scenarios:workflow-labels-policy-unit//test/scenarios:workflow-labels-policy-unit-pylint5bee16915: 4/4 targets passed.//src/service/core/workflow/tests:test_helpers//src/service/core/workflow:workflow-pylint//src/service/core/workflow/tests:test_workflow_labels_db//src/service/core/workflow/tests:test_workflow_labels_db-pylintEXPLAINproves planner eligibility for GIN existence/containment plus curated PPP prefix/missing-label indexes without claiming PostgreSQL will always avoid sequential scans.helm lint deployments/charts/servicepassed.run_migrations.sh; it contains no 007 migration file.jsonb_opsGIN catalog predicates.bash -n, andgit diff --checkpassed.Prior deployed prototype evidence
The earlier integration commit
035bbc43awas deployed tojiaenr-devthrough Argo CD commit492c73afdwith AMD64 service image2026.7.21.c7e79d19.jiaenr.bazel-b41b5a70. That deployment used ConfigMap mode withPPPin warn mode and passed the then-current dev OETF scenario with 70 explicit checks.The completed invalid-label workflow and durable receipt from that deployment remain historical evidence for
035bbc43a:Those results are not represented as end-to-end verification of the newer
5bee16915head. The current head still needs a new build, deployment, and OETF run.Remaining before promotion
78c8e9073, Add workflow label support #1194 CI passed backend, docs, Helm, and PR-description checks. UI type-check and lint passed, butui-buildstopped at Prettier drift in four UI files; that separate formatting fix has not been approved. CI for5bee16915is pending.No ARM OSMO service image is needed for this prototype. The AGX Orin node runs workflows, while OSMO services run on the Linux AWS cluster.
Rollout dependencies
PPPandosmo.poolin kube-state-metrics.offtowarntoenforce.Label membership is administrative metadata, not verified identity. Do not use it for billing or quota enforcement without additional controls.
Checklist