feat: Implement Xline native watch mechanism for rks #137

Flash0615 · 2025-09-01T15:34:36Z

Summary

Replace broadcast simulation with real xline/etcd watch implementation.

Changes

Add XlineStore abstraction layer
Implement atomic snapshot + watch operations
Add comprehensive test coverage
Support proper reconnection handling

Breaking Changes

Watch API now requires xline endpoints instead of broadcast channels

Testing

Unit tests:Passing
Integration tests

Replace broadcast simulation with xline watch for pods and nodes. Signed-off-by: Haojun Xie <[email protected]>

>> Replace broadcast simulation with xline watch for pods and nodes. Signed-off-by: Haojun Xie <[email protected]>

Replace broadcast simulation with real xline/etcd watch implementation. Adds XlineStore abstraction and atomic snapshot+revision operations. Breaking change: requires xline endpoints instead of broadcast channels. Signed-off-by: Haojun Xie <[email protected]>

Copilot

Pull Request Overview

This PR replaces the broadcast-based pod watching mechanism with a native Xline/etcd watch implementation, providing more reliable and efficient state synchronization for the Kubernetes-like orchestration system.

Replace broadcast channels with atomic snapshot + watch operations using etcd client
Add comprehensive test coverage for watch operations including edge cases
Implement proper reconnection handling and cleanup mechanisms

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
project/rks/tests/test_xline.rs	Refactored test structure with shared store loading and unique pod naming
project/rks/tests/test_watch.rs	Added comprehensive watch functionality tests covering CRUD operations and edge cases
project/rks/src/server.rs	Major refactor removing broadcast channels, implementing native watch streams, improved logging
project/rks/src/network/manager.rs	Added XlineStore initialization in tests
project/rks/src/network/ip.rs	Marked flaky test as ignored
project/rks/src/main.rs	Added logger initialization
project/rks/src/commands/delete.rs	Removed broadcast dependency, simplified delete logic
project/rks/src/commands/create.rs	Enhanced error handling and removed broadcast dependency
project/rks/src/api/xlinestore.rs	Expanded API with snapshot, watch, and configuration methods
project/rks/Cargo.toml	Added futures-util and time dependencies

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

project/rks/tests/test_xline.rs

Copilot · 2025-09-02T01:24:52Z

project/rks/tests/test_watch.rs

+    Arc::new(tokio::runtime::Handle::current().block_on(async {
+        XlineStore::new(&endpoints).await.expect("connect xline failed")


Using block_on within an async context can cause deadlocks or panics. Since load_store() is called from async test functions, the XlineStore creation should remain async rather than blocking on the current runtime.

Copilot · 2025-09-02T01:24:52Z

project/rks/src/server.rs

+                                let pod_name = String::from_utf8_lossy(kv.key())
+                                    .replace("/registry/pods/", "");


This key parsing logic is fragile and could produce incorrect pod names if the key doesn't start with the expected prefix. Use proper prefix stripping: String::from_utf8_lossy(kv.key()).strip_prefix(\"/registry/pods/\").unwrap_or(\"\").to_string()

Suggested change

let pod_name = String::from_utf8_lossy(kv.key())

.replace("/registry/pods/", "");

.strip_prefix("/registry/pods/")

.unwrap_or("")

.to_string();

Copilot · 2025-09-02T01:24:52Z

project/rks/src/commands/delete.rs

 ) -> Result<()> {
-    let _ = tx.send(RksMessage::DeletePod(pod_name.clone()));
+    xline_store.delete_pod(&pod_name).await?;
+    println!("[user_delete] deleted pod {} (written to xline)",pod_name);


Missing space before the opening parenthesis. Should be: println!(\"[user_delete] deleted pod {} (written to xline)\", pod_name);

Signed-off-by: InChh <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-16T01:36:07Z

project/rks/tests/test_watch.rs

+        .iter()
+        .map(|s| s.as_str())
+        .collect();
+    Arc::new(tokio::runtime::Handle::current().block_on(async {


Using block_on within an async context can cause deadlocks or performance issues. Consider making load_store async and awaiting it in the test functions instead.

Copilot · 2025-09-16T01:36:08Z

project/rks/tests/test_watch.rs

+    let resp = timeout(Duration::from_secs(5), stream.next())
+        .await
+        .unwrap()
+        .unwrap()
+        .unwrap();


Chained .unwrap() calls make error handling difficult to debug. Consider using expect() with descriptive messages or proper error handling for better debugging experience.

Copilot · 2025-09-16T01:36:08Z

project/rks/src/server.rs

+                                if let Ok(pod_task) = serde_yaml::from_str::<PodTask>(&pod_yaml)
+                                    && pod_task.spec.node_name.as_deref() == Some(&node_id)
+                                {


[nitpick] The let-chain pattern can be hard to read. Consider breaking this into separate if let statements or using match for better readability.

Suggested change

if let Ok(pod_task) = serde_yaml::from_str::<PodTask>(&pod_yaml)

&& pod_task.spec.node_name.as_deref() == Some(&node_id)

{

if let Ok(pod_task) = serde_yaml::from_str::<PodTask>(&pod_yaml) {

if pod_task.spec.node_name.as_deref() == Some(&node_id) {

Copilot · 2025-09-16T01:36:08Z

project/rks/src/server.rs

+                        etcd_client::EventType::Delete => {
+                            if let Some(kv) = event.prev_kv() {
+                                let pod_name = String::from_utf8_lossy(kv.key())
+                                    .replace("/registry/pods/", "");


Using string replacement for key parsing is fragile. Consider using a more robust approach like splitting on '/' and taking the last component, or defining a constant for the prefix.

Suggested change

.replace("/registry/pods/", "");

.rsplit('/')

.next()

.unwrap_or("")

.to_string();

Copilot · 2025-09-16T01:36:09Z

project/rks/src/api/xlinestore.rs

@@ -1,63 +1,77 @@
-#![allow(dead_code)]
+#![allow(unused)]


Using #![allow(unused)] at the module level suppresses important warnings. Consider removing this and addressing unused code warnings individually or using more specific allows like #[allow(dead_code)] on specific items.

Suggested change

#![allow(unused)]

Copilot · 2025-09-16T01:36:09Z

project/rks/src/commands/delete.rs

@@ -1,9 +1,9 @@
+#![allow(unused)]


Using #![allow(unused)] at the module level suppresses important warnings. Consider removing this and addressing unused code warnings individually.

Suggested change

#![allow(unused)]

Copilot · 2025-09-16T01:36:09Z

project/rks/src/commands/create.rs

@@ -1,10 +1,11 @@
+#![allow(unused)]


Using #![allow(unused)] at the module level suppresses important warnings. Consider removing this and addressing unused code warnings individually.

Suggested change

#![allow(unused)]

# Conflicts: # project/rks/BUCK

Flash0615 added 3 commits September 1, 2025 17:16

refactor: implement xline-based watch mechanism

c948545

Replace broadcast simulation with xline watch for pods and nodes. Signed-off-by: Haojun Xie <[email protected]>

refactor: implement xline-based watch mechanism

a8fdf4b

>> Replace broadcast simulation with xline watch for pods and nodes. Signed-off-by: Haojun Xie <[email protected]>

genedna requested review from Copilot and tcytan September 2, 2025 01:24

Copilot AI reviewed Sep 2, 2025

View reviewed changes

fix: fix check errors

7025750

Signed-off-by: InChh <[email protected]>

genedna added rks scheduler labels Sep 15, 2025

genedna added this to the 0.1 - Node 节点运行容器基本功能 milestone Sep 15, 2025

Merge remote-tracking branch 'upstream/main' into MyBranch

d70e18f

genedna requested review from Copilot and InChh and removed request for tcytan September 16, 2025 01:34

Copilot AI reviewed Sep 16, 2025

View reviewed changes

InChh added 3 commits September 16, 2025 18:42

chore: fix test errors

bae7f20

chore: apply copilot suggestion

ddbea51

Merge remote-tracking branch 'upstream/main' into MyBranch

22762f1

# Conflicts: # project/rks/BUCK

InChh approved these changes Sep 16, 2025

View reviewed changes

InChh added this pull request to the merge queue Sep 16, 2025

Merged via the queue into rk8s-dev:main with commit 458cdc9 Sep 16, 2025
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Implement Xline native watch mechanism for rks #137

feat: Implement Xline native watch mechanism for rks #137

Uh oh!

Flash0615 commented Sep 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Uh oh!

Uh oh!

		Arc::new(tokio::runtime::Handle::current().block_on(async {
		XlineStore::new(&endpoints).await.expect("connect xline failed")

		let pod_name = String::from_utf8_lossy(kv.key())
		.replace("/registry/pods/", "");

-                                let pod_name = String::from_utf8_lossy(kv.key())
-                                    .replace("/registry/pods/", "");
+                                    .strip_prefix("/registry/pods/")
+                                    .unwrap_or("")
+                                    .to_string();

feat: Implement Xline native watch mechanism for rks #137

feat: Implement Xline native watch mechanism for rks #137

Uh oh!

Conversation

Flash0615 commented Sep 1, 2025

Summary

Changes

Breaking Changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!