Revert "Added admitted resource providers to the manager's registry."
This reverts commit 23ff6622fde41f40ee1d4ee914b5302c7b25de01. Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2855a6b6 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2855a6b6 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2855a6b6 Branch: refs/heads/master Commit: 2855a6b607510e4114b22bddf43197c72176995e Parents: f13a6e2 Author: Alexander Rukletsov <[email protected]> Authored: Wed Apr 25 17:07:20 2018 +0200 Committer: Alexander Rukletsov <[email protected]> Committed: Wed Apr 25 17:07:20 2018 +0200 ---------------------------------------------------------------------- src/resource_provider/manager.cpp | 45 ++-------------------- src/resource_provider/registrar.cpp | 10 ----- src/resource_provider/registry.proto | 1 - src/tests/resource_provider_manager_tests.cpp | 28 ++++---------- 4 files changed, 11 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/2855a6b6/src/resource_provider/manager.cpp ---------------------------------------------------------------------- diff --git a/src/resource_provider/manager.cpp b/src/resource_provider/manager.cpp index 1219bb7..32f23a7 100644 --- a/src/resource_provider/manager.cpp +++ b/src/resource_provider/manager.cpp @@ -58,7 +58,6 @@ using std::string; using mesos::internal::resource_provider::validation::call::validate; -using mesos::resource_provider::AdmitResourceProvider; using mesos::resource_provider::Call; using mesos::resource_provider::Event; using mesos::resource_provider::Registrar; @@ -185,10 +184,6 @@ private: const HttpConnection& http, const Call::Subscribe& subscribe); - void _subscribe( - const Future<bool>& admitResourceProvider, - Owned<ResourceProvider> resourceProvider); - void updateOperationStatus( ResourceProvider* resourceProvider, const Call::UpdateOperationStatus& update); @@ -662,53 +657,17 @@ void ResourceProviderManagerProcess::subscribe( Owned<ResourceProvider> resourceProvider( new ResourceProvider(resourceProviderInfo, http)); - Future<bool> admitResourceProvider; - if (!resourceProviderInfo.has_id()) { // The resource provider is subscribing for the first time. resourceProvider->info.mutable_id()->CopyFrom(newResourceProviderId()); - - // If we are handing out a new `ResourceProviderID` persist the ID by - // triggering a `AdmitResourceProvider` operation on the registrar. - admitResourceProvider = - registrar->apply(Owned<mesos::resource_provider::Registrar::Operation>( - new AdmitResourceProvider(resourceProvider->info.id()))); } else { // TODO(chhsiao): The resource provider is resubscribing after being // restarted or an agent failover. The 'ResourceProviderInfo' might // have been updated, but its type and name should remain the same. // We should checkpoint its 'type', 'name' and ID, then check if the // resubscribption is consistent with the checkpointed record. - - // If the resource provider is known we do not need to admit it - // again, and the registrar operation implicitly succeeded. - admitResourceProvider = true; } - admitResourceProvider.onAny(defer( - self(), - &ResourceProviderManagerProcess::_subscribe, - lambda::_1, - std::move(resourceProvider))); -} - - -void ResourceProviderManagerProcess::_subscribe( - const Future<bool>& admitResourceProvider, - Owned<ResourceProvider> resourceProvider) -{ - if (!admitResourceProvider.isReady()) { - LOG(INFO) - << "Not subscribing resource provider " << resourceProvider->info.id() - << " as registry update did not succeed: " << admitResourceProvider; - - return; - } - - CHECK(admitResourceProvider.get()) - << "Could not admit resource provider " << resourceProvider->info.id() - << " as registry update was rejected"; - const ResourceProviderID& resourceProviderId = resourceProvider->info.id(); Event event; @@ -722,7 +681,7 @@ void ResourceProviderManagerProcess::_subscribe( return; } - resourceProvider->http.closed() + http.closed() .onAny(defer(self(), [=](const Future<Nothing>& future) { // Iff the remote side closes the HTTP connection, the future will be // ready. We will remove the resource provider in that case. @@ -757,6 +716,8 @@ void ResourceProviderManagerProcess::_subscribe( resourceProviders.known.put( resourceProviderId, std::move(resourceProvider_)); + + // TODO(bbannier): Persist this information in the registry. } } http://git-wip-us.apache.org/repos/asf/mesos/blob/2855a6b6/src/resource_provider/registrar.cpp ---------------------------------------------------------------------- diff --git a/src/resource_provider/registrar.cpp b/src/resource_provider/registrar.cpp index c506ec1..d7ec6a6 100644 --- a/src/resource_provider/registrar.cpp +++ b/src/resource_provider/registrar.cpp @@ -115,15 +115,6 @@ Try<bool> AdmitResourceProvider::perform(Registry* registry) return Error("Resource provider already admitted"); } - if (std::find_if( - registry->removed_resource_providers().begin(), - registry->removed_resource_providers().end(), - [this](const ResourceProvider& resourceProvider) { - return resourceProvider.id() == this->id; - }) != registry->removed_resource_providers().end()) { - return Error("Resource provider was removed"); - } - ResourceProvider resourceProvider; resourceProvider.mutable_id()->CopyFrom(id); @@ -150,7 +141,6 @@ Try<bool> RemoveResourceProvider::perform(Registry* registry) return Error("Attempted to remove an unknown resource provider"); } - registry->add_removed_resource_providers()->CopyFrom(*pos); registry->mutable_resource_providers()->erase(pos); return true; // Mutation. http://git-wip-us.apache.org/repos/asf/mesos/blob/2855a6b6/src/resource_provider/registry.proto ---------------------------------------------------------------------- diff --git a/src/resource_provider/registry.proto b/src/resource_provider/registry.proto index 491263e..14bd433 100644 --- a/src/resource_provider/registry.proto +++ b/src/resource_provider/registry.proto @@ -35,5 +35,4 @@ message ResourceProvider { // A top level object that is managed by the Registrar and persisted. message Registry { repeated ResourceProvider resource_providers = 1; - repeated ResourceProvider removed_resource_providers = 2; } http://git-wip-us.apache.org/repos/asf/mesos/blob/2855a6b6/src/tests/resource_provider_manager_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/resource_provider_manager_tests.cpp b/src/tests/resource_provider_manager_tests.cpp index 8c364a1..eb8e4fc 100644 --- a/src/tests/resource_provider_manager_tests.cpp +++ b/src/tests/resource_provider_manager_tests.cpp @@ -843,24 +843,17 @@ TEST_F(ResourceProviderRegistrarTest, GenericRegistrar) AWAIT_READY(registrar.get()->recover()); - Future<bool> admitResourceProvider1 = + Future<bool> admitResourceProvider = registrar.get()->apply(Owned<Registrar::Operation>( new AdmitResourceProvider(resourceProviderId))); - AWAIT_READY(admitResourceProvider1); - EXPECT_TRUE(admitResourceProvider1.get()); + AWAIT_READY(admitResourceProvider); + EXPECT_TRUE(admitResourceProvider.get()); Future<bool> removeResourceProvider = registrar.get()->apply(Owned<Registrar::Operation>( new RemoveResourceProvider(resourceProviderId))); AWAIT_READY(removeResourceProvider); EXPECT_TRUE(removeResourceProvider.get()); - - // A removed resource provider cannot be admitted again. - Future<bool> admitResourceProvider2 = - registrar.get()->apply(Owned<Registrar::Operation>( - new AdmitResourceProvider(resourceProviderId))); - AWAIT_READY(admitResourceProvider2); - EXPECT_FALSE(admitResourceProvider2.get()); } @@ -886,24 +879,19 @@ TEST_F(ResourceProviderRegistrarTest, MasterRegistrar) ASSERT_SOME(registrar); ASSERT_NE(nullptr, registrar->get()); - Future<bool> admitResourceProvider1 = + AWAIT_READY(masterRegistrar.recover(masterInfo)); + + Future<bool> admitResourceProvider = registrar.get()->apply(Owned<Registrar::Operation>( new AdmitResourceProvider(resourceProviderId))); - AWAIT_READY(admitResourceProvider1); - EXPECT_TRUE(admitResourceProvider1.get()); + AWAIT_READY(admitResourceProvider); + EXPECT_TRUE(admitResourceProvider.get()); Future<bool> removeResourceProvider = registrar.get()->apply(Owned<Registrar::Operation>( new RemoveResourceProvider(resourceProviderId))); AWAIT_READY(removeResourceProvider); EXPECT_TRUE(removeResourceProvider.get()); - - // A removed resource provider cannot be admitted again. - Future<bool> admitResourceProvider2 = - registrar.get()->apply(Owned<Registrar::Operation>( - new AdmitResourceProvider(resourceProviderId))); - AWAIT_READY(admitResourceProvider2); - EXPECT_FALSE(admitResourceProvider2.get()); }
