iSulad/0065-start-sandbox-before-setup-network-by-default.patch

From a13e021620c62f32dfb1fd5242a3cf43c1d163b8 Mon Sep 17 00:00:00 2001
From: zhangxiaoyu <zhangxiaoyu58@huawei.com>
Date: Wed, 14 Dec 2022 10:50:49 +0800
Subject: [PATCH 65/65] start sandbox before setup network by default

Signed-off-by: zhangxiaoyu <zhangxiaoyu58@huawei.com>
---
 src/daemon/entry/cri/cni_network_plugin.cc    |  43 +++---
 src/daemon/entry/cri/cni_network_plugin.h     |   2 +
 src/daemon/entry/cri/cri_helpers.cc           |  12 ++
 src/daemon/entry/cri/cri_helpers.h            |   3 +
 .../cri_pod_sandbox_manager_service_impl.cc   | 135 +++++++++++-------
 .../cri_pod_sandbox_manager_service_impl.h    |   2 +-
 src/daemon/entry/cri/cri_security_context.cc  |   3 +-
 src/daemon/modules/spec/specs.c               |   2 +-
 src/utils/cutils/utils_file.c                 |   2 +-
 9 files changed, 135 insertions(+), 69 deletions(-)

diff --git a/src/daemon/entry/cri/cni_network_plugin.cc b/src/daemon/entry/cri/cni_network_plugin.cc
index c850bc32..976a21a4 100644
--- a/src/daemon/entry/cri/cni_network_plugin.cc
+++ b/src/daemon/entry/cri/cni_network_plugin.cc
@@ -479,6 +479,27 @@ cleanup:
     return result;
 }
 
+auto CniNetworkPlugin::GetNetNSPath(const std::string &id, const std::map<std::string, std::string> &annotations,
+                                    Errors &err) -> std::string
+{
+    std::string netnsPath;
+
+    if (CRIHelpers::SetupNetworkFront(annotations)) {
+        auto iter = annotations.find(CRIHelpers::Constants::POD_SANDBOX_KEY);
+        if (iter == annotations.end()) {
+            ERROR("Failed to find sandbox key from annotations");
+            return netnsPath;
+        }
+        return iter->second;
+    }
+
+    netnsPath = GetNetNS(id, err);
+    if (err.NotEmpty()) {
+        ERROR("CNI failed to retrieve network namespace path: %s", err.GetCMessage());
+    }
+
+    return netnsPath;
+}
 
 void CniNetworkPlugin::SetUpPod(const std::string &ns, const std::string &name, const std::string &interfaceName,
                                 const std::string &id, const std::map<std::string, std::string> &annotations,
@@ -489,12 +510,7 @@ void CniNetworkPlugin::SetUpPod(const std::string &ns, const std::string &name,
         return;
     }
 
-    auto iter = annotations.find(CRIHelpers::Constants::POD_SANDBOX_KEY);
-    if (iter == annotations.end()) {
-        ERROR("Failed to find sandbox key from annotations");
-        return;
-    }
-    const std::string netnsPath = iter->second;
+    std::string netnsPath = GetNetNSPath(id, annotations, err);
     if (netnsPath.length() == 0) {
         ERROR("Failed to get network namespace path");
         return;
@@ -600,21 +616,14 @@ void CniNetworkPlugin::TearDownPod(const std::string &ns, const std::string &nam
     }
     Errors tmpErr;
 
-    auto iter = annotations.find(CRIHelpers::Constants::POD_SANDBOX_KEY);
-    if (iter == annotations.end()) {
-        ERROR("Failed to find sandbox key from annotations");
-        return;
-    }
-    std::string netnsPath = iter->second;
-    if (netnsPath.length() == 0) {
-        ERROR("Failed to get network namespace path");
-        return;
-    }
+    std::string netnsPath = GetNetNSPath(id, annotations, err);
 
     // When netns file does not exist, netnsPath is assigned to an
     // empty string so that lxc can handle the path properly
-    if (!util_file_exists(netnsPath.c_str())) {
+    if (!util_file_exists(netnsPath.c_str()) || err.NotEmpty()) {
+        ERROR("Failed to get network namespace path, maybe podsandbox '%s' has been stopped", id.c_str());
         netnsPath = "";
+        err.Clear();
     }
 
     RLockNetworkMap(err);
diff --git a/src/daemon/entry/cri/cni_network_plugin.h b/src/daemon/entry/cri/cni_network_plugin.h
index 8d51a94d..434222b5 100644
--- a/src/daemon/entry/cri/cni_network_plugin.h
+++ b/src/daemon/entry/cri/cni_network_plugin.h
@@ -118,6 +118,8 @@ public:
 private:
     auto GetNetNS(const std::string &podSandboxID, Errors &err) -> std::string;
 
+    auto GetNetNSPath(const std::string &id, const std::map<std::string, std::string> &annotations,
+                      Errors &err) -> std::string;
 
 private:
     virtual void PlatformInit(Errors &error);
diff --git a/src/daemon/entry/cri/cri_helpers.cc b/src/daemon/entry/cri/cri_helpers.cc
index 64cea7ba..711196ba 100644
--- a/src/daemon/entry/cri/cri_helpers.cc
+++ b/src/daemon/entry/cri/cri_helpers.cc
@@ -49,6 +49,8 @@ const std::string Constants::DOCKER_IMAGEID_PREFIX { "docker://" };
 const std::string Constants::DOCKER_PULLABLE_IMAGEID_PREFIX { "docker-pullable://" };
 const std::string Constants::RUNTIME_READY { "RuntimeReady" };
 const std::string Constants::NETWORK_READY { "NetworkReady" };
+// Kata 2.x need create network namespace and setup network befoce run podsandbox
+const std::string Constants::NETWORK_SETUP_ANNOTATION_KEY { "cri.sandbox.network.setup.v2" };
 const std::string Constants::POD_CHECKPOINT_KEY { "cri.sandbox.isulad.checkpoint" };
 const std::string Constants::CONTAINER_TYPE_ANNOTATION_KEY { "io.kubernetes.cri.container-type" };
 const std::string Constants::CONTAINER_NAME_ANNOTATION_KEY { "io.kubernetes.cri.container-name" };
@@ -1009,4 +1011,14 @@ out:
     return runtime_val;
 }
 
+bool SetupNetworkFront(const std::map<std::string, std::string> &annotations)
+{
+    auto iter = annotations.find(CRIHelpers::Constants::NETWORK_SETUP_ANNOTATION_KEY);
+    if (iter == annotations.end()) {
+        return false;
+    }
+
+    return iter->second == std::string("true");
+}
+
 } // namespace CRIHelpers
diff --git a/src/daemon/entry/cri/cri_helpers.h b/src/daemon/entry/cri/cri_helpers.h
index b3bfafe4..d50759ad 100644
--- a/src/daemon/entry/cri/cri_helpers.h
+++ b/src/daemon/entry/cri/cri_helpers.h
@@ -48,6 +48,7 @@ public:
     static const std::string DOCKER_PULLABLE_IMAGEID_PREFIX;
     static const std::string RUNTIME_READY;
     static const std::string NETWORK_READY;
+    static const std::string NETWORK_SETUP_ANNOTATION_KEY;
     static const std::string POD_CHECKPOINT_KEY;
     static const size_t MAX_CHECKPOINT_KEY_LEN { 250 };
     static const std::string CONTAINER_TYPE_ANNOTATION_KEY;
@@ -151,6 +152,8 @@ void StopContainer(service_executor_t *cb, const std::string &containerID, int64
 char *GenerateExecSuffix();
 
 char *cri_runtime_convert(const char *runtime);
+
+bool SetupNetworkFront(const std::map<std::string, std::string> &annotations);
 }; // namespace CRIHelpers
 
 #endif // DAEMON_ENTRY_CRI_CRI_HELPERS_H
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 f7b3119d..4c245763 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
@@ -99,10 +99,20 @@ void PodSandboxManagerServiceImpl::ApplySandboxResources(const runtime::v1alpha2
 }
 
 
-void PodSandboxManagerServiceImpl::SetHostConfigDefaultValue(host_config *hc)
+void PodSandboxManagerServiceImpl::SetHostConfigDefaultValue(const google::protobuf::Map<std::string, std::string> &annotations, 
+                                                             host_config *hc)
 {
     free(hc->network_mode);
-    hc->network_mode = util_strdup_s(CRI::Constants::namespaceModeFile.c_str());
+
+    auto iter = annotations.find(CRIHelpers::Constants::NETWORK_SETUP_ANNOTATION_KEY);
+    // set network mode file when kata 2.x
+    if (iter != annotations.end() && iter->second == std::string("true")) {
+        hc->network_mode = util_strdup_s(CRI::Constants::namespaceModeFile.c_str());
+        return;
+    }
+
+    // default network mode is empty string
+    hc->network_mode = util_strdup_s("");
 }
 
 void PodSandboxManagerServiceImpl::MakeSandboxIsuladConfig(const runtime::v1alpha2::PodSandboxConfig &c,
@@ -168,7 +178,7 @@ void PodSandboxManagerServiceImpl::MakeSandboxIsuladConfig(const runtime::v1alph
         custom_config->hostname = util_strdup_s(c.hostname().c_str());
     }
 
-    SetHostConfigDefaultValue(hc);
+    SetHostConfigDefaultValue(c.annotations(), hc);
 
     if (c.has_linux()) {
         ApplySandboxLinuxOptions(c.linux(), hc, custom_config, error);
@@ -487,7 +497,6 @@ void PodSandboxManagerServiceImpl::SetupSandboxNetwork(const runtime::v1alpha2::
 {
     std::map<std::string, std::string> stdAnnos;
     std::map<std::string, std::string> networkOptions;
-    char* sandbox_key = get_sandbox_key(inspect_data);
 
     // Setup sandbox files
     if (config.has_dns_config() && inspect_data->resolv_conf_path != nullptr) {
@@ -495,32 +504,36 @@ void PodSandboxManagerServiceImpl::SetupSandboxNetwork(const runtime::v1alpha2::
         SetupSandboxFiles(inspect_data->resolv_conf_path, config, error);
         if (error.NotEmpty()) {
             ERROR("failed to setup sandbox files");
-            goto cleanup;
+            return;
         }
     }
     // Do not invoke network plugins if in hostNetwork mode.
     if (config.linux().security_context().namespace_options().network() == runtime::v1alpha2::NamespaceMode::NODE) {
-        goto cleanup;
+        return;
     }
 
     // Setup networking for the sandbox.
     CRIHelpers::ProtobufAnnoMapToStd(config.annotations(), stdAnnos);
     stdAnnos[CRIHelpers::Constants::POD_CHECKPOINT_KEY] = jsonCheckpoint;
     networkOptions["UID"] = config.metadata().uid();
-    if (sandbox_key == NULL) {
-        goto cleanup;
+
+    if (namespace_is_file(inspect_data->host_config->network_mode)) {
+        char *sandbox_key = get_sandbox_key(inspect_data);
+        if (sandbox_key == nullptr) {
+            ERROR("Empty sandbox key");
+            error.SetError("Empty sandbox key");
+            return;
+        }
+        stdAnnos.insert(std::pair<std::string, std::string>(CRIHelpers::Constants::POD_SANDBOX_KEY,
+                        std::string(sandbox_key)));
+        free(sandbox_key);
     }
-    stdAnnos.insert(std::pair<std::string, std::string>(CRIHelpers::Constants::POD_SANDBOX_KEY, sandbox_key));
     m_pluginManager->SetUpPod(config.metadata().namespace_(), config.metadata().name(),
                               Network::DEFAULT_NETWORK_INTERFACE_NAME, response_id, stdAnnos, networkOptions, error);
     if (error.NotEmpty()) {
         ERROR("SetupPod failed: %s", error.GetCMessage());
-        StopContainerHelper(response_id, error);
-        goto cleanup;
     }
 
-cleanup:
-    free(sandbox_key);
     return;
 }
 
@@ -568,8 +581,9 @@ auto PodSandboxManagerServiceImpl::RunPodSandbox(const runtime::v1alpha2::PodSan
         ERROR("Failed to retrieve inspect data");
         goto cleanup;
     }
-    netnsPath = get_sandbox_key(inspect_data);
+
     if (namespace_is_file(inspect_data->host_config->network_mode)) {
+        netnsPath = get_sandbox_key(inspect_data);
         if (!util_file_exists(netnsPath) || util_mount_namespace(netnsPath) != 0) {
             error.Errorf("Failed to mount network namespace");
             ERROR("Failed to mount network namespace");
@@ -578,9 +592,11 @@ auto PodSandboxManagerServiceImpl::RunPodSandbox(const runtime::v1alpha2::PodSan
     }
 
     // Step 5: Setup networking for the sandbox.
-    SetupSandboxNetwork(config, response_id, jsonCheckpoint, inspect_data, error);
-    if (error.NotEmpty()) {
-        goto cleanup;
+    if (namespace_is_file(inspect_data->host_config->network_mode)) {
+        SetupSandboxNetwork(config, response_id, jsonCheckpoint, inspect_data, error);
+        if (error.NotEmpty()) {
+            goto cleanup;
+        }
     }
 
     // Step 6: Start the sandbox container.
@@ -589,6 +605,15 @@ auto PodSandboxManagerServiceImpl::RunPodSandbox(const runtime::v1alpha2::PodSan
         goto cleanup;
     }
 
+    // If netns mode is not file, setup network after start sandbox container
+    if (!namespace_is_file(inspect_data->host_config->network_mode)) {
+        SetupSandboxNetwork(config, response_id, jsonCheckpoint, inspect_data, error);
+        if (error.NotEmpty()) {
+            StopContainerHelper(response_id, error);
+            goto cleanup;
+        }
+    }
+
 cleanup:
     if (error.Empty()) {
         SetNetworkReady(response_id, true, error);
@@ -723,44 +748,58 @@ auto PodSandboxManagerServiceImpl::ClearCniNetwork(const std::string &realSandbo
                                                    /*error*/) -> int
 {
     Errors networkErr;
-    container_inspect* inspect_data = nullptr;
+    container_inspect *inspect_data = nullptr;
+    char *netnsPath = nullptr;
+
+    if (hostNetwork) {
+        return 0;
+    }
 
     bool ready = GetNetworkReady(realSandboxID, networkErr);
-    if (!hostNetwork && (ready || networkErr.NotEmpty())) {
-        Errors pluginErr;
-
-        // hostNetwork has indicated network mode which render host config unnecessary
-        // so that with_host_config is set to be false.
-        inspect_data = CRIHelpers::InspectContainer(realSandboxID, pluginErr, false);
-        if (pluginErr.NotEmpty()) {
-            ERROR("Failed to inspect container");
-        }
+    if (!ready && networkErr.Empty()) {
+        WARN("Network not ready");
+        return 0;
+    }
 
-        char* netnsPath = get_sandbox_key(inspect_data);
-        if (netnsPath == nullptr) {
-            ERROR("Failed to get network namespace path");
-            return 0;
-        }
+    Errors pluginErr;
+    inspect_data = CRIHelpers::InspectContainer(realSandboxID, pluginErr, true);
+    if (pluginErr.NotEmpty()) {
+        ERROR("Failed to inspect container");
+        // not return and make sure teardown network
+    }
 
-        stdAnnos.insert(std::pair<std::string, std::string>(CRIHelpers::Constants::POD_SANDBOX_KEY, netnsPath));
-        m_pluginManager->TearDownPod(ns, name, Network::DEFAULT_NETWORK_INTERFACE_NAME, realSandboxID, stdAnnos,
-                                     pluginErr);
-        if (pluginErr.NotEmpty()) {
-            WARN("TearDownPod cni network failed: %s", pluginErr.GetCMessage());
-            errlist.push_back(pluginErr.GetMessage());
+    if (inspect_data != nullptr && namespace_is_file(inspect_data->host_config->network_mode)) {
+        netnsPath = get_sandbox_key(inspect_data);
+        if (netnsPath == nullptr) {
+            ERROR("Get sandbox key failed");
+            // not return and make sure teardown network
         } else {
-            INFO("TearDownPod cni network: success");
-            SetNetworkReady(realSandboxID, false, pluginErr);
-            if (pluginErr.NotEmpty()) {
-                WARN("set network ready: %s", pluginErr.GetCMessage());
-            }
-            // umount netns when cni removed network successfully
-            if (util_umount_namespace(netnsPath) != 0) {
-                ERROR("Failed to umount directory %s:%s", netnsPath, strerror(errno));
-            }
+            stdAnnos.insert(std::pair<std::string, std::string>(CRIHelpers::Constants::POD_SANDBOX_KEY,
+                                                    std::string(netnsPath)));
         }
-        free(netnsPath);
     }
+
+    m_pluginManager->TearDownPod(ns, name, Network::DEFAULT_NETWORK_INTERFACE_NAME, realSandboxID, stdAnnos,
+                                    pluginErr);
+    if (pluginErr.NotEmpty()) {
+        WARN("TearDownPod cni network failed: %s", pluginErr.GetCMessage());
+        errlist.push_back(pluginErr.GetMessage());
+        goto out;
+    }
+
+    INFO("TearDownPod cni network: success");
+    SetNetworkReady(realSandboxID, false, pluginErr);
+    if (pluginErr.NotEmpty()) {
+        WARN("set network ready: %s", pluginErr.GetCMessage());
+    }
+    // umount netns when cni removed network successfully
+    if (inspect_data != nullptr && namespace_is_file(inspect_data->host_config->network_mode) &&
+        util_umount_namespace(netnsPath) != 0) {
+        ERROR("Failed to umount directory %s:%s", netnsPath, strerror(errno));
+    }
+
+out:
+    free(netnsPath);
     free_container_inspect(inspect_data);
     return 0;
 }
diff --git a/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.h b/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.h
index 68a9d919..f7c0aa00 100644
--- a/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.h
+++ b/src/daemon/entry/cri/cri_pod_sandbox_manager_service_impl.h
@@ -66,7 +66,7 @@ private:
                                                const std::string &image, std::string &jsonCheckpoint,
                                                const std::string &runtimeHandler,
                                                Errors &error) -> container_create_request *;
-    void SetHostConfigDefaultValue(host_config *hc);
+    void SetHostConfigDefaultValue(const google::protobuf::Map<std::string, std::string> &annotations, host_config *hc);
     void MakeSandboxIsuladConfig(const runtime::v1alpha2::PodSandboxConfig &c, host_config *hc,
                                  container_config *custom_config, Errors &error);
     void ApplySandboxLinuxOptions(const runtime::v1alpha2::LinuxPodSandboxConfig &lc, host_config *hc,
diff --git a/src/daemon/entry/cri/cri_security_context.cc b/src/daemon/entry/cri/cri_security_context.cc
index 8484108c..1cd9287d 100644
--- a/src/daemon/entry/cri/cri_security_context.cc
+++ b/src/daemon/entry/cri/cri_security_context.cc
@@ -162,7 +162,8 @@ static void ModifyHostNetworkOptionForSandbox(const runtime::v1alpha2::Namespace
         free(hostConfig->uts_mode);
         hostConfig->uts_mode = util_strdup_s(CRI::Constants::namespaceModeHost.c_str());
     }
-    // Note: default networkMode is file
+
+    // Set networkMode to file or "" in SetHostConfigDefaultValue
 }
 
 static void ModifyContainerNamespaceOptions(const runtime::v1alpha2::NamespaceOption &nsOpts,
diff --git a/src/daemon/modules/spec/specs.c b/src/daemon/modules/spec/specs.c
index ec6385a6..138a0989 100644
--- a/src/daemon/modules/spec/specs.c
+++ b/src/daemon/modules/spec/specs.c
@@ -1460,7 +1460,7 @@ static int merge_share_network_namespace(oci_runtime_spec *oci_spec, const host_
     int ret = 0;
     char *ns_path = NULL;
 
-    if (host_spec->network_mode == NULL) {
+    if (host_spec->network_mode == NULL || strlen(host_spec->network_mode) == 0) {
         return 0;
     }
 
diff --git a/src/utils/cutils/utils_file.c b/src/utils/cutils/utils_file.c
index f06f4d49..7b1d5cb6 100644
--- a/src/utils/cutils/utils_file.c
+++ b/src/utils/cutils/utils_file.c
@@ -85,7 +85,7 @@ bool util_file_exists(const char *f)
     struct stat buf;
     int nret;
 
-    if (f == NULL) {
+    if (f == NULL || strlen(f) == 0) {
         return false;
     }
 
-- 
2.25.1