This is an automated email from the ASF dual-hosted git repository. chhsiao pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push: new ff2f1d5 Return 409 if `UPDATE_RESOURCE_PROVIDER_CONFIG` names a missing config. ff2f1d5 is described below commit ff2f1d5bc9d068352e791245a3d867c2e6518f59 Author: Chun-Hung Hsiao <chhs...@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.