diff options
Diffstat (limited to '0001-sandbox-del-m_containers-and-m_containersMutex.patch')
| -rw-r--r-- | 0001-sandbox-del-m_containers-and-m_containersMutex.patch | 347 |
1 files changed, 347 insertions, 0 deletions
diff --git a/0001-sandbox-del-m_containers-and-m_containersMutex.patch b/0001-sandbox-del-m_containers-and-m_containersMutex.patch new file mode 100644 index 0000000..84c3455 --- /dev/null +++ b/0001-sandbox-del-m_containers-and-m_containersMutex.patch @@ -0,0 +1,347 @@ +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 + |