path: root/0001-sandbox-del-m_containers-and-m_containersMutex.patch

From d1aa4166d8ce7f3db83ff1ffbd54b796943233b3 Mon Sep 17 00:00:00 2001
From: liuxu <liuxu156@huawei.com>
Date: Tue, 24 Oct 2023 16:19:15 +0800
Subject: [PATCH 01/14] sandbox:del m_containers and m_containersMutex

---
 .../v1/v1_cri_container_manager_service.cc    |  30 -----
 .../cri/v1/v1_cri_container_manager_service.h |   1 -
 .../v1/v1_cri_pod_sandbox_manager_service.cc  | 103 +++++++++++++++---
 .../v1/v1_cri_pod_sandbox_manager_service.h   |   7 +-
 src/daemon/sandbox/sandbox.cc                 |  31 ------
 src/daemon/sandbox/sandbox.h                  |   7 --
 6 files changed, 95 insertions(+), 84 deletions(-)

diff --git a/src/daemon/entry/cri/v1/v1_cri_container_manager_service.cc b/src/daemon/entry/cri/v1/v1_cri_container_manager_service.cc
index eb19cac6..70629591 100644
--- a/src/daemon/entry/cri/v1/v1_cri_container_manager_service.cc
+++ b/src/daemon/entry/cri/v1/v1_cri_container_manager_service.cc
@@ -499,7 +499,6 @@ std::string ContainerManagerService::CreateContainer(const std::string &podSandb
     }
 
     response_id = response->id;
-    sandbox->AddContainer(response_id);
 
 cleanup:
     free_container_create_request(request);
@@ -591,37 +590,8 @@ void ContainerManagerService::StopContainer(const std::string &containerID, int6
     CRIHelpers::StopContainer(m_cb, containerID, timeout, error);
 }
 
