fix 模块 pod 替换导致 JVM 进程内模块实例消失 #45
Conversation
WalkthroughAdds a BizModelVersion field to BizOperationResponse and threads a podKey into StartBiz/StopBiz for HTTP and MQTT tunnels; MQTT now wraps install/uninstall payloads in Ark service request types and logs include bizModelVersion. go.mod dependency versions updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Controller
participant Tunnel as HTTP/MQTT Tunnel
participant Ark as Ark Service
Controller->>Tunnel: StartBiz(nodeName, podKey, container)
Note right of Tunnel #dff0d8: Build BizModel with BizModelVersion = podKey
alt MQTT Tunnel
Tunnel->>Ark: InstallBiz(InstallBizRequest{ BizModel })
else HTTP Tunnel
Tunnel->>Ark: InstallBiz(BizModel)
end
Ark-->>Tunnel: BizOperationResponse (bizModelVersion)
Tunnel-->>Controller: return success/error
sequenceDiagram
autonumber
actor Controller
participant Tunnel as HTTP/MQTT Tunnel
participant Ark as Ark Service
Controller->>Tunnel: StopBiz(nodeName, podKey, container)
Note right of Tunnel #f0f7ff: BizModelVersion = podKey
alt MQTT Tunnel
Tunnel->>Ark: UninstallBiz(UninstallBizRequest{ BizModel })
else HTTP Tunnel
Tunnel->>Ark: UninstallBiz(BizModel)
end
Ark-->>Tunnel: BizOperationResponse (bizModelVersion)
Tunnel-->>Controller: return success/error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)module_tunnels/koupleless_http_tunnel/http_tunnel.go (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
module_tunnels/koupleless_mqtt_tunnel/mqtt_tunnel.go (1)
305-364: Guard against stale MQTT operation responses.Other MQTT callbacks drop messages older than 10s, but bizOperationResponseCallback does not. Add the same expiry check to prevent late uninstall/failed install responses from flipping state after a pod replacement.
Apply:
err := json.Unmarshal(msg.Payload(), &data) if err != nil { zlogger.Error(err, "Error unmarshalling biz response") return } + // Drop stale operation responses (aligns with other MQTT handlers) + if utils.Expired(data.PublishTimestamp, 1000*10) { + return + }
🧹 Nitpick comments (5)
common/model/model.go (1)
34-38: Layering: common/model depends on module_tunnels/…/ark_service.Common models importing an HTTP-tunnel subpackage is an inversion of dependency flow and risks cycles. Consider moving ArkResponse into a neutral package (e.g., common/arkproto or common/arkservice) and have both tunnels depend on that.
module_tunnels/koupleless_mqtt_tunnel/mqtt_tunnel.go (4)
18-19: Package boundary: importing ark_service from HTTP tunnel leaks transport-specific model into MQTT.Move the request wrappers (InstallBizRequest/UninstallBizRequest) to a transport-neutral location (e.g., common/arkproto) so both tunnels depend “upwards,” not sideways.
434-444: Marshal error is ignored; handle and surface it.If JSON marshal fails, the publish will send an empty payload silently. Log and return the error.
Apply:
- installBizRequestBytes, _ := json.Marshal(ark_service.InstallBizRequest{ - BizModel: bizModel, - }) + req := ark_service.InstallBizRequest{BizModel: bizModel} + installBizRequestBytes, err := json.Marshal(req) + if err != nil { + zlogger.Error(err, "failed to marshal InstallBizRequest") + return err + }
446-456: Same marshal‑error handling for uninstall.Apply:
- unInstallBizRequestBytes, _ := json.Marshal(ark_service.UninstallBizRequest{ - BizModel: bizModel, - }) + req := ark_service.UninstallBizRequest{BizModel: bizModel} + unInstallBizRequestBytes, err := json.Marshal(req) + if err != nil { + zlogger.Error(err, "failed to marshal UninstallBizRequest") + return err + }
439-441: Note: json:",inline" on embedded request types has no effect with encoding/json.The wrappers embed BizModel, which already flattens fields for JSON. The
,inlinetag is a YAML concept and is ignored by encoding/json. Consider removing it in ark_service/model.go to avoid confusion.Would you like me to open a small follow‑up PR to adjust the wrapper tags?
Also applies to: 449-451
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
common/model/model.go(1 hunks)module_tunnels/koupleless_http_tunnel/http_tunnel.go(4 hunks)module_tunnels/koupleless_mqtt_tunnel/mqtt_tunnel.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
common/model/model.go (1)
module_tunnels/koupleless_http_tunnel/ark_service/model.go (1)
ArkResponse(16-31)
module_tunnels/koupleless_http_tunnel/http_tunnel.go (3)
common/zaplogger/logger.go (1)
FromContext(19-26)common/model/model.go (1)
BizOperationResponse(33-39)common/model/consts.go (2)
CommandInstallBiz(71-71)CommandUnInstallBiz(73-73)
module_tunnels/koupleless_mqtt_tunnel/mqtt_tunnel.go (5)
common/utils/utils.go (2)
TranslateCoreV1ContainerToBizModel(58-64)FormatArkletCommandTopic(36-38)common/zaplogger/logger.go (1)
FromContext(19-26)module_tunnels/koupleless_http_tunnel/ark_service/model.go (2)
InstallBizRequest(6-8)UninstallBizRequest(11-13)common/model/consts.go (1)
CommandInstallBiz(71-71)module_tunnels/koupleless_mqtt_tunnel/mqtt/mqtt.go (1)
Qos0(19-19)
🔇 Additional comments (4)
module_tunnels/koupleless_http_tunnel/http_tunnel.go (4)
338-348: Correctly propagating podKey into BizModel and response.Setting BizModel.BizModelVersion and echoing it in BizOperationResponse aligns controller state with pod identity.
362-362: API surface: StopBiz second arg renamed to podKey; consistent with StartBiz.
372-381: Stop path also propagates BizModelVersion; good.Mirrors the start flow and keeps logs correlated.
327-327: API surface: StartBiz now clearly names the second arg podKey.
- Signature is type-compatible with Tunnel.
- grep results: only definitions found in
- module_tunnels/koupleless_http_tunnel/http_tunnel.go (StartBiz/StopBiz)
- module_tunnels/koupleless_mqtt_tunnel/mqtt_tunnel.go (StartBiz/StopBiz; mqtt assigns bizModel.BizModelVersion = podKey)
- No other callers found in this repo — verify all callers (including external callers/repos) pass a non-empty podKey.
| Command string `json:"command"` // Operation command executed | ||
| BizName string `json:"bizName"` // ClusterName of the business | ||
| BizVersion string `json:"bizVersion"` // Version of the business | ||
| BizModelVersion string `json:"bizModelVersion"` // Pod key (podKey) used for aligning module state with Kubernetes pod state to prevent old uninstall requests from removing new modules. Format: {bizName}-{template}-{randomString} (e.g., biz1-web-single-host-786dfc476f-x2hwk) | ||
| Response ark_service.ArkResponse[ark.ArkResponseData] `json:"response"` // Response from ark service |
There was a problem hiding this comment.
🧩 Analysis chain
BizModelVersion addition looks correct; consider making it optional and verify upstream type alignment.
- Good addition to carry podKey across the wire.
- Suggest tag
omitemptyto avoid emitting empty values when older producers don’t populate it. - Please verify arkctl’s ark.BizModel includes
BizModelVersionso assignments in tunnels compile and serialize as expected.
Apply:
- BizModelVersion string `json:"bizModelVersion"` // Pod key (podKey) used for aligning module state with Kubernetes pod state to prevent old uninstall requests from removing new modules. Format: {bizName}-{template}-{randomString} (e.g., biz1-web-single-host-786dfc476f-x2hwk)
+ BizModelVersion string `json:"bizModelVersion,omitempty"` // Pod key (podKey) used for aligning module state with Kubernetes pod state to prevent old uninstall requests from removing new modules. Format: {bizName}-{template}-{randomString} (e.g., biz1-web-single-host-786dfc476f-x2hwk)To double‑check usage and upstream type availability:
Optionally confirm upstream library shape via web:
🏁 Script executed:
#!/bin/bash
# Find all assignments/usages to BizModelVersion across the repo
rg -nP -C2 '\bBizModelVersion\b'
# Find StartBiz/StopBiz call sites and confirm a non-empty podKey is passed
rg -nP -C3 '\b(StartBiz|StopBiz)\s*\('Length of output: 7764
🌐 Web query:
Does github.com/koupleless/arkctl/v1/service/ark define BizModel with a field named BizModelVersion?
💡 Result:
No. The BizModel defined in github.com/koupleless/arkctl/v1/service/ark has fields BizName, BizVersion and BizUrl — it does not define a field named BizModelVersion. [1]
Sources:
[1] pkg.go.dev reference for github.com/koupleless/arkctl/v1/service/ark (type BizModel).
Add omitempty and fix upstream type mismatch (BizModelVersion missing in ark.BizModel)
- Apply this diff to common/model/model.go:
- BizModelVersion string `json:"bizModelVersion"` // Pod key (podKey) used for aligning module state with Kubernetes pod state to prevent old uninstall requests from removing new modules. Format: {bizName}-{template}-{randomString} (e.g., biz1-web-single-host-786dfc476f-x2hwk)
+ BizModelVersion string `json:"bizModelVersion,omitempty"` // Pod key (podKey) used for aligning module state with Kubernetes pod state to prevent old uninstall requests from removing new modules. Format: {bizName}-{template}-{randomString} (e.g., biz1-web-single-host-786dfc476f-x2hwk)- Verification result: github.com/koupleless/arkctl/v1/service/ark.BizModel does NOT define BizModelVersion. Current code assigns bizModel.BizModelVersion = podKey and includes BizModelVersion in requests (see module_tunnels/koupleless_http_tunnel/http_tunnel.go and module_tunnels/koupleless_mqtt_tunnel/mqtt_tunnel.go). This creates a type/serialization mismatch — resolve by either:
- adding BizModelVersion to the upstream ark.BizModel, or
- moving the podKey into the request field(s) that ark_service expects and updating all call sites, then rebuild to confirm.
🤖 Prompt for AI Agents
In common/model/model.go around lines 34 to 38, the JSON tag for BizModelVersion
should include `omitempty` and the current inclusion of BizModelVersion creates
a type/serialization mismatch because ark.BizModel upstream does not define
BizModelVersion; fix by either (A) adding BizModelVersion to the upstream
ark.BizModel definition (matching name and type) so serialization aligns, or (B)
remove BizModelVersion from this model and move the podKey value into the
request field(s) that ark_service expects, then update all call sites
(module_tunnels/*) to use the new request field(s) and rebuild to confirm the
mismatch is resolved; also ensure the BizModelVersion JSON tag is changed to
include `omitempty` if you keep the field locally.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
module_tunnels/koupleless_http_tunnel/http_tunnel.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
module_tunnels/koupleless_http_tunnel/http_tunnel.go (2)
common/zaplogger/logger.go (1)
FromContext(19-26)common/model/model.go (1)
BizOperationResponse(33-39)
🪛 GitHub Actions: golangci-lint
module_tunnels/koupleless_http_tunnel/http_tunnel.go
[error] 328-328: golangci-lint: unnecessary leading newline (whitespace) at 328:92. Command: golangci-lint run --new-from-patch=/tmp/tmp-2119-SOgD9aRL861G/pull.patch --new=false --new-from-rev= --new-from-merge-base=
🪛 GitHub Check: lint
module_tunnels/koupleless_http_tunnel/http_tunnel.go
[failure] 328-328:
unnecessary leading newline (whitespace)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: unit-test
🔇 Additional comments (2)
module_tunnels/koupleless_http_tunnel/http_tunnel.go (2)
339-349: LGTM! podKey correctly threaded through install flow.The podKey parameter is properly assigned to
bizModel.BizModelVersionand flows through toBizOperationResponse.BizModelVersion, enabling the controller to align module state with Kubernetes pod state and prevent stale uninstall requests from removing new modules.
363-393: LGTM! podKey correctly threaded through uninstall flow.The podKey parameter is properly assigned to
bizModel.BizModelVersionand flows through toBizOperationResponse.BizModelVersion, maintaining consistency with the install flow. This enables the controller to reject stale uninstall requests that target replaced pods.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
+ Coverage 66.92% 67.13% +0.20%
==========================================
Files 12 12
Lines 1427 1436 +9
==========================================
+ Hits 955 964 +9
Misses 405 405
Partials 67 67
🚀 New features to boost your workflow:
|
2 participants
Summary by CodeRabbit
New Features
Improvements
Chores