Skip to content

fix(translator): Early status push for headless Service#5827

Merged
arkodg merged 16 commits intoenvoyproxy:mainfrom
ShinyaIshitobi:validate-headless
Apr 29, 2025
Merged

fix(translator): Early status push for headless Service#5827
arkodg merged 16 commits intoenvoyproxy:mainfrom
ShinyaIshitobi:validate-headless

Conversation

@ShinyaIshitobi
Copy link
Copy Markdown
Contributor

What type of PR is this?

fix(translator): Early status push for headless Service

What this PR does / why we need it:

  • When Endpoint routing is disabled (e.g. headless Service), we now reject
    headless Services early by setting RouteConditionResolvedRefs=False with
    reason=UnsupportedSetting, preventing xDS validation failures.
  • Extracts headless detection into isHeadlessService helper.
  • Adds unit tests for isHeadlessService and validateDestinationSettings.

Which issue(s) this PR fixes:

Fixes #5708

Release Notes: No

Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
@ShinyaIshitobi ShinyaIshitobi requested a review from a team as a code owner April 26, 2025 09:21
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.30%. Comparing base (c55b814) to head (bebc4c5).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5827      +/-   ##
==========================================
- Coverage   65.31%   65.30%   -0.01%     
==========================================
  Files         221      221              
  Lines       35314    35325      +11     
==========================================
+ Hits        23064    23069       +5     
- Misses      10825    10831       +6     
  Partials     1425     1425              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
zhaohuabing
zhaohuabing previously approved these changes Apr 27, 2025
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

zirain
zirain previously approved these changes Apr 27, 2025
@zirain
Copy link
Copy Markdown
Member

zirain commented Apr 27, 2025

/retest

@ShinyaIshitobi
Copy link
Copy Markdown
Contributor Author

Thank you for review!
Is there anything else I need to do on my end?

Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
@zirain zirain force-pushed the validate-headless branch from dbdb8a1 to 2a0ecf1 Compare April 28, 2025 02:01
@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Apr 28, 2025

Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
@ShinyaIshitobi ShinyaIshitobi dismissed stale reviews from zhaohuabing and zirain via 8693921 April 29, 2025 08:08
@ShinyaIshitobi
Copy link
Copy Markdown
Contributor Author

@arkodg

I added two test cases:

  • Headless service with routingType=Endpoint
    • should be created successfully
  • Headless service with routingType=Service
    • with this PR, the status is pushed

Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
shawnh2
shawnh2 previously approved these changes Apr 29, 2025
Copy link
Copy Markdown
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

lgtm

case resource.KindService, resource.KindServiceImport:
if endpointRoutingDisabled && isHeadlessService(destinationSettings) {
return status.NewRouteStatusError(
fmt.Errorf("service %s is headless Service, please set routingType=Endpoint", destinationSettings.Name),
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.

Suggested change
fmt.Errorf("service %s is headless Service, please set routingType=Endpoint", destinationSettings.Name),
fmt.Errorf("service %s is a headless Service, please set routingType=Endpoint", destinationSettings.Name),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: ShinyaIshitobi <shinya.ishitobi@gmail.com>
@arkodg arkodg merged commit 5528bff into envoyproxy:main Apr 29, 2025
24 of 25 checks passed
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.

xDS Updates are failing with Ray Services

5 participants