From d1aa4166d8ce7f3db83ff1ffbd54b796943233b3 Mon Sep 17 00:00:00 2001 From: liuxu 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::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 &containers, - Errors &error) -> bool +auto PodSandboxManagerService::GetContainerListResponse(const std::string &readSandboxID, + std::vector &errors) -> std::unique_ptr> { + 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(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(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 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 &containers, +void PodSandboxManagerService::RemoveAllContainersInSandbox(const std::string &readSandboxID, std::vector &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, Errors &error); void StopContainerHelper(const std::string &containerID, Errors &error); - auto StopAllContainersInSandbox(const std::vector &containers, Errors &error) -> bool; + auto GetContainerListResponse(const std::string &readSandboxID, + std::vector &errors) -> std::unique_ptr>; + auto StopAllContainersInSandbox(const std::string &readSandboxID, Errors &error) -> int; auto GetNetworkReady(const std::string &podSandboxID, Errors &error) -> bool; - void RemoveAllContainersInSandbox(const std::vector &containers, std::vector &errors); + void RemoveAllContainersInSandbox(const std::string &readSandboxID, std::vector &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 -{ - ReadGuard 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 lock(m_containersMutex); - m_containers.push_back(id); -} - -void Sandbox::SetConatiners(const std::vector &cons) -{ - WriteGuard lock(m_containersMutex); - m_containers = cons; -} - -void Sandbox::RemoveContainer(const std::string &id) -{ - WriteGuard 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; auto GetImage() -> const std::string &; void SetNetMode(const std::string &mode); void SetController(std::shared_ptr 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 &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 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