You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by qi...@apache.org on 2020/05/05 08:12:55 UTC

[mesos] 03/05: Updated Docker containerizer by not updating resources for command task.

This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 97dc2b069965929105d6241c57f8cb6ee77a5e35
Author: Qian Zhang <zh...@gmail.com>
AuthorDate: Tue Apr 21 16:46:17 2020 +0800

    Updated Docker containerizer by not updating resources for command task.
    
    For command task, its resources will be set when it is launched as a Docker
    container by Docker executor, and we do not need to update its resources
    afterward since we do not support task resizing. But for the case that a
    custom executor launched as a Docker container by Docker containerizer, we
    need to update its resources when it launches a new task or an existing task
    terminates.
    
    Review: https://reviews.apache.org/r/72401
---
 src/slave/containerizer/docker.cpp                 |  11 ++
 src/slave/containerizer/docker.hpp                 |   5 +-
 .../containerizer/docker_containerizer_tests.cpp   | 157 ---------------------
 3 files changed, 15 insertions(+), 158 deletions(-)

diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 492ac27..3aa6a99 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -1007,6 +1007,7 @@ Future<Nothing> DockerContainerizerProcess::_recover(
         Container* container = new Container(containerId);
         containers_[containerId] = container;
         container->state = Container::RUNNING;
+        container->generatedForCommandTask = executor.generatedForCommandTask;
         container->launchesExecutorContainer =
           executorContainers.contains(containerId);
 
@@ -1675,6 +1676,16 @@ Future<Nothing> DockerContainerizerProcess::update(
     return Nothing();
   }
 
+  if (container->generatedForCommandTask) {
+    LOG(INFO) << "Ignoring updating container " << containerId
+              << " because it is generated for a command task";
+
+    // Store the resources for usage().
+    container->resources = resourceRequests;
+
+    return Nothing();
+  }
+
   if (container->resources == resourceRequests && !force) {
     LOG(INFO) << "Ignoring updating container " << containerId
               << " because resources passed to update are identical to"
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index 09fc279..d3d5f3a 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -353,7 +353,8 @@ private:
         symlinked(symlinked),
         containerWorkDir(containerWorkDir),
         containerName(name(id)),
-        launchesExecutorContainer(launchesExecutorContainer)
+        launchesExecutorContainer(launchesExecutorContainer),
+        generatedForCommandTask(_containerConfig.has_task_info())
     {
       // NOTE: The task's resources are included in the executor's
       // resources in order to make sure when launching the executor
@@ -531,6 +532,8 @@ private:
     // Marks if this container launches an executor in a docker
     // container.
     bool launchesExecutorContainer;
+
+    bool generatedForCommandTask;
   };
 
   hashmap<ContainerID, Container*> containers_;
diff --git a/src/tests/containerizer/docker_containerizer_tests.cpp b/src/tests/containerizer/docker_containerizer_tests.cpp
index b069f51..42692dc 100644
--- a/src/tests/containerizer/docker_containerizer_tests.cpp
+++ b/src/tests/containerizer/docker_containerizer_tests.cpp
@@ -1086,163 +1086,6 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Usage)
 }
 
 
-#ifdef __linux__
-TEST_F(DockerContainerizerTest, ROOT_DOCKER_Update)
-{
-  Try<Owned<cluster::Master>> master = StartMaster();
-  ASSERT_SOME(master);
-
-  MockDocker* mockDocker =
-    new MockDocker(tests::flags.docker, tests::flags.docker_socket);
-
-  Shared<Docker> docker(mockDocker);
-
-  slave::Flags flags = CreateSlaveFlags();
-
-  Fetcher fetcher(flags);
-
-  Try<ContainerLogger*> logger =
-    ContainerLogger::create(flags.container_logger);
-
-  ASSERT_SOME(logger);
-
-  MockDockerContainerizer dockerContainerizer(
-      flags,
-      &fetcher,
-      Owned<ContainerLogger>(logger.get()),
-      docker);
-
-  Owned<MasterDetector> detector = master.get()->createDetector();
-
-  Try<Owned<cluster::Slave>> slave =
-    StartSlave(detector.get(), &dockerContainerizer, flags);
-  ASSERT_SOME(slave);
-
-  MockScheduler sched;
-  MesosSchedulerDriver driver(
-      &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
-
-  Future<FrameworkID> frameworkId;
-  EXPECT_CALL(sched, registered(&driver, _, _))
-    .WillOnce(FutureArg<1>(&frameworkId));
-
-  Future<vector<Offer>> offers;
-  EXPECT_CALL(sched, resourceOffers(&driver, _))
-    .WillOnce(FutureArg<1>(&offers))
-    .WillRepeatedly(Return()); // Ignore subsequent offers.
-
-  driver.start();
-
-  AWAIT_READY(frameworkId);
-
-  AWAIT_READY(offers);
-  ASSERT_FALSE(offers->empty());
-
-  TaskInfo task = createTask(
-      offers->front().slave_id(),
-      offers->front().resources(),
-      SLEEP_COMMAND(1000));
-
-  // TODO(tnachen): Use local image to test if possible.
-  task.mutable_container()->CopyFrom(createDockerInfo("alpine"));
-
-  Future<ContainerID> containerId;
-  EXPECT_CALL(dockerContainerizer, launch(_, _, _, _))
-    .WillOnce(DoAll(FutureArg<0>(&containerId),
-                    Invoke(&dockerContainerizer,
-                           &MockDockerContainerizer::_launch)));
-
-  Future<TaskStatus> statusStarting;
-  Future<TaskStatus> statusRunning;
-  EXPECT_CALL(sched, statusUpdate(&driver, _))
-    .WillOnce(FutureArg<1>(&statusStarting))
-    .WillOnce(FutureArg<1>(&statusRunning))
-    .WillRepeatedly(DoDefault());
-
-  driver.launchTasks(offers.get()[0].id(), {task});
-
-  AWAIT_READY(containerId);
-
-  AWAIT_READY_FOR(statusStarting, Seconds(60));
-  EXPECT_EQ(TASK_STARTING, statusStarting->state());
-
-  AWAIT_READY_FOR(statusRunning, Seconds(60));
-  EXPECT_EQ(TASK_RUNNING, statusRunning->state());
-
-  ASSERT_TRUE(
-    exists(docker, containerId.get(), ContainerState::RUNNING));
-
-  string name = containerName(containerId.get());
-
-  Future<Docker::Container> inspect = docker->inspect(name);
-
-  AWAIT_READY(inspect);
-
-  Try<Resources> newResources = Resources::parse("cpus:1;mem:128");
-
-  ASSERT_SOME(newResources);
-
-  Future<Nothing> update =
-    dockerContainerizer.update(containerId.get(), newResources.get(), {});
-
-  AWAIT_READY(update);
-
-  Result<string> cpuHierarchy = cgroups::hierarchy("cpu");
-  Result<string> memoryHierarchy = cgroups::hierarchy("memory");
-
-  ASSERT_SOME(cpuHierarchy);
-  ASSERT_SOME(memoryHierarchy);
-
-  Option<pid_t> pid = inspect->pid;
-  ASSERT_SOME(pid);
-
-  Result<string> cpuCgroup = cgroups::cpu::cgroup(pid.get());
-  ASSERT_SOME(cpuCgroup);
-
-  Result<string> memoryCgroup = cgroups::memory::cgroup(pid.get());
-  ASSERT_SOME(memoryCgroup);
-
-  Try<uint64_t> cpu = cgroups::cpu::shares(
-      cpuHierarchy.get(),
-      cpuCgroup.get());
-
-  ASSERT_SOME(cpu);
-
-  Try<Bytes> mem = cgroups::memory::soft_limit_in_bytes(
-      memoryHierarchy.get(),
-      memoryCgroup.get());
-
-  ASSERT_SOME(mem);
-
-  EXPECT_EQ(1024u, cpu.get());
-  EXPECT_EQ(128u, mem->bytes() / Bytes::MEGABYTES);
-
-  newResources = Resources::parse("cpus:1;mem:144");
-
-  // Issue second update that uses the cached cgroups instead of inspect.
-  update = dockerContainerizer.update(containerId.get(), newResources.get(), {});
-
-  AWAIT_READY(update);
-
-  cpu = cgroups::cpu::shares(cpuHierarchy.get(), cpuCgroup.get());
-
-  ASSERT_SOME(cpu);
-
-  mem = cgroups::memory::soft_limit_in_bytes(
-      memoryHierarchy.get(),
-      memoryCgroup.get());
-
-  ASSERT_SOME(mem);
-
-  EXPECT_EQ(1024u, cpu.get());
-  EXPECT_EQ(144u, mem->bytes() / Bytes::MEGABYTES);
-
-  driver.stop();
-  driver.join();
-}
-#endif // __linux__
-
-
 TEST_F(DockerContainerizerTest, ROOT_DOCKER_Recover)
 {
   MockDocker* mockDocker =