-// TODO: Consider to refactor the way we handle container list in sandbox.
-//       This function might be removed after that.
-void ContainerManagerService::RemoveContainerIDFromSandbox(const std::string &containerID)
-{
-    std::string realContainerID;
-    std::string podSandboxID;
-    Errors error;
-
-    CRIHelpersV1::GetContainerSandboxID(containerID, realContainerID, podSandboxID, error);
-    if (error.NotEmpty()) {
-        WARN("Failed to get sandbox id for container %s: %s", containerID.c_str(), error.GetCMessage());
-        return;
-    }
-
-    std::shared_ptr<sandbox::Sandbox> sandbox = sandbox::SandboxManager::GetInstance()->GetSandbox(podSandboxID);
-    if (sandbox == nullptr) {
-        ERROR("Failed to get sandbox instance: %s for creating container", podSandboxID.c_str());
-        error.Errorf("Failed to get sandbox instance: %s for creating container", podSandboxID.c_str());
-        return;
-    }
-
-    sandbox->RemoveContainer(realContainerID);
-}
-
 void ContainerManagerService::RemoveContainer(const std::string &containerID, Errors &error)
 {
-    // TODO: Refactor after adding the ability to use sandbox manager for sandboxid query
-    //       This will remove container id from sandbox container_list first,
-    //       if the following operation failed, it could cause inconsistency.
-    RemoveContainerIDFromSandbox(containerID);
-
     CRIHelpers::RemoveContainer(m_cb, containerID, error);
     if (error.NotEmpty()) {
         WARN("Failed to remove container %s", containerID.c_str());
diff --git a/src/daemon/entry/cri/v1/v1_cri_container_manager_service.h b/src/daemon/entry/cri/v1/v1_cri_container_manager_service.h
index 31449170..1d210416 100644
--- a/src/daemon/entry/cri/v1/v1_cri_container_manager_service.h
+++ b/src/daemon/entry/cri/v1/v1_cri_container_manager_service.h
@@ -97,7 +97,6 @@ private:
     void MakeContainerConfig(const runtime::v1::ContainerConfig &config, container_config *cConfig,
                              Errors &error);
     void CreateContainerLogSymlink(const std::string &containerID, Errors &error);
-    void RemoveContainerIDFromSandbox(const std::string &containerID);
     void ListContainersFromGRPC(const runtime::v1::ContainerFilter *filter, container_list_request **request,
                                 Errors &error);
     void ListContainersToGRPC(container_list_response *response,
diff --git a/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.cc b/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.cc
index 901ef231..2c802900 100644
--- a/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.cc
+++ b/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.cc
@@ -452,20 +452,90 @@ auto PodSandboxManagerService::GetSandboxKey(const container_inspect *inspect_da
     return std::string(inspect_data->network_settings->sandbox_key);
 }
 
-auto PodSandboxManagerService::StopAllContainersInSandbox(const std::vector<std::string> &containers,
-                                                          Errors &error) -> bool
+auto PodSandboxManagerService::GetContainerListResponse(const std::string &readSandboxID,
+                                                        std::vector<std::string> &errors) -> std::unique_ptr<CStructWrapper<container_list_response>>
 {
+    int ret = 0;
+    container_list_request *list_request { nullptr };
+    container_list_response *list_response { nullptr };
+
+    if (m_cb == nullptr || m_cb->container.list == nullptr) {
+        ERROR("Unimplemented callback");
+        errors.push_back("Unimplemented callback");
+        return nullptr;
+    }
+
+    // list all containers to stop
+    auto list_request_wrapper = makeUniquePtrCStructWrapper<container_list_request>(free_container_list_request);
+    if (list_request_wrapper == nullptr) {
+        ERROR("Out of memory");
+        errors.push_back("Out of memory");
+        return nullptr;
+    }
+    list_request = list_request_wrapper->get();
+    list_request->all = true;
+
+    list_request->filters = (defs_filters *)util_common_calloc_s(sizeof(defs_filters));
+    if (list_request->filters == nullptr) {
+        ERROR("Out of memory");
+        errors.push_back("Out of memory");
+        return nullptr;
+    }
+
+    // Add sandbox label
+    if (CRIHelpers::FiltersAddLabel(list_request->filters, CRIHelpers::Constants::SANDBOX_ID_LABEL_KEY,
+                                    readSandboxID) != 0) {
+        std::string tmp_errmsg = "Failed to add label in sandbox" + readSandboxID;
+        ERROR(tmp_errmsg.c_str());
+        errors.push_back(tmp_errmsg);
+        return nullptr;
+    }
+
+    ret = m_cb->container.list(list_request, &list_response);
+    auto list_response_wrapper = makeUniquePtrCStructWrapper<container_list_response>(list_response, free_container_list_response);
+    if (list_response_wrapper == nullptr) {
+        ERROR("Failed to call list container callback");
+        errors.push_back("Failed to call list container callback");
+        return nullptr;
+    }
+    if (ret != 0) {
+        if (list_response != nullptr && list_response->errmsg != nullptr) {
+            ERROR(list_response->errmsg);
+            errors.push_back(list_response->errmsg);
+        } else {
+            ERROR("Failed to call list container callback");
+            errors.push_back("Failed to call list container callback");
+        }
+        return nullptr;
+    }
+
+    return list_response_wrapper;
+}                                                          
+
+auto PodSandboxManagerService::StopAllContainersInSandbox(const std::string &readSandboxID,
+                                                          Errors &error) -> int
+{
+    int ret = 0;
+    std::vector<std::string> errors;
+    auto list_response_wrapper = GetContainerListResponse(readSandboxID, errors);
+    if (list_response_wrapper == nullptr) {
+        error.SetAggregate(errors);
+        return -1;
+    }
+    auto list_response = list_response_wrapper->get();
+
     // Stop all containers in the sandbox.
-    for (const auto &con : containers) {
+    for (size_t i = 0; i < list_response->containers_len; i++) {
         Errors stopError;
-        CRIHelpers::StopContainerHelper(m_cb, con, 0, stopError);
+        CRIHelpers::StopContainerHelper(m_cb, list_response->containers[i]->id, 0, stopError);
         if (stopError.NotEmpty() && !CRIHelpers::IsContainerNotFoundError(stopError.GetMessage())) {
-            ERROR("Error stop container: %s: %s", con.c_str(), stopError.GetCMessage());
+            ERROR("Error stop container: %s: %s", list_response->containers[i]->id, stopError.GetCMessage());
             error.SetError(stopError.GetMessage());
-            return false;
+            return -1;
         }
     }
-    return true;
+
+    return ret;
 }
 
 auto PodSandboxManagerService::GetNetworkReady(const std::string &podSandboxID, Errors &error) -> bool
@@ -508,7 +578,7 @@ void PodSandboxManagerService::StopPodSandbox(const std::string &podSandboxID, E
     // Stop all containers inside the sandbox. This terminates the container forcibly,
     // and container may still be created, so production should not rely on this behavior.
     // TODO: according to the state(stopping and removal) in sandbox to avoid future container creation.
-    if (!StopAllContainersInSandbox(sandbox->GetContainers(), error)) {
+    if (StopAllContainersInSandbox(sandbox->GetId(), error) != 0) {
         return;
     }
 
@@ -524,15 +594,22 @@ void PodSandboxManagerService::StopPodSandbox(const std::string &podSandboxID, E
     sandbox->Stop(sandbox::DEFAULT_STOP_TIMEOUT, error);
 }
 
-void PodSandboxManagerService::RemoveAllContainersInSandbox(const std::vector<std::string> &containers,
+void PodSandboxManagerService::RemoveAllContainersInSandbox(const std::string &readSandboxID,
                                                             std::vector<std::string> &errors)
 {
+    auto list_response_wrapper = GetContainerListResponse(readSandboxID, errors);
+    if (list_response_wrapper == nullptr) {
+        return;
+    }
+
+    auto list_response = list_response_wrapper->get();
+
     // Remove all containers in the sandbox.
-    for (const auto &con : containers) {
+    for (size_t i = 0; i < list_response->containers_len; i++) {
         Errors rmError;
-        CRIHelpers::RemoveContainerHelper(m_cb, con, rmError);
+        CRIHelpers::RemoveContainerHelper(m_cb, list_response->containers[i]->id, rmError);
         if (rmError.NotEmpty() && !CRIHelpers::IsContainerNotFoundError(rmError.GetMessage())) {
-            ERROR("Error remove container: %s: %s", con.c_str(), rmError.GetCMessage());
+            ERROR("Error remove container: %s: %s", list_response->containers[i]->id, rmError.GetCMessage());
             errors.push_back(rmError.GetMessage());
         }
     }
@@ -598,7 +675,7 @@ void PodSandboxManagerService::RemovePodSandbox(const std::string &podSandboxID,
     // Remove all containers inside the sandbox.
     // container may still be created, so production should not rely on this behavior.
     // TODO: according to the state(stopping and removal) in sandbox to avoid future container creation.
-    RemoveAllContainersInSandbox(sandbox->GetContainers(), errors);
+    RemoveAllContainersInSandbox(sandbox->GetId(), errors);
     if (errors.size() != 0) {
         error.SetAggregate(errors);
         return;
diff --git a/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.h b/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.h
index 48a7cf7f..2bd28007 100644
--- a/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.h
+++ b/src/daemon/entry/cri/v1/v1_cri_pod_sandbox_manager_service.h
@@ -32,6 +32,7 @@
 #include "cgroup.h"
 #include "sandbox.h"
 #include "v1_cri_container_manager_service.h"
+#include "cstruct_wrapper.h"
 
 namespace CRIV1 {
 class PodSandboxManagerService {
@@ -89,9 +90,11 @@ private:
                              Errors &error);
     void ClearCniNetwork(const std::shared_ptr<sandbox::Sandbox> sandbox, Errors &error);
     void StopContainerHelper(const std::string &containerID, Errors &error);
-    auto StopAllContainersInSandbox(const std::vector<std::string> &containers, Errors &error) -> bool;
+    auto GetContainerListResponse(const std::string &readSandboxID,
+                                  std::vector<std::string> &errors) -> std::unique_ptr<CStructWrapper<container_list_response>>;
+    auto StopAllContainersInSandbox(const std::string &readSandboxID, Errors &error) -> int;
     auto GetNetworkReady(const std::string &podSandboxID, Errors &error) -> bool;
-    void RemoveAllContainersInSandbox(const std::vector<std::string> &containers, std::vector<std::string> &errors);
+    void RemoveAllContainersInSandbox(const std::string &readSandboxID, std::vector<std::string> &errors);
     void ClearNetworkReady(const std::string &podSandboxID);
     auto SharesHostNetwork(const container_inspect *inspect) -> runtime::v1::NamespaceMode;
     auto SharesHostPid(const container_inspect *inspect) -> runtime::v1::NamespaceMode;
diff --git a/src/daemon/sandbox/sandbox.cc b/src/daemon/sandbox/sandbox.cc
index ece28f4d..c8fd30be 100644
--- a/src/daemon/sandbox/sandbox.cc
+++ b/src/daemon/sandbox/sandbox.cc
@@ -135,12 +135,6 @@ auto Sandbox::GetRuntimeHandle() const -> const std::string &
     return m_runtimeInfo.runtimeHandler;
 }
 
-auto Sandbox::GetContainers() -> std::vector<std::string>
-{
-    ReadGuard<RWMutex> lock(m_containersMutex);
-    return m_containers;
-}
-
 auto Sandbox::GetSandboxConfig() const -> const runtime::v1::PodSandboxConfig &
 {
     return *m_sandboxConfig;
@@ -409,27 +403,6 @@ void Sandbox::RemoveLabels(const std::string &key)
     m_sandboxConfig->mutable_labels()->erase(key);
 }
 
-void Sandbox::AddContainer(const std::string &id)
-{
-    WriteGuard<RWMutex> lock(m_containersMutex);
-    m_containers.push_back(id);
-}
-
-void Sandbox::SetConatiners(const std::vector<std::string> &cons)
-{
-    WriteGuard<RWMutex> lock(m_containersMutex);
-    m_containers = cons;
-}
-
-void Sandbox::RemoveContainer(const std::string &id)
-{
-    WriteGuard<RWMutex> lock(m_containersMutex);
-    auto it = std::find(m_containers.begin(), m_containers.end(), id);
-    if (it != m_containers.end()) {
-        m_containers.erase(it);
-    }
-}
-
 void Sandbox::UpdateNetworkSettings(const std::string &settingsJson, Errors &error)
 {
     if (settingsJson.length() == 0) {
@@ -1009,8 +982,6 @@ auto Sandbox::LoadMetadata(Errors &error) -> bool
     m_networkReady = metadata->get()->network_ready;
     m_taskAddress = std::string(metadata->get()->task_address);
     m_netNsPath = std::string(metadata->get()->net_ns_path);
-    Transform::CharArrayToStringVector((const char **)metadata->get()->containers,
-                                       util_array_len((const char **)metadata->get()->containers), m_containers);
 
     ret = google::protobuf::util::JsonStringToMessage(metadata->get()->sandbox_config_json, &config).ok();
     if (!ret) {
@@ -1120,8 +1091,6 @@ void Sandbox::FillSandboxMetadata(sandbox_metadata* metadata, Errors &error)
     metadata->task_address = util_strdup_s(m_taskAddress.c_str());
     metadata->net_ns_path = util_strdup_s(m_netNsPath.c_str());
 
-    metadata->containers = Transform::StringVectorToCharArray(m_containers);
-
     google::protobuf::util::MessageToJsonString(*m_sandboxConfig.get(), &jsonStr);
     if (jsonStr.length() == 0) {
         error.Errorf("Failed to get sandbox config json for sandbox: '%s'", m_id.c_str());
diff --git a/src/daemon/sandbox/sandbox.h b/src/daemon/sandbox/sandbox.h
index 13ee4958..20a8e338 100644
--- a/src/daemon/sandbox/sandbox.h
+++ b/src/daemon/sandbox/sandbox.h
@@ -104,7 +104,6 @@ public:
     auto GetCreatedAt() -> uint64_t;
     auto GetPid() -> uint32_t;
     auto GetTaskAddress() const -> const std::string &;
-    auto GetContainers() -> std::vector<std::string>;
     auto GetImage() -> const std::string &;
     void SetNetMode(const std::string &mode);
     void SetController(std::shared_ptr<Controller> controller);
@@ -112,9 +111,6 @@ public:
     void RemoveAnnotations(const std::string &key);
     void AddLabels(const std::string &key, const std::string &value);
     void RemoveLabels(const std::string &key);
-    void AddContainer(const std::string &id);
-    void SetConatiners(const std::vector<std::string> &cons);
-    void RemoveContainer(const std::string &id);
     void UpdateNetworkSettings(const std::string &settingsJson, Errors &error);
     auto UpdateStatsInfo(const StatsInfo &info) -> StatsInfo;
     void SetNetworkReady(bool ready);
@@ -203,9 +199,6 @@ private:
     bool m_networkReady;
     std::string m_networkSettings;
     std::string m_image;
-    // container id lists
-    std::vector<std::string> m_containers;
-    RWMutex m_containersMutex;
     // TOOD: m_sandboxConfig is a protobuf message, it can be converted to json string directly
     //       if save json string directly for sandbox recover, we need to consider hot
     //       upgrade between different CRI versions
-- 
2.42.0