Skip to content

fix(raven): prevent stale gateway public Service/Endpoints on failover and multi-replica - #2762

Open
WorrierKhushal wants to merge 2 commits into
openyurtio:masterfrom
WorrierKhushal:fix/2760-gateway-public-service
Open

fix(raven): prevent stale gateway public Service/Endpoints on failover and multi-replica#2762
WorrierKhushal wants to merge 2 commits into
openyurtio:masterfrom
WorrierKhushal:fix/2760-gateway-public-service

Conversation

@WorrierKhushal

@WorrierKhushal WorrierKhushal commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind bug

What this PR does / why we need it:

The GatewayPublicService controller (pkg/yurtmanager/controller/raven/gatewaypublicservice/gateway_public_service_controller.go) derived Service/Endpoints object names only from (gatewayName, type), e.g. x-raven-proxy-svc-<gw>, while the classify logic (classifyService/classifyEndpoints) keyed objects by (nodeName, type). This mismatch meant:

  • On failover (active endpoint moves nodeA -> nodeB), the reconcile tried to Create a Service for nodeB with the same name as the still-existing nodeA Service, hit AlreadyExists, and the code returned nil before the delete step for the stale nodeA object ever ran. The stale Service/Endpoints (pointing at the dead node) persisted forever, and reconcile kept reporting success.
  • With ProxyConfig.Replicas: 2 (which the Gateway webhook explicitly allows for >=2 proxy endpoints), only the first endpoint's Service/Endpoints were ever created — the second Create hit the same AlreadyExists short-circuit and was silently dropped.

This PR:

  • Includes the active endpoint's NodeName in the generated Service/Endpoints object names, so names uniquely match the classify key and no longer collide across nodes.
  • Changes the AlreadyExists handling in manageService and manageEndpoints to reconcile the object as an update instead of aborting the reconcile, so the delete step for genuinely stale objects still runs.
  • Adds TestReconcileService_TwoReplicas and TestReconcileService_Failover to cover both previously-broken scenarios with deterministic assertions (replacing the old err == nil-only check that passed even though only one endpoint was ever exposed).

Verified locally with:
go test ./pkg/yurtmanager/controller/raven/gatewaypublicservice/... -v
go test -race ./pkg/yurtmanager/controller/raven/gatewaypublicservice/... -v
go vet ./pkg/yurtmanager/controller/raven/...
golangci-lint run -v ./pkg/yurtmanager/controller/raven/gatewaypublicservice/...

All clean — 0 test failures, 0 vet issues, 0 lint issues.

Which issue(s) this PR fixes:

Fixes #2760

Special notes for your reviewer:

/assign @zyjhtangtang
corev1.Endpoints is used throughout this controller (not introduced by this PR) and is flagged by staticcheck (SA1019) as deprecated in favor of discoveryv1.EndpointSlice. Migrating to EndpointSlice is a much larger, separate refactor — this PR follows the existing repo convention of suppressing that specific warning with //nolint:staticcheck // SA1019: ..., matching the same directive already present in the production file and sibling test files in this package.

FormatName appends a random hash suffix to object names, so the new tests locate Services/Endpoints via the LabelCurrentGatewayEndpoints label rather than predicting exact names.

Does this PR introduce a user-facing change?

NONE

fix(raven): the public LoadBalancer Service/Endpoints for a raven Gateway are now correctly recreated on failover and correctly created for all active endpoints when ProxyConfig.Replicas > 1, instead of silently pointing at a dead node or exposing only one endpoint.

other Note

NONE

@WorrierKhushal
WorrierKhushal requested a review from a team as a code owner August 19, 2026 21:36
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.54%. Comparing base (ddf22f7) to head (f8bf3a7).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2762      +/-   ##
==========================================
+ Coverage   46.20%   46.54%   +0.34%     
==========================================
  Files         405      405              
  Lines       27540    27567      +27     
==========================================
+ Hits        12724    12831     +107     
+ Misses      13649    13573      -76     
+ Partials     1167     1163       -4     
Flag Coverage Δ
unittests 46.54% <100.00%> (+0.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@sonarqubecloud

Copy link
Copy Markdown

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] Raven GatewayPublicService fails to update on failover or multiple replicas

1 participant