fix(runtime): preserve event channel metadata compatibility [DYN-3846] - #12708
Conversation
Signed-off-by: PeaBrane <yanrpei@gmail.com>
WalkthroughKubernetes discovery metadata now converts scoped event channels to legacy namespace/component fields during CR construction and restores ChangesEvent metadata compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/runtime/src/discovery/kube/crd.rs (1)
343-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct coverage for legacy namespace scope.
Add a legacy fixture with
"component": "". Assert thatdeserialize_metadatacreatesEventScope::Namespace. This covers the branch at Line 140 and protects the v1.2 compatibility mapping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/runtime/src/discovery/kube/crd.rs` around lines 343 - 372, Add a legacy event-channel fixture in the existing deserialization test using an empty "component" value, then call deserialize_metadata and assert the resulting DiscoveryInstance::EventChannel uses EventScope::Namespace with the expected namespace, topic, instance_id, and transport. Keep the existing component-scoped assertion and ensure the new case exercises the legacy namespace mapping branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/runtime/src/discovery/kube/crd.rs`:
- Around line 343-372: Add a legacy event-channel fixture in the existing
deserialization test using an empty "component" value, then call
deserialize_metadata and assert the resulting DiscoveryInstance::EventChannel
uses EventScope::Namespace with the expected namespace, topic, instance_id, and
transport. Keep the existing component-scoped assertion and ensure the new case
exercises the legacy namespace mapping branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b4948e75-1da3-484d-b072-ca07653bca6f
📒 Files selected for processing (2)
lib/runtime/src/discovery/kube/crd.rslib/runtime/src/discovery/kube/daemon.rs
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Summary
EventChannelmetadata by reconstructing its legacynamespace/componentfields as the current internal scope before DWM deserialization.namespace/componentfields alongside the currentscopeso v1.2 frontends can deserialize DWM records from current workers.Root cause
#11841 replaced the serialized event-channel
namespace/componentfields withscope. During a rolling upgrade, either side treats the other version's required field as missing, and one incompatible event channel rejects the worker's entireDynamoWorkerMetadatarecord.This patch confines the compatibility projection to the Kubernetes DWM boundary. Legacy channels remain component-scoped when read by current code; v1.2 workers cannot provide the newer endpoint-scoped
event_sourcesrecovery membership, and this patch does not synthesize that attribution.Validation
cargo test -p dynamo-runtime discovery::kube::crd::testsSummary by CodeRabbit