Update cgroup resolver to try all possible base paths

src/shared/metadata/cgroup_path_resolver.cc

@@ -27,12 +27,13 @@
 #include "src/common/fs/fs_wrapper.h"
 #include "src/shared/metadata/cgroup_path_resolver.h"
 
-DEFINE_bool(force_cgroup2_mode, true, "Flag to force assume cgroup2 fs for testing purposes");
+DEFINE_bool(force_cgroup2_mode, false, "Flag to force assume cgroup2 fs for testing purposes");
 
 namespace px {
 namespace md {
 
-StatusOr<std::string> CGroupBasePath(std::string_view sysfs_path) {
+StatusOr<std::vector<std::string>> CGroupBasePaths(std::string_view sysfs_path) {
+ std::vector<std::string> base_paths;
  // Different hosts may mount different cgroup dirs. Try a couple for robustness.
  const std::vector<std::string> cgroup_dirs = {"cpu,cpuacct", "cpu", "pids"};
 
@@ -43,7 +44,7 @@ StatusOr<std::string> CGroupBasePath(std::string_view sysfs_path) {
  std::string base_path = absl::StrCat(sysfs_path, "/cgroup/", cgroup_dir);
 
  if (fs::Exists(base_path)) {
- return base_path;
+ base_paths.push_back(base_path);
  }
  }
 
@@ -53,11 +54,17 @@ StatusOr<std::string> CGroupBasePath(std::string_view sysfs_path) {
  bool cgroupv2 = (fs_status == 0) && (info.f_type == CGROUP2_SUPER_MAGIC);
 
  if (cgroupv2 || FLAGS_force_cgroup2_mode) {
- return cgv2_base_path;
+ if (FLAGS_force_cgroup2_mode) {
+ return std::vector{cgv2_base_path};
+ }
+ base_paths.push_back(cgv2_base_path);
  }
  // (TODO): This check for cgroup2FS is eventually to be moved above the cgroupv1 check.
 
- return error::NotFound("Could not find CGroup base path");
+ if (base_paths.empty()) {
+ return error::NotFound("Could not find CGroup base path");
+ }
+ return base_paths;
 }
 
 StatusOr<std::string> FindSelfCGroupProcs(std::string_view base_path) {
@@ -137,17 +144,32 @@ StatusOr<CGroupTemplateSpec> CreateCGroupTemplateSpecFromPath(std::string_view p
 }
 
 StatusOr<CGroupTemplateSpec> AutoDiscoverCGroupTemplate(std::string_view sysfs_path) {
- PX_ASSIGN_OR_RETURN(std::string base_path, CGroupBasePath(sysfs_path));
- LOG(INFO) << "Auto-discovered CGroup base path: " << base_path;
-
- PX_ASSIGN_OR_RETURN(std::string self_cgroup_procs, FindSelfCGroupProcs(base_path));
- LOG(INFO) << "Auto-discovered example path: " << self_cgroup_procs;
-
- PX_ASSIGN_OR_RETURN(CGroupTemplateSpec cgroup_path_template,
- CreateCGroupTemplateSpecFromPath(self_cgroup_procs));
- LOG(INFO) << "Auto-discovered template: " << cgroup_path_template.templated_path;
+ PX_ASSIGN_OR_RETURN(std::vector<std::string> base_paths, CGroupBasePaths(sysfs_path));
+ for (const auto& base_path : base_paths) {
+ LOG(INFO) << "Auto-discovered CGroup base path: " << base_path;
+
+ auto self_cgroup_procs_status = FindSelfCGroupProcs(base_path);
+ if (!self_cgroup_procs_status.ok()) {
+ LOG(WARNING) << "Could not find self in cgroup procs. Trying next base path.";
+ continue;
+ }
+ auto self_cgroup_procs = self_cgroup_procs_status.ConsumeValueOrDie();
+ LOG(INFO) << "Auto-discovered example path: " << self_cgroup_procs;
+
+ auto cgroup_path_template_status = CreateCGroupTemplateSpecFromPath(self_cgroup_procs);
+ if (!cgroup_path_template_status.ok()) {
+ LOG(WARNING) << absl::Substitute(
+ "Failed to create cgroup template spec from path $0. Trying next base path.",
+ self_cgroup_procs);
+ continue;
+ }
+ auto cgroup_path_template = cgroup_path_template_status.ConsumeValueOrDie();
+ LOG(INFO) << "Auto-discovered template: " << cgroup_path_template.templated_path;
 
- return cgroup_path_template;
+ return cgroup_path_template;
+ }
+ return error::NotFound("Unable to auto discover cgroup template from $0",
+ absl::StrJoin(base_paths, ", "));
 }
 
 StatusOr<std::unique_ptr<CGroupPathResolver>> CGroupPathResolver::Create(
@@ -222,51 +244,52 @@ Status LegacyCGroupPathResolver::Init(std::string_view sysfs_path) {
  // $2 = container runtime
  // These template parameters are resolved by calls to PodPath.
  // Different hosts may mount different cgroup dirs. Try a couple for robustness.
- PX_ASSIGN_OR_RETURN(std::string cgroup_dir, CGroupBasePath(sysfs_path));
-
- // Attempt assuming naming scheme #1.
- std::string cgroup_kubepods_base_path = absl::Substitute("$0/kubepods", cgroup_dir);
- if (fs::Exists(cgroup_kubepods_base_path)) {
- cgroup_kubepod_guaranteed_path_template_ =
- absl::StrCat(cgroup_kubepods_base_path, "/pod$0/$1/cgroup.procs");
- cgroup_kubepod_besteffort_path_template_ =
- absl::StrCat(cgroup_kubepods_base_path, "/besteffort/pod$0/$1/cgroup.procs");
- cgroup_kubepod_burstable_path_template_ =
- absl::StrCat(cgroup_kubepods_base_path, "/burstable/pod$0/$1/cgroup.procs");
- cgroup_kubepod_convert_dashes_ = false;
- return Status::OK();
- }
+ PX_ASSIGN_OR_RETURN(std::vector<std::string> cgroup_dirs, CGroupBasePaths(sysfs_path));
 
- // Attempt assuming naming scheme #3.
- // Must be before the scheme below, since there have been systems that have both paths,
- // but this must take priority.
- cgroup_kubepods_base_path = absl::Substitute("$0/system.slice/containerd.service", cgroup_dir);
- if (fs::Exists(cgroup_kubepods_base_path)) {
- cgroup_kubepod_guaranteed_path_template_ =
- absl::StrCat(cgroup_kubepods_base_path, "/kubepods-pod$0.slice:$2:$1/cgroup.procs");
- cgroup_kubepod_besteffort_path_template_ = absl::StrCat(
- cgroup_kubepods_base_path, "/kubepods-besteffort-pod$0.slice:$2:$1/cgroup.procs");
- cgroup_kubepod_burstable_path_template_ = absl::StrCat(
- cgroup_kubepods_base_path, "/kubepods-burstable-pod$0.slice:$2:$1/cgroup.procs");
- cgroup_kubepod_convert_dashes_ = true;
- return Status::OK();
- }
+ for (const auto& cgroup_dir : cgroup_dirs) {
+ // Attempt assuming naming scheme #1.
+ std::string cgroup_kubepods_base_path = absl::Substitute("$0/kubepods", cgroup_dir);
+ if (fs::Exists(cgroup_kubepods_base_path)) {
+ cgroup_kubepod_guaranteed_path_template_ =
+ absl::StrCat(cgroup_kubepods_base_path, "/pod$0/$1/cgroup.procs");
+ cgroup_kubepod_besteffort_path_template_ =
+ absl::StrCat(cgroup_kubepods_base_path, "/besteffort/pod$0/$1/cgroup.procs");
+ cgroup_kubepod_burstable_path_template_ =
+ absl::StrCat(cgroup_kubepods_base_path, "/burstable/pod$0/$1/cgroup.procs");
+ cgroup_kubepod_convert_dashes_ = false;
+ return Status::OK();
+ }
 
- // Attempt assuming naming scheme #2.
- cgroup_kubepods_base_path = absl::Substitute("$0/kubepods.slice", cgroup_dir);
- if (fs::Exists(cgroup_kubepods_base_path)) {
- cgroup_kubepod_guaranteed_path_template_ =
-  absl::StrCat(cgroup_kubepods_base_path, "/kubepods-pod$0.slice/$2-$1.scope/cgroup.procs");
- cgroup_kubepod_besteffort_path_template_ = absl::StrCat(
- cgroup_kubepods_base_path,
-  "/kubepods-besteffort.slice/kubepods-besteffort-pod$0.slice/$2-$1.scope/cgroup.procs");
- cgroup_kubepod_burstable_path_template_ = absl::StrCat(
-  cgroup_kubepods_base_path,
- "/kubepods-burstable.slice/kubepods-burstable-pod$0.slice/$2-$1.scope/cgroup.procs");
- cgroup_kubepod_convert_dashes_ = true;
- return Status::OK();
- }
+  // Attempt assuming naming scheme #3.
+  // Must be before the scheme below, since there have been systems that have both paths,
+  // but this must take priority.
+ cgroup_kubepods_base_path = absl::Substitute("$0/system.slice/containerd.service", cgroup_dir);
+ if (fs::Exists(cgroup_kubepods_base_path)) {
+  cgroup_kubepod_guaranteed_path_template_ =
+  absl::StrCat(cgroup_kubepods_base_path, "/kubepods-pod$0.slice:$2:$1/cgroup.procs");
+ cgroup_kubepod_besteffort_path_template_ = absl::StrCat(
+  cgroup_kubepods_base_path, "/kubepods-besteffort-pod$0.slice:$2:$1/cgroup.procs");
+ cgroup_kubepod_burstable_path_template_ = absl::StrCat(
+  cgroup_kubepods_base_path, "/kubepods-burstable-pod$0.slice:$2:$1/cgroup.procs");
+  cgroup_kubepod_convert_dashes_ = true;
+  return Status::OK();
+  }
 
+ // Attempt assuming naming scheme #2.
+ cgroup_kubepods_base_path = absl::Substitute("$0/kubepods.slice", cgroup_dir);
+ if (fs::Exists(cgroup_kubepods_base_path)) {
+ cgroup_kubepod_guaranteed_path_template_ =
+ absl::StrCat(cgroup_kubepods_base_path, "/kubepods-pod$0.slice/$2-$1.scope/cgroup.procs");
+ cgroup_kubepod_besteffort_path_template_ = absl::StrCat(
+ cgroup_kubepods_base_path,
+ "/kubepods-besteffort.slice/kubepods-besteffort-pod$0.slice/$2-$1.scope/cgroup.procs");
+ cgroup_kubepod_burstable_path_template_ = absl::StrCat(
+ cgroup_kubepods_base_path,
+ "/kubepods-burstable.slice/kubepods-burstable-pod$0.slice/$2-$1.scope/cgroup.procs");
+ cgroup_kubepod_convert_dashes_ = true;
+ return Status::OK();
+ }
+ }
  return error::NotFound("Could not find kubepods slice under sysfs ($0)", sysfs_path);
 }
 

src/shared/metadata/cgroup_path_resolver_test.cc

@@ -317,6 +317,7 @@ TEST(LegacyCGroupPathResolverTest, StandardFormat) {
 }
 
 TEST(LeagcyCGroupPathResolverTest, Cgroup2Format) {
+ PX_SET_FOR_SCOPE(FLAGS_force_cgroup2_mode, true);
  ASSERT_OK_AND_ASSIGN(
  auto path_resolver,
  LegacyCGroupPathResolver::Create(GetSysFsPathFromTestDataFile(
@@ -326,7 +327,6 @@ TEST(LeagcyCGroupPathResolverTest, Cgroup2Format) {
  "cgroup.procs",
  "testdata/sysfs3")));
 
- FLAGS_force_cgroup2_mode = true;
  EXPECT_EQ(
  GetPathToTestDataFile(
  "testdata/sysfs3/cgroup/kubepods.slice/kubepods-besteffort.slice/"
@@ -348,6 +348,46 @@ TEST(LeagcyCGroupPathResolverTest, Cgroup2Format) {
  ContainerType::kDocker));
 }
 
+// This tests simulates a system where cgroup v1 and v2 are both present. This has been observed
+// in OVH's managed kubernetes service.
+//
+// The testdata/sysfs4-cgroup-v1andv2 directory was created with the following commands:
+// sudo mount -t overlay overlay -o lowerdir=sysfs2/cgroup:sysfs3/cgroup \
+// sysfs4-cgroup-v1andv2/cgroup
+//
+// sysfsv2 and sysfsv3 model cgroupv1 and cgroupv2 respectively.
+TEST(LeagcyCGroupPathResolverTest, Cgroup2With1) {
+ PX_SET_FOR_SCOPE(FLAGS_force_cgroup2_mode, true);
+ ASSERT_OK_AND_ASSIGN(
+ auto path_resolver,
+ LegacyCGroupPathResolver::Create(GetSysFsPathFromTestDataFile(
+ "testdata/sysfs4-cgroup-v1andv2/cgroup/kubepods.slice/kubepods-besteffort.slice/"
+ "kubepods-besteffort-pod47810e8e_b9cb_4ac6_b12d_9e0577fa8237.slice/"
+ "docker-28efca84cc7d707bdfbc5646144bba6c4417de2cf63f8583179603ce434d6dfe.scope/"
+ "cgroup.procs",
+ "testdata/sysfs4-cgroup-v1andv2")));
+
+ EXPECT_EQ(
+ GetPathToTestDataFile(
+ "testdata/sysfs4-cgroup-v1andv2/cgroup/kubepods.slice/kubepods-besteffort.slice/"
+ "kubepods-besteffort-pod47810e8e_b9cb_4ac6_b12d_9e0577fa8237.slice/"
+ "docker-28efca84cc7d707bdfbc5646144bba6c4417de2cf63f8583179603ce434d6dfe.scope/"
+ "cgroup.procs"),
+ path_resolver->PodPath(PodQOSClass::kBestEffort, "47810e8e_b9cb_4ac6_b12d_9e0577fa8237",
+ "28efca84cc7d707bdfbc5646144bba6c4417de2cf63f8583179603ce434d6dfe",
+ ContainerType::kDocker));
+
+ EXPECT_EQ(
+ GetPathToTestDataFile(
+ "testdata/sysfs4-cgroup-v1andv2/cgroup/kubepods.slice/kubepods-burstable.slice/"
+ "kubepods-burstable-pod16de73f898f4460d96d28cf19ba8407f.slice/"
+ "docker-23ac1540f833b029f76af6a513c4861a54bb9b77a6e3648b6f8392b1a09686ba.scope/"
+ "cgroup.procs"),
+ path_resolver->PodPath(PodQOSClass::kBurstable, "16de73f898f4460d96d28cf19ba8407f",
+ "23ac1540f833b029f76af6a513c4861a54bb9b77a6e3648b6f8392b1a09686ba",
+ ContainerType::kDocker));
+}
+
 TEST(CGroupPathResolver, Cgroup2Format) {
  std::string cgroup_kubepod_path =
  "/sys/fs/cgroup/kubepods.slice/"

src/shared/metadata/testdata/sysfs4-cgroup-v1andv2/cgroup/cpu,cpuacct/kubepods.slice/kubepods-besteffort.slice/kubepods-besteffort-podd7f73b62_b4bf_41dc_a50d_5bdf3b02d3e5.slice/docker-37d1600d21282ce1bc32ebbaf99bf2241f66ca096109a4cb5484e2aa305514b4.scope/cgroup.procs

@@ -0,0 +1,3 @@
+123
+456
+789