-
Notifications
You must be signed in to change notification settings - Fork 3k
feat: add manual discovery upstream status #6315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Aias00
wants to merge
16
commits into
master
Choose a base branch
from
feat/upstream-manual-status
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
911f043
feat: add manual discovery upstream status
Aias00 1041025
Update shenyu-admin/src/main/resources/mappers/discovery-upstream-sql…
Aias00 a0e40b2
Update shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/U…
Aias00 9648ac5
Merge branch 'master' into feat/upstream-manual-status
Aias00 e74205b
fix: add discovery upstream manual status ddl
Aias00 f111247
Merge branch 'master' into feat/upstream-manual-status
Aias00 315cec2
fix discovery upstream manual status fallback
Aias00 59ada22
Merge branch 'master' into feat/upstream-manual-status
Aias00 ae77e1e
Merge branch 'master' into feat/upstream-manual-status
Aias00 1722504
Merge branch 'master' into feat/upstream-manual-status
Aias00 54bcd99
Merge branch 'master' into feat/upstream-manual-status
Aias00 8993aca
Merge branch 'master' into feat/upstream-manual-status
Aias00 53b999b
Merge branch 'master' into feat/upstream-manual-status
Aias00 ba2aad1
Merge branch 'master' into feat/upstream-manual-status
Aias00 cbb516e
Merge branch 'master' into feat/upstream-manual-status
Aias00 47a3ff8
Merge branch 'master' into feat/upstream-manual-status
Aias00 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
88 changes: 88 additions & 0 deletions
88
docs/superpowers/plans/2026-04-01-upstream-manual-status.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| # Upstream Manual Status Implementation Plan | ||
|
|
||
| > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. | ||
|
|
||
| **Goal:** Add persisted manual upstream offline control and make Admin, sync payloads, and Gateway selection honor it end to end. | ||
|
|
||
| **Architecture:** Introduce a shared `UpstreamManualStatusEnum`, persist it on `discovery_upstream`, update Admin service/controller flows to publish sync events after manual changes, and carry the new field through sync DTOs into Gateway cache objects where load-balancer selection filters forced-offline upstreams. | ||
|
|
||
| **Tech Stack:** Java, Spring MVC, MyBatis, Maven, JUnit 5, Mockito | ||
|
|
||
| --- | ||
|
|
||
| ### Task 1: Add Failing Admin Tests | ||
|
|
||
| **Files:** | ||
| - Modify: `shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryUpstreamServiceTest.java` | ||
| - Modify: `shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SyncDataServiceTest.java` | ||
| - Test: `shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DiscoveryUpstreamServiceTest.java` | ||
|
|
||
| - [ ] **Step 1: Write failing tests for manual status update and status short-circuit** | ||
| - [ ] **Step 2: Write failing assertions that sync payload exposes `manualStatus`** | ||
| - [ ] **Step 3: Run admin tests to verify they fail for missing field and behavior** | ||
| - [ ] **Step 4: Keep failures focused on the new contract** | ||
|
|
||
| ### Task 2: Add Failing Gateway Tests | ||
|
|
||
| **Files:** | ||
| - Modify: `shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/factory/LoadBalancerFactoryTest.java` | ||
| - Modify: `shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/test/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandlerTest.java` | ||
| - Test: `shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/factory/LoadBalancerFactoryTest.java` | ||
|
|
||
| - [ ] **Step 1: Add a failing load-balancer test that excludes `FORCE_OFFLINE` upstreams** | ||
| - [ ] **Step 2: Add a failing divide handler test that maps sync payload `manualStatus` into cached upstreams** | ||
| - [ ] **Step 3: Run targeted gateway tests to verify red state** | ||
|
|
||
| ### Task 3: Implement Shared Enum And DTO Changes | ||
|
|
||
| **Files:** | ||
| - Create: `shenyu-common/src/main/java/org/apache/shenyu/common/enums/UpstreamManualStatusEnum.java` | ||
| - Modify: `shenyu-common/src/main/java/org/apache/shenyu/common/dto/DiscoveryUpstreamData.java` | ||
| - Modify: `shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java` | ||
|
|
||
| - [ ] **Step 1: Add the shared enum with `NONE` and `FORCE_OFFLINE`** | ||
| - [ ] **Step 2: Extend sync DTO and cached upstream entity with `manualStatus`** | ||
| - [ ] **Step 3: Keep defaults backward compatible with `NONE`** | ||
|
|
||
| ### Task 4: Implement Admin Persistence And API | ||
|
|
||
| **Files:** | ||
| - Modify: `shenyu-admin/src/main/resources/sql-script/h2/schema.sql` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/DiscoveryUpstreamDO.java` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/DiscoveryUpstreamDTO.java` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/model/vo/DiscoveryUpstreamVO.java` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/DiscoveryUpstreamMapper.java` | ||
| - Modify: `shenyu-admin/src/main/resources/mappers/discovery-upstream-sqlmap.xml` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/DiscoveryTransfer.java` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/service/DiscoveryUpstreamService.java` | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DiscoveryUpstreamServiceImpl.java` | ||
| - Create: `shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/UpstreamManualStatusDTO.java` | ||
| - Create: `shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/UpstreamController.java` | ||
|
|
||
| - [ ] **Step 1: Persist `manual_status` and map it through DO/DTO/VO/Mapper** | ||
| - [ ] **Step 2: Add service methods to change manual status and publish fresh discovery events** | ||
| - [ ] **Step 3: Add `/upstream/offline` and `/upstream/online` controller endpoints** | ||
|
|
||
| ### Task 5: Implement Heartbeat Short-Circuit And Gateway Filtering | ||
|
|
||
| **Files:** | ||
| - Modify: `shenyu-admin/src/main/java/org/apache/shenyu/admin/service/register/AbstractShenyuClientRegisterServiceImpl.java` | ||
| - Modify: `shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/handler/DivideUpstreamDataHandler.java` | ||
| - Modify: `shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-websocket/src/main/java/org/apache/shenyu/plugin/websocket/handler/WebSocketUpstreamDataHandler.java` | ||
| - Modify: `shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-rpc/shenyu-plugin-grpc/src/main/java/org/apache/shenyu/plugin/grpc/handler/GrpcDiscoveryUpstreamDataHandler.java` | ||
| - Modify: `shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/factory/LoadBalancerFactory.java` | ||
|
|
||
| - [ ] **Step 1: Prevent alive/status recovery when the DB record is `FORCE_OFFLINE`** | ||
| - [ ] **Step 2: Map synced `manualStatus` into plugin-specific upstream cache objects** | ||
| - [ ] **Step 3: Filter forced-offline upstreams before selection** | ||
|
|
||
| ### Task 6: Verify Green State | ||
|
|
||
| **Files:** | ||
| - Modify: `docs/superpowers/specs/2026-04-01-upstream-manual-status-design.md` | ||
| - Modify: `docs/superpowers/plans/2026-04-01-upstream-manual-status.md` | ||
|
|
||
| - [ ] **Step 1: Run targeted Maven tests for admin, loadbalancer, and divide modules** | ||
| - [ ] **Step 2: Run a focused compile if any cross-module breakage appears** | ||
| - [ ] **Step 3: Review git diff for unintended changes** | ||
| - [ ] **Step 4: Commit with one feature commit** |
92 changes: 92 additions & 0 deletions
92
docs/superpowers/specs/2026-04-01-upstream-manual-status-design.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| # Upstream Manual Status Design | ||
|
|
||
| ## Goal | ||
|
|
||
| Add a persisted manual upstream control flag that lets Admin force a discovery upstream offline without being overwritten by heartbeat recovery, and make Gateway honor that flag during upstream selection. | ||
|
|
||
| ## Background | ||
|
|
||
| Today `discovery_upstream.upstream_status` is used for automatic liveness. Admin-triggered manual offline and automatic health recovery share the same status channel, so a heartbeat or recovery path can bring a manually disabled upstream back into traffic. | ||
|
|
||
| ## Chosen Approach | ||
|
|
||
| Use a separate manual status field. | ||
|
|
||
| - Persist `manual_status` on `discovery_upstream` with default `NONE`. | ||
| - Represent manual control with a shared enum `NONE` and `FORCE_OFFLINE`. | ||
| - Keep `upstream_status` for automatic health only. | ||
| - Make Admin manual APIs write only `manualStatus`. | ||
| - Let heartbeat or recovery logic skip `status=true` updates when `manualStatus == FORCE_OFFLINE`. | ||
| - Include `manualStatus` in discovery sync payloads and Gateway cache objects. | ||
| - Filter `FORCE_OFFLINE` upstreams before load-balancer selection. | ||
|
|
||
| This keeps automatic and manual state independent and avoids hidden coupling. | ||
|
|
||
| ## Alternatives Considered | ||
|
|
||
| ### Reuse `upstream_status` | ||
|
|
||
| Rejected because heartbeat and health check would continue to overwrite manual operations. | ||
|
|
||
| ### Keep manual state only in Gateway memory | ||
|
|
||
| Rejected because it would not survive restarts or sync across Admin and Gateway nodes. | ||
|
|
||
| ## Data Model | ||
|
|
||
| Add `manual_status varchar(32) not null default 'NONE'` to `discovery_upstream`. | ||
|
|
||
| Shared enum: | ||
|
|
||
| - `NONE` | ||
| - `FORCE_OFFLINE` | ||
|
|
||
| `/upstream/online` resets the field to `NONE`. | ||
|
|
||
| ## Admin API | ||
|
|
||
| Add a new Admin controller rooted at `/upstream` with: | ||
|
|
||
| - `POST /upstream/offline` | ||
| - `POST /upstream/online` | ||
|
|
||
| Request body will identify the upstream by `selectorId` and `url`. | ||
|
|
||
| Behavior: | ||
|
|
||
| - Look up the related discovery handler by selector id. | ||
| - Update only `manual_status`. | ||
| - Publish a fresh `DISCOVER_UPSTREAM` event built from current DB data so gateways receive the new flag immediately. | ||
|
|
||
| ## Status Update Rules | ||
|
|
||
| Automatic writers keep their current responsibility for `upstream_status`. | ||
|
|
||
| Additional rule: | ||
|
|
||
| - If a write intends to mark an upstream alive (`status=true`) and the record is `FORCE_OFFLINE`, skip the status update. | ||
|
|
||
| This protects the manual offline decision from heartbeat recovery without blocking automatic offline transitions. | ||
|
|
||
| ## Sync Contract | ||
|
|
||
| Extend `DiscoveryUpstreamData` and all transfer paths to include `manualStatus`. | ||
|
|
||
| Admin event producers and Gateway sync consumers will continue to use the same payload shape, now with one extra field. | ||
|
|
||
| ## Gateway Behavior | ||
|
|
||
| Extend cached upstream objects with `manualStatus`. | ||
|
|
||
| Gateway will filter out `FORCE_OFFLINE` upstreams before selection. This ensures: | ||
|
|
||
| - Manually offline nodes are never chosen. | ||
| - Existing health-check metadata can still be retained. | ||
| - Re-enabling an upstream only requires Admin to push a new sync event with `manualStatus=NONE`. | ||
|
|
||
| ## Testing Strategy | ||
|
|
||
| - Admin service tests for manual status update and status recovery short-circuit. | ||
| - Sync/transfer tests for `manualStatus` propagation. | ||
| - Load-balancer tests for filtering `FORCE_OFFLINE`. | ||
| - Divide discovery handler test for mapping sync payload to cached upstream manual status. |
71 changes: 71 additions & 0 deletions
71
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/UpstreamController.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.shenyu.admin.controller; | ||
|
|
||
| import jakarta.validation.Valid; | ||
| import org.apache.shenyu.admin.aspect.annotation.RestApi; | ||
| import org.apache.shenyu.admin.model.dto.UpstreamManualStatusDTO; | ||
| import org.apache.shenyu.admin.model.result.ShenyuAdminResult; | ||
| import org.apache.shenyu.admin.service.DiscoveryUpstreamService; | ||
| import org.apache.shenyu.admin.utils.ShenyuResultMessage; | ||
| import org.apache.shenyu.common.enums.UpstreamManualStatusEnum; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
|
|
||
| /** | ||
| * Upstream controller. | ||
| */ | ||
| @RestApi("/upstream") | ||
| public class UpstreamController { | ||
|
|
||
| private final DiscoveryUpstreamService discoveryUpstreamService; | ||
|
|
||
| public UpstreamController(final DiscoveryUpstreamService discoveryUpstreamService) { | ||
| this.discoveryUpstreamService = discoveryUpstreamService; | ||
| } | ||
|
|
||
| /** | ||
| * manual offline. | ||
| * | ||
| * @param upstreamManualStatusDTO upstream request | ||
| * @return result | ||
| */ | ||
| @PostMapping("/offline") | ||
| public ShenyuAdminResult offline(@Valid @RequestBody final UpstreamManualStatusDTO upstreamManualStatusDTO) { | ||
| discoveryUpstreamService.changeManualStatusBySelectorIdAndUrl( | ||
| upstreamManualStatusDTO.getSelectorId(), | ||
| upstreamManualStatusDTO.getUrl(), | ||
| UpstreamManualStatusEnum.FORCE_OFFLINE); | ||
| return ShenyuAdminResult.success(ShenyuResultMessage.UPDATE_SUCCESS); | ||
| } | ||
|
|
||
| /** | ||
| * manual online. | ||
| * | ||
| * @param upstreamManualStatusDTO upstream request | ||
| * @return result | ||
| */ | ||
| @PostMapping("/online") | ||
| public ShenyuAdminResult online(@Valid @RequestBody final UpstreamManualStatusDTO upstreamManualStatusDTO) { | ||
| discoveryUpstreamService.changeManualStatusBySelectorIdAndUrl( | ||
| upstreamManualStatusDTO.getSelectorId(), | ||
| upstreamManualStatusDTO.getUrl(), | ||
| UpstreamManualStatusEnum.NONE); | ||
| return ShenyuAdminResult.success(ShenyuResultMessage.UPDATE_SUCCESS); | ||
| } | ||
| } |
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
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
74 changes: 74 additions & 0 deletions
74
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/dto/UpstreamManualStatusDTO.java
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,74 @@ | ||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||||||||||||||||||||
| * contributor license agreements. See the NOTICE file distributed with | ||||||||||||||||||||||||||||||
| * this work for additional information regarding copyright ownership. | ||||||||||||||||||||||||||||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||||||||||||||||||||
| * (the "License"); you may not use this file except in compliance with | ||||||||||||||||||||||||||||||
| * the License. You may obtain a copy of the License at | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| package org.apache.shenyu.admin.model.dto; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.NotBlank; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Manual upstream status request. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| public class UpstreamManualStatusDTO { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * selector id. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| @NotBlank(message = "selectorId can't be null") | ||||||||||||||||||||||||||||||
| private String selectorId; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * upstream url. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| @NotBlank(message = "url can't be null") | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| @NotBlank(message = "selectorId can't be null") | |
| private String selectorId; | |
| /** | |
| * upstream url. | |
| */ | |
| @NotBlank(message = "url can't be null") | |
| @NotBlank(message = "selectorId can't be null or blank") | |
| private String selectorId; | |
| /** | |
| * upstream url. | |
| */ | |
| @NotBlank(message = "url can't be null or blank") |
Aias00 marked this conversation as resolved.
Outdated
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This setter treats an empty string as a provided value (it will be normalized, likely to
NONE), while other parts of the service useStringUtils.hasLength(...)to detect 'not provided' and preserve existing DB state. To keep request semantics consistent, consider treating blank/empty asnullhere as well (so callers can omit the field without accidentally overwriting toNONE).