[mesos] 02/02: Changed agent to not report disconnected resource providers.

[bb...@apache.org]

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

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

commit 7447fbc45d1375dd64644a3b130007da061fd5dc
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Fri Jan 11 09:44:24 2019 +0100

    Changed agent to not report disconnected resource providers.
    
    This patch changes the agent to not report disconnected resource
    providers in the `UpdateSlaveMessage` it emits. We also fix how it
    generates resource provider-related updates when no resource providers
    are connected to allow the master to react properly when a resource
    provider disappears.
    
    Review: https://reviews.apache.org/r/69338/
---
 src/slave/slave.cpp                           | 19 +++++++++----------
 src/tests/resource_provider_manager_tests.cpp | 13 +++----------
 2 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index ad3b693..10cbc19 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -7673,6 +7673,11 @@ UpdateSlaveMessage Slave::generateResourceProviderUpdate() const
     }
   }
 
+  // Always add a `resource_providers` field so we can distinguish the
+  // empty and unset case.
+  UpdateSlaveMessage::ResourceProviders* providers =
+    message.mutable_resource_providers();
+
   foreachvalue (ResourceProvider* resourceProvider, resourceProviders) {
     // If the resource provider has not updated its state we do not
     // need to and cannot include its information in an
@@ -7681,8 +7686,7 @@ UpdateSlaveMessage Slave::generateResourceProviderUpdate() const
       continue;
     }
 
-    UpdateSlaveMessage::ResourceProvider* provider =
-      message.mutable_resource_providers()->add_providers();
+    UpdateSlaveMessage::ResourceProvider* provider = providers->add_providers();
 
     provider->mutable_info()->CopyFrom(
         resourceProvider->info);
@@ -7961,17 +7965,12 @@ void Slave::handleResourceProviderMessage(
         break;
       }
 
-      // A disconnected resource provider effectively results in its
-      // total resources to be set to empty. This will cause offers to
-      // be rescinded so that no operation or task can be launched
-      // using resources from the disconnected resource provider. If
-      // later, the resource provider reconnects, it'll result in
-      // an `UpdateSlaveMessage` so that its resources can be offered
-      // again by the master.
+      // Remove the resource provider's resources from the agent's
+      // total resources and remove it from our internal tracking.
       CHECK(totalResources.contains(resourceProvider->totalResources));
       totalResources -= resourceProvider->totalResources;
 
-      resourceProvider->totalResources = Resources();
+      resourceProviders.erase(resourceProviderId);
 
       // Send the updated resources to the master if the agent is running. Note
       // that since we have already updated our copy of the latest resource
diff --git a/src/tests/resource_provider_manager_tests.cpp b/src/tests/resource_provider_manager_tests.cpp
index 7049698..20cfb34 100644
--- a/src/tests/resource_provider_manager_tests.cpp
+++ b/src/tests/resource_provider_manager_tests.cpp
@@ -1350,16 +1350,9 @@ TEST_P(ResourceProviderManagerHttpApiTest, ResourceProviderDisconnect)
   // Simulate a resource provider disconnection.
   resourceProvider.reset();
 
-  {
-    AWAIT_READY(updateSlaveMessage);
-    ASSERT_TRUE(updateSlaveMessage->has_resource_providers());
-    ASSERT_EQ(1, updateSlaveMessage->resource_providers().providers_size());
-
-    const Resources& totalResources =
-      updateSlaveMessage->resource_providers().providers(0).total_resources();
-
-    EXPECT_FALSE(totalResources.contains(devolve(disk)));
-  }
+  AWAIT_READY(updateSlaveMessage);
+  ASSERT_TRUE(updateSlaveMessage->has_resource_providers());
+  EXPECT_EQ(0, updateSlaveMessage->resource_providers().providers_size());
 }