From e3e5cf6d2a6858f9f83ee42f8ceeaaef4752ff1b Mon Sep 17 00:00:00 2001 From: zhangxiaoyu Date: Thu, 2 Mar 2023 14:27:01 +0800 Subject: [PATCH 20/26] fix CRI SetupPod and TearDownPod deadlock Signed-off-by: zhangxiaoyu --- src/daemon/entry/cri/cni_network_plugin.cc | 44 +++++++++++++------ .../cri_pod_sandbox_manager_service_impl.cc | 2 + src/daemon/entry/cri/network_plugin.cc | 39 +++++++++++----- 3 files changed, 61 insertions(+), 24 deletions(-) diff --git a/src/daemon/entry/cri/cni_network_plugin.cc b/src/daemon/entry/cri/cni_network_plugin.cc index 9b03bea1..02e75ffe 100644 --- a/src/daemon/entry/cri/cni_network_plugin.cc +++ b/src/daemon/entry/cri/cni_network_plugin.cc @@ -110,9 +110,12 @@ void CniNetworkPlugin::SetDefaultNetwork(std::unique_ptr network, st if (network == nullptr) { return; } - WLockNetworkMap(err); - if (err.NotEmpty()) { - ERROR("%s", err.GetCMessage()); + + Errors tmpErr; + WLockNetworkMap(tmpErr); + if (tmpErr.NotEmpty()) { + ERROR("%s", tmpErr.GetCMessage()); + err.AppendError(tmpErr.GetCMessage()); return; } m_defaultNetwork = std::move(network); @@ -120,9 +123,10 @@ void CniNetworkPlugin::SetDefaultNetwork(std::unique_ptr network, st DEBUG("Update new cni network: \"%s\"", m_defaultNetwork->GetName().c_str()); - UnlockNetworkMap(err); - if (err.NotEmpty()) { - ERROR("%s", err.GetCMessage()); + UnlockNetworkMap(tmpErr); + if (tmpErr.NotEmpty()) { + ERROR("%s", tmpErr.GetCMessage()); + err.AppendError(tmpErr.GetCMessage()); } } @@ -132,8 +136,11 @@ void CniNetworkPlugin::UpdateMutlNetworks(std::vectorGetName()] = std::move(*iter); } - UnlockNetworkMap(err); + UnlockNetworkMap(tmpErr); + if (tmpErr.NotEmpty()) { + err.AppendError(tmpErr.GetCMessage()); + } } CniNetworkPlugin::CniNetworkPlugin(std::vector &binDirs, const std::string &confDir, @@ -336,13 +346,20 @@ free_out: void CniNetworkPlugin::CheckInitialized(Errors &err) { - RLockNetworkMap(err); - if (err.NotEmpty()) { - ERROR("%s", err.GetCMessage()); + Errors tmpErr; + RLockNetworkMap(tmpErr); + if (tmpErr.NotEmpty()) { + ERROR("%s", tmpErr.GetCMessage()); + err.AppendError(tmpErr.GetCMessage()); return; } bool inited = (m_defaultNetwork != nullptr); - UnlockNetworkMap(err); + + UnlockNetworkMap(tmpErr); + if (tmpErr.NotEmpty()) { + err.AppendError(tmpErr.GetCMessage()); + } + if (!inited) { err.AppendError("cni config uninitialized"); } @@ -527,6 +544,7 @@ void CniNetworkPlugin::SetUpPod(const std::string &ns, const std::string &name, } } + err.Clear(); RLockNetworkMap(err); if (err.NotEmpty()) { ERROR("%s", err.GetCMessage()); diff --git a/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.cc b/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.cc index 2ebd800e..7ff545db 100644 --- a/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.cc +++ b/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.cc @@ -642,6 +642,7 @@ auto PodSandboxManagerServiceImpl::GetRealSandboxIDToStop(const std::string &pod if (status->linux().namespaces().has_options()) { hostNetwork = (status->linux().namespaces().options().network() == runtime::v1alpha2::NamespaceMode::NODE); } + // if metadata is invalid, don't return -1 and continue stopping pod if (status->has_metadata()) { name = status->metadata().name(); ns = status->metadata().namespace_(); @@ -779,6 +780,7 @@ auto PodSandboxManagerServiceImpl::ClearCniNetwork(const std::string &realSandbo } } + pluginErr.Clear(); m_pluginManager->TearDownPod(ns, name, Network::DEFAULT_NETWORK_INTERFACE_NAME, realSandboxID, stdAnnos, pluginErr); if (pluginErr.NotEmpty()) { diff --git a/src/daemon/entry/cri/network_plugin.cc b/src/daemon/entry/cri/network_plugin.cc index 7a957de3..9933b584 100644 --- a/src/daemon/entry/cri/network_plugin.cc +++ b/src/daemon/entry/cri/network_plugin.cc @@ -422,21 +422,27 @@ void PluginManager::GetPodNetworkStatus(const std::string &ns, const std::string const std::string &interfaceName, const std::string &podSandboxID, PodNetworkStatus &status, Errors &error) { + Errors tmpErr; std::string fullName = name + "_" + ns; - Lock(fullName, error); - if (error.NotEmpty()) { + Lock(fullName, tmpErr); + if (tmpErr.NotEmpty()) { + error.AppendError(tmpErr.GetCMessage()); return; } if (m_plugin != nullptr) { - Errors tmpErr; m_plugin->GetPodNetworkStatus(ns, name, interfaceName, podSandboxID, status, tmpErr); if (tmpErr.NotEmpty()) { error.Errorf("NetworkPlugin %s failed on the status hook for pod %s: %s", m_plugin->Name().c_str(), fullName.c_str(), tmpErr.GetCMessage()); } } - Unlock(fullName, error); + + tmpErr.Clear(); + Unlock(fullName, tmpErr); + if (tmpErr.NotEmpty()) { + error.AppendError(tmpErr.GetCMessage()); + } } void PluginManager::SetUpPod(const std::string &ns, const std::string &name, const std::string &interfaceName, @@ -447,20 +453,26 @@ void PluginManager::SetUpPod(const std::string &ns, const std::string &name, con return; } + Errors tmpErr; std::string fullName = name + "_" + ns; - Lock(fullName, error); - if (error.NotEmpty()) { + Lock(fullName, tmpErr); + if (tmpErr.NotEmpty()) { + error.AppendError(tmpErr.GetCMessage()); return; } INFO("Calling network plugin %s to set up pod %s", m_plugin->Name().c_str(), fullName.c_str()); - Errors tmpErr; m_plugin->SetUpPod(ns, name, interfaceName, podSandboxID, annotations, options, tmpErr); if (tmpErr.NotEmpty()) { error.Errorf("NetworkPlugin %s failed to set up pod %s network: %s", m_plugin->Name().c_str(), fullName.c_str(), tmpErr.GetCMessage()); } - Unlock(fullName, error); + + tmpErr.Clear(); + Unlock(fullName, tmpErr); + if (tmpErr.NotEmpty()) { + error.AppendError(tmpErr.GetCMessage()); + } } void PluginManager::TearDownPod(const std::string &ns, const std::string &name, const std::string &interfaceName, @@ -469,8 +481,9 @@ void PluginManager::TearDownPod(const std::string &ns, const std::string &name, { Errors tmpErr; std::string fullName = name + "_" + ns; - Lock(fullName, error); - if (error.NotEmpty()) { + Lock(fullName, tmpErr); + if (tmpErr.NotEmpty()) { + error.AppendError(tmpErr.GetCMessage()); return; } if (m_plugin == nullptr) { @@ -484,7 +497,11 @@ void PluginManager::TearDownPod(const std::string &ns, const std::string &name, fullName.c_str(), tmpErr.GetCMessage()); } unlock: - Unlock(fullName, error); + tmpErr.Clear(); + Unlock(fullName, tmpErr); + if (tmpErr.NotEmpty()) { + error.AppendError(tmpErr.GetCMessage()); + } } void NoopNetworkPlugin::Init(const std::string &hairpinMode, const std::string &nonMasqueradeCIDR, int mtu, -- 2.25.1