From fe3413bb8ebae90f29ce3cc02373f3fc2b5d2fd2 Mon Sep 17 00:00:00 2001 From: jikai Date: Mon, 22 Jan 2024 20:19:29 +0800 Subject: [PATCH 08/43] bug fix for device/cgroup/ulimt oci update Signed-off-by: jikai --- .../executor/container_cb/execution_create.c | 7 ++- src/daemon/modules/api/specs_api.h | 4 ++ .../modules/service/service_container.c | 18 +++--- src/daemon/modules/spec/specs.c | 60 +++++++++++++++---- 4 files changed, 63 insertions(+), 26 deletions(-) diff --git a/src/daemon/executor/container_cb/execution_create.c b/src/daemon/executor/container_cb/execution_create.c index ca2a9163..e00afb68 100644 --- a/src/daemon/executor/container_cb/execution_create.c +++ b/src/daemon/executor/container_cb/execution_create.c @@ -533,12 +533,15 @@ static int merge_config_for_syscontainer(const container_create_request *request value = request->rootfs; } - if (append_json_map_string_string(oci_spec->annotations, "rootfs.mount", value)) { + // should also update to container spec + if (append_json_map_string_string(container_spec->annotations, "rootfs.mount", value) + || append_json_map_string_string(oci_spec->annotations, "rootfs.mount", value)) { ERROR("Realloc annotations failed"); ret = -1; goto out; } - if (request->rootfs != NULL && append_json_map_string_string(oci_spec->annotations, "external.rootfs", "true")) { + if (request->rootfs != NULL && (append_json_map_string_string(container_spec->annotations, "external.rootfs", "true") + || append_json_map_string_string(oci_spec->annotations, "external.rootfs", "true"))) { ERROR("Realloc annotations failed"); ret = -1; goto out; diff --git a/src/daemon/modules/api/specs_api.h b/src/daemon/modules/api/specs_api.h index f5f6ad8b..f54c0d31 100644 --- a/src/daemon/modules/api/specs_api.h +++ b/src/daemon/modules/api/specs_api.h @@ -47,6 +47,10 @@ oci_runtime_spec *load_oci_config(const char *rootpath, const char *name); oci_runtime_spec *default_spec(bool system_container); +int update_oci_container_cgroups_path(const char *id, oci_runtime_spec *oci_spec, const host_config *host_spec); + +int update_oci_ulimit(oci_runtime_spec *oci_spec, const host_config *host_spec); + const oci_runtime_spec *get_readonly_default_oci_spec(bool system_container); int spec_module_init(void); diff --git a/src/daemon/modules/service/service_container.c b/src/daemon/modules/service/service_container.c index 239783b8..a3606a82 100644 --- a/src/daemon/modules/service/service_container.c +++ b/src/daemon/modules/service/service_container.c @@ -693,26 +693,21 @@ out: static int do_oci_spec_update(const char *id, oci_runtime_spec *oci_spec, container_config *container_spec, host_config *hostconfig) { - __isula_auto_free char *cgroup_parent = NULL; int ret; - // First renew annotations for oci spec, cgroup path, rootfs.mount, native.mask - // for iSulad daemon might get updated + // Renew annotations for oci spec, cgroup path only, + // since lxc uses the "cgroup.dir" in oci annotations to create cgroup + // should ensure that container spec has the same annotations as oci spec ret = update_spec_annotations(oci_spec, container_spec, hostconfig); if (ret < 0) { return -1; } // If isulad daemon cgroup parent updated, we should update this config into oci spec - cgroup_parent = merge_container_cgroups_path(id, hostconfig); - if (cgroup_parent == NULL) { + ret = update_oci_container_cgroups_path(id, oci_spec, hostconfig); + if (ret < 0) { return -1; } - if (oci_spec->linux->cgroups_path != NULL && strcmp(oci_spec->linux->cgroups_path, cgroup_parent) != 0) { - free(oci_spec->linux->cgroups_path); - oci_spec->linux->cgroups_path = cgroup_parent; - cgroup_parent = NULL; - } // For Linux.Resources, isula update will save changes into oci spec; // so we just skip it; @@ -725,7 +720,8 @@ static int do_oci_spec_update(const char *id, oci_runtime_spec *oci_spec, contai } // If isulad daemon ulimit updated, we should update this config into oci spec. - if (merge_global_ulimit(oci_spec) != 0) { + ret = update_oci_ulimit(oci_spec, hostconfig); + if (ret < 0) { return -1; } diff --git a/src/daemon/modules/spec/specs.c b/src/daemon/modules/spec/specs.c index 62e340b1..464b4fb4 100644 --- a/src/daemon/modules/spec/specs.c +++ b/src/daemon/modules/spec/specs.c @@ -402,19 +402,8 @@ int update_spec_annotations(oci_runtime_spec *oci_spec, container_config *contai return -1; } - /* add rootfs.mount */ - ret = add_rootfs_mount(container_spec); - if (ret != 0) { - ERROR("Failed to add rootfs mount"); - return -1; - } - - /* add native.umask */ - ret = add_native_umask(container_spec); - if (ret != 0) { - ERROR("Failed to add native umask"); - return -1; - } + // other annotations will either not be updated after containers created + // or for rootfs mnt and umask, we do not support the update operation if (merge_annotations(oci_spec, container_spec)) { return -1; @@ -2302,6 +2291,27 @@ char *merge_container_cgroups_path(const char *id, const host_config *host_spec) return util_path_join(path, id); } +int update_oci_container_cgroups_path(const char *id, oci_runtime_spec *oci_spec, const host_config *hostconfig) +{ + if (oci_spec == NULL || oci_spec->linux == NULL) { + ERROR("Invalid arguments"); + return -1; + } + + __isula_auto_free char *cgroup_parent = merge_container_cgroups_path(id, hostconfig); + if (cgroup_parent == NULL) { + return -1; + } + + if (oci_spec->linux->cgroups_path != NULL && strcmp(oci_spec->linux->cgroups_path, cgroup_parent) != 0) { + free(oci_spec->linux->cgroups_path); + oci_spec->linux->cgroups_path = cgroup_parent; + cgroup_parent = NULL; + } + + return 0; +} + static int merge_oci_cgroups_path(const char *id, oci_runtime_spec *oci_spec, const host_config *host_spec) { if (id == NULL || oci_spec == NULL || host_spec == NULL) { @@ -2445,6 +2455,30 @@ out: return ret; } +int update_oci_ulimit(oci_runtime_spec *oci_spec, const host_config *hostconfig) { + if (oci_spec == NULL || hostconfig == NULL) { + ERROR("Invalid arguments"); + return -1; + } + + size_t i = 0; + if (oci_spec->process != NULL) { + for (i = 0; i < oci_spec->process->rlimits_len; i++) { + free_defs_process_rlimits_element(oci_spec->process->rlimits[i]); + oci_spec->process->rlimits[i] = NULL; + } + free(oci_spec->process->rlimits); + oci_spec->process->rlimits = NULL; + oci_spec->process->rlimits_len = 0; + } + + if (merge_conf_ulimits(oci_spec, hostconfig) != 0 || merge_global_ulimit(oci_spec) != 0) { + return -1; + } + + return 0; +} + /* read oci config */ oci_runtime_spec *load_oci_config(const char *rootpath, const char *name) { -- 2.34.1