Skip to content

fix(sts-reconciler): use compatible headless service name for statefu…#81

Merged
cheyang merged 1 commit intosgl-project:mainfrom
TrafalgarZZZ:bugfix/fix_issue_80
Oct 30, 2025
Merged

fix(sts-reconciler): use compatible headless service name for statefu…#81
cheyang merged 1 commit intosgl-project:mainfrom
TrafalgarZZZ:bugfix/fix_issue_80

Conversation

@TrafalgarZZZ
Copy link
Copy Markdown
Collaborator

Ⅰ. Motivation

Ⅱ. Modifications

  • Update StatefulSet reconciliation to use a compatible headless service name
  • Add error handling for cases where a compatible service name cannot be found
  • This change ensures compatibility with headless services in various environments

Ⅲ. Does this pull request fix one issue?

fixes #80

Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅴ. Describe how to verify it

VI. Special notes for reviews

Checklist

  • Format your code make fmt.
  • Add unit tests or integration tests.
  • Update the documentation related to the change.

…lsets

- Update StatefulSet reconciliation to use a compatible headless service name
- Add error handling for cases where a compatible service name cannot be found
- This change ensures compatibility with headless services in various environments

Signed-off-by: trafalgarzzz <[email protected]>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@RongGu RongGu requested a review from Copilot October 30, 2025 06:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the StatefulSet service name configuration to use a compatible headless service name instead of the workload name. This ensures consistency across the codebase for service naming, particularly when services need to comply with DNS naming conventions.

  • Uses GetCompatibleHeadlessServiceName to determine the correct service name for StatefulSet configuration
  • Replaces hardcoded rbg.GetWorkloadName(role) with the dynamically determined svcName

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

WithSpec(
appsapplyv1.StatefulSetSpec().
WithServiceName(rbg.GetWorkloadName(role)).
// WithServiceName(rbg.GetWorkloadName(role)).
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Remove the commented-out code. Commented-out code should not be left in production as it reduces code clarity and can cause confusion.

Suggested change
// WithServiceName(rbg.GetWorkloadName(role)).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@cheyang cheyang merged commit 3e66f7d into sgl-project:main Oct 30, 2025
9 checks passed
@cheyang cheyang added this to the v0.5.0 milestone Oct 30, 2025
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.

[Bug] Headless svc name is not consistent in statefulsets created by RBG

4 participants