Add rust implementation of container Python script #38

saiarcot895 · 2025-01-07T06:16:40Z

Add a rust implementation of the container script in the SONiC image. This script is primarily responsible for starting, waiting on, and stopping containers.

In the case of local containers, this script is basically a wrapper around docker start, docker wait, and docker stop. In the case of Kubernetes containers, there's some additional logic/checks on whether the container should be started or not; this is not fully implemented.

Basic testing has been done with local containers (the most common use case).

This handles most use cases for local containers. For Kubernetes containers, there's very limited handling in the start function, and pretty much nothing else. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-07T06:16:42Z

/azp run

azure-pipelines · 2025-01-07T06:16:50Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-07T06:35:44Z

/azp run

azure-pipelines · 2025-01-07T06:35:53Z

Azure Pipelines successfully started running 1 pipeline(s).

r12f · 2025-01-20T19:02:32Z

Hi Sai, looks like it is hitting CI errors. Do you mind to help take a look?

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-23T07:40:28Z

/azp run

azure-pipelines · 2025-01-23T07:40:38Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-23T20:46:19Z

/azp run

azure-pipelines · 2025-01-23T20:46:29Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-24T19:39:43Z

/azp run

azure-pipelines · 2025-01-24T19:39:51Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-27T20:14:36Z

/azp run

azure-pipelines · 2025-01-27T20:14:46Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-31T02:01:27Z

/azp run

azure-pipelines · 2025-01-31T02:01:36Z

Azure Pipelines successfully started running 1 pipeline(s).

r12f · 2025-02-01T21:21:11Z

crates/container/Cargo.toml

+[dependencies]
+bollard = { version = "0.17.1", features = ["chrono"] }
+chrono = "0.4.38"
+enumset = "1.1.5"


Will be better to move these dependencies to workspace root.

r12f · 2025-02-01T21:22:46Z

crates/container/Cargo.toml

+bollard = { version = "0.17.1", features = ["chrono"] }
+chrono = "0.4.38"
+enumset = "1.1.5"
+futures-util = "0.3.30"


0.3 will be good enough. The last minor version can be ignored, since we have the lock file and will help with upgrade.

r12f · 2025-02-01T21:24:17Z

crates/container/src/main.rs

+
+// DB field names
+//const FEATURE_TABLE: &str = "FEATURE";
+const SET_OWNER: &str = "set_owner";


Maybe better to add a prefix to specify these are db fields.

Added DB_ or DB_FIELD_ prefix.

r12f · 2025-02-01T22:10:01Z

crates/container/src/main.rs

+
+fn set_label(db_connections: &DbConnections, _feature: &str, _create: bool) {
+    if db_connections.remote_ctr_enabled {
+        todo!();


Is this part missing?

It was. Current todo! sections have been implemented, but since they're Kubernetes-specific, I can't test them.

r12f · 2025-02-01T22:14:30Z

crates/container/src/main.rs

+    }
+
+    let rt = tokio::runtime::Builder::new_current_thread()
+        .enable_all()


Looks like we can use async_main to simplify the code. Is this delayed block_on intentional?

Reworked the code to use async main with tokio. Not sure what you mean by if the delayed block_on is intentional, but docker.wait_container is an async method, and I needed to block until that was complete.

Edit: Collecting the results specifically (the try_collect method) is async, and I needed to wait on that.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-02-03T22:03:09Z

/azp run

azure-pipelines · 2025-02-03T22:03:18Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-03-03T16:49:53Z

/azp run

azure-pipelines · 2025-03-03T16:50:02Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-03-03T18:06:20Z

/azp run

azure-pipelines · 2025-03-03T18:06:30Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-03-06T01:23:44Z

/azp run

azure-pipelines · 2025-03-06T01:23:52Z

Azure Pipelines successfully started running 1 pipeline(s).

r12f · 2025-03-07T06:09:50Z

crates/container/src/main.rs

+    let cli = Cli::parse();
+
+    match cli.action {
+        Action::Start => container_start(&cli.name).await,


Hi Sai, I wonder if we can create a container class and make it more OOP.

the container code will be better to move into container.rs.

Done. I will note that memory usage has increased slightly with this change, by almost 2MB. I'm guessing it's because Rust now needs to actually create and store an object.

admin@vlab-01:~$ ps -eF | grep "wait pmon" admin 91569 91562 0 17556 47720 0 Mar07 ? 00:00:00 python3 /usr/local/bin/container wait pmon admin 459955 5819 0 5770 10996 0 20:45 pts/0 00:00:00 ./container-before-updates wait pmon admin 459956 5819 0 73382 12628 0 20:45 pts/0 00:00:00 ./container-after-oop wait pmon

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

lguohan · 2025-03-08T18:59:16Z

crates/container/src/main.rs

@@ -0,0 +1,399 @@
+use bollard::container::*;


is this one replacing this code?

https://github.com/sonic-net/sonic-buildimage/tree/master/src/sonic-ctrmgrd/ctrmgr

Yes, specifically https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-ctrmgrd/ctrmgr/container.

Local containers has been tested, Kubernetes has not been tested.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-03-09T16:55:12Z

/azp run

azure-pipelines · 2025-03-09T16:55:21Z

Azure Pipelines successfully started running 1 pipeline(s).

saiarcot895 added 9 commits November 1, 2024 14:06

Partially implement the container script in Rust

192ac81

This handles most use cases for local containers. For Kubernetes containers, there's very limited handling in the start function, and pretty much nothing else. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Update Cargo.lock for new crates that have been added

73fc12c

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix clap missing dependency

b713642

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Implement most of the parts that involve Kubernetes

3ef720f

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Update for using CxxString

61f6fa8

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Remove potential extra copy by using std::borrow::Cow

500c4f1

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into container-rust

8ed9942

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Add support for stop timeout

393f44e

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into container-rust

c8f7da0

Fix formatting

d04d9cc

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into container-rust

fd42c16

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

saiarcot895 added 3 commits January 23, 2025 12:22

Fix formatting

d9fa9ef

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Comment out unused variables

c58bf63

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Remove extra newline

179d48b

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

saiarcot895 added 2 commits January 24, 2025 11:39

Fix syntax and formatting

31d8717

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into container-rust

dafb626

Fix another linter error

e60b819

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into container-rust

14c41ee

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

r12f reviewed Feb 1, 2025

View reviewed changes

saiarcot895 added 4 commits February 2, 2025 17:58

Implement the Kubernetes aspects

23805db

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Return an Option for the container version

71df3c7

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Make main method be async

9595ba1

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Actually have remote_ctr_enabled do something

fcd9550

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

saiarcot895 added 4 commits February 3, 2025 14:46

Move crate packages up to the workspace Cargo file

204724d

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Update the futures-* packages for version consistency with futures-core

95ecadf

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into container-rust

47ab84d

Merge remote-tracking branch 'origin/master' into container-rust

a039008

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Port over some missing code

e3ef749

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Rename DB-related fields to add DB_ or DB_FIELD_ prefix

178e210

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

r12f reviewed Mar 7, 2025

View reviewed changes

Move container functions into a class

8ba0bd6

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

lguohan reviewed Mar 8, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into container-rust

a32bc4b

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Add rust implementation of container Python script #38

Are you sure you want to change the base?

Add rust implementation of container Python script #38

Conversation

saiarcot895 commented Jan 7, 2025

mssonicbld commented Jan 7, 2025

azure-pipelines bot commented Jan 7, 2025

mssonicbld commented Jan 7, 2025

azure-pipelines bot commented Jan 7, 2025

r12f commented Jan 20, 2025

mssonicbld commented Jan 23, 2025

azure-pipelines bot commented Jan 23, 2025

mssonicbld commented Jan 23, 2025

azure-pipelines bot commented Jan 23, 2025

mssonicbld commented Jan 24, 2025

azure-pipelines bot commented Jan 24, 2025

mssonicbld commented Jan 27, 2025

azure-pipelines bot commented Jan 27, 2025

mssonicbld commented Jan 31, 2025

azure-pipelines bot commented Jan 31, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saiarcot895 Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

mssonicbld commented Feb 3, 2025

azure-pipelines bot commented Feb 3, 2025

mssonicbld commented Mar 3, 2025

azure-pipelines bot commented Mar 3, 2025

mssonicbld commented Mar 3, 2025

azure-pipelines bot commented Mar 3, 2025

mssonicbld commented Mar 6, 2025

azure-pipelines bot commented Mar 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssonicbld commented Mar 9, 2025

azure-pipelines bot commented Mar 9, 2025

saiarcot895 Mar 3, 2025 •

edited

Loading