Skip to content

test(operator): strengthen GMS ResourceClaim name regression coverage - #12218

Merged
julienmancuso merged 4 commits into
mainfrom
jsm/ops-7051
Jul 29, 2026
Merged

test(operator): strengthen GMS ResourceClaim name regression coverage#12218
julienmancuso merged 4 commits into
mainfrom
jsm/ops-7051

Conversation

@julienmancuso

@julienmancuso julienmancuso commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Backfills and strengthens unit-test coverage for NVBug 6198276 / DYN-3083.

The original bug allowed mixed-case Dynamo service names such as VllmWorker to propagate into GMS ResourceClaimTemplate and resource-sharing names. Grove later used those values in Kubernetes resource names, which failed RFC 1123 validation and prevented PCSG synchronization.

The production fix was delivered in #9829 and backported in #9834. This MR adds broader regression coverage around that behavior by:

  • Converting TestGmsRCTName into a table-driven test.
  • Covering already-normalized, lowercase, CamelCase, uppercase, and dotted service names.
  • Verifying both the exact normalized output and RFC 1123 compliance.
  • Exercising mixed-case names through the multinode gmsResourceClaimTemplateConfigs path.
  • Exercising mixed-case names through the multinode gmsResourceSharingEntries path.
  • Validating the generated name for every GMS rank.

This is a test-only change and does not modify operator behavior.

Related Issues

Closes OPS-7051


Open in Devin Review

Summary by CodeRabbit

  • Tests
    • Expanded coverage for service-name normalization across lowercase, uppercase, camel-case, and dot-containing names.
    • Added validation that generated resource names comply with RFC1123 standards.
    • Confirmed GPU request settings remain unchanged in multinode configurations.
    • Added checks ensuring rank-specific resource-sharing entries reference the correct cliques.

@julienmancuso
julienmancuso requested a review from a team as a code owner July 27, 2026 20:27
@github-actions github-actions Bot added test deployment::k8s Relates to dynamo deployment in kubernetes labels Jul 27, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4147bde1-0698-48bb-b12c-46aa44ac8f61

📥 Commits

Reviewing files that changed from the base of the PR and between 16ff7d2 and 3911df2.

📒 Files selected for processing (1)
  • deploy/operator/internal/dynamo/failover_test.go

Walkthrough

Changes

GMS test coverage

Layer / File(s) Summary
Name normalization contract
deploy/operator/internal/dynamo/failover_test.go
Table-driven cases cover normalized, lowercase, camel-case, uppercase, and dotted service names while checking RFC1123 compliance.
Multinode configuration validation
deploy/operator/internal/dynamo/failover_test.go
Multinode tests use a mixed-case service name and validate normalized names, preserved GPU configuration, logging, and rank-specific clique filters.

Estimated code review effort: 3 (Moderate) | ~15 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: expanded regression coverage for GMS ResourceClaim name normalization.
Description check ✅ Passed The description covers the change summary and linked issue, but it doesn't follow the template headings for Overview, Details, or reviewer start.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

@datadog-official

This comment has been minimized.

@julienmancuso
julienmancuso enabled auto-merge (squash) July 29, 2026 16:42
@julienmancuso
julienmancuso merged commit 481b34a into main Jul 29, 2026
97 of 98 checks passed
@julienmancuso
julienmancuso deleted the jsm/ops-7051 branch July 29, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment::k8s Relates to dynamo deployment in kubernetes size/M test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants