You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2019/05/21 21:23:51 UTC
[mesos] 02/02: Return 409 if `UPDATE_RESOURCE_PROVIDER_CONFIG`
names a missing config.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 484782c324ebead46f5f7977ee4f667cdc45f25f
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Fri May 10 15:57:28 2019 -0700
Return 409 if `UPDATE_RESOURCE_PROVIDER_CONFIG` names a missing config.
Since 404 is returned if the API endpoint route is not set yet, this
error code becomes ambiguous and makes clients hard to programmatically
handle it. Therefore, the error code for specifying a missing config
in this API call is changed to 409 Conflict.
Review: https://reviews.apache.org/r/70628
---
include/mesos/agent/agent.proto | 10 +++++-----
include/mesos/v1/agent/agent.proto | 10 +++++-----
src/slave/http.cpp | 12 ++++++++----
src/tests/agent_resource_provider_config_api_tests.cpp | 4 ++--
4 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/include/mesos/agent/agent.proto b/include/mesos/agent/agent.proto
index 316a384..83eb7bb 100644
--- a/include/mesos/agent/agent.proto
+++ b/include/mesos/agent/agent.proto
@@ -315,9 +315,9 @@ message Call {
// exists.
// Returns 400 Bad Request if `info` is not well-formed.
// Returns 403 Forbidden if the call is not authorized.
- // Returns 409 Conflict if another config file that describes a
- // resource provider of the same type and name exists, but the content is
- // not identical.
+ // Returns 409 Conflict if another config file that describes a resource
+ // provider of the same type and name exists, but the content is not
+ // identical.
// Returns 500 Internal Server Error if anything goes wrong.
//
// NOTE: For the time being, this API is subject to change and the related
@@ -342,8 +342,8 @@ message Call {
// in the config file.
// Returns 400 Bad Request if `info` is not well-formed.
// Returns 403 Forbidden if the call is not authorized.
- // Returns 404 Not Found if no config file describes a resource
- // provider of the same type and name exists.
+ // Returns 409 Conflict if no config file describes a resource provider of the
+ // same type and name exists.
// Returns 500 Internal Server Error if anything goes wrong.
//
// NOTE: For the time being, this API is subject to change and the related
diff --git a/include/mesos/v1/agent/agent.proto b/include/mesos/v1/agent/agent.proto
index 2797d20..f6574cb 100644
--- a/include/mesos/v1/agent/agent.proto
+++ b/include/mesos/v1/agent/agent.proto
@@ -315,9 +315,9 @@ message Call {
// exists.
// Returns 400 Bad Request if `info` is not well-formed.
// Returns 403 Forbidden if the call is not authorized.
- // Returns 409 Conflict if another config file that describes a
- // resource provider of the same type and name exists, but the content is
- // not identical.
+ // Returns 409 Conflict if another config file that describes a resource
+ // provider of the same type and name exists, but the content is not
+ // identical.
// Returns 500 Internal Server Error if anything goes wrong.
//
// NOTE: For the time being, this API is subject to change and the related
@@ -342,8 +342,8 @@ message Call {
// in the config file.
// Returns 400 Bad Request if `info` is not well-formed.
// Returns 403 Forbidden if the call is not authorized.
- // Returns 404 Not Found if no config file describes a resource
- // provider of the same type and name exists.
+ // Returns 409 Conflict if no config file describes a resource provider of the
+ // same type and name exists.
// Returns 500 Internal Server Error if anything goes wrong.
//
// NOTE: For the time being, this API is subject to change and the related
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 2c4e792..69e6d74 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -3249,9 +3249,11 @@ Future<Response> Http::addResourceProviderConfig(
}
return slave->localResourceProviderDaemon->add(info)
- .then([](bool added) -> Response {
+ .then([info](bool added) -> Response {
if (!added) {
- return Conflict();
+ return Conflict(
+ "Resource provider with type '" + info.type() +
+ "' and name '" + info.name() + "' already exists");
}
return OK();
@@ -3294,9 +3296,11 @@ Future<Response> Http::updateResourceProviderConfig(
}
return slave->localResourceProviderDaemon->update(info)
- .then([](bool updated) -> Response {
+ .then([info](bool updated) -> Response {
if (!updated) {
- return NotFound();
+ return Conflict(
+ "Resource provider with type '" + info.type() +
+ "' and name '" + info.name() + "' does not exist");
}
return OK();
diff --git a/src/tests/agent_resource_provider_config_api_tests.cpp b/src/tests/agent_resource_provider_config_api_tests.cpp
index 7185ac0..aadebd3 100644
--- a/src/tests/agent_resource_provider_config_api_tests.cpp
+++ b/src/tests/agent_resource_provider_config_api_tests.cpp
@@ -760,7 +760,7 @@ TEST_P(AgentResourceProviderConfigApiTest, IdempotentUpdate)
// This test checks that updating a nonexistent resource provider config is
// rejected.
-TEST_P(AgentResourceProviderConfigApiTest, UpdateNotFound)
+TEST_P(AgentResourceProviderConfigApiTest, UpdateConflict)
{
const ContentType contentType = GetParam();
@@ -780,7 +780,7 @@ TEST_P(AgentResourceProviderConfigApiTest, UpdateNotFound)
ResourceProviderInfo info = createResourceProviderInfo("volume1:4GB");
AWAIT_EXPECT_RESPONSE_STATUS_EQ(
- http::NotFound().status,
+ http::Conflict().status,
updateResourceProviderConfig(slave.get()->pid, contentType, info));
// Check that no new config is created.