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
commit 35e57187b885afa87210c7d66e1dd43c08eded99 Author: Chun-Hung Hsiao <chhs...@mesosphere.io> AuthorDate: Mon Mar 25 18:16:21 2019 -0700 Moved CSI v0 type helpers to the `mesos/csi/v0.hpp` header. The equality check and output helpers for CSI v0 protobufs are now declared in the `v0.hpp` header to ensure ADL works properly. The implementation is also moved to a new `v0.cpp` file. The header and implementation files for CSI v0 utility helpers are also renamed for future CSI v1 support. Review: https://reviews.apache.org/r/70303 --- include/mesos/csi/v0.hpp | 67 ++++++++++++ src/CMakeLists.txt | 3 +- src/Makefile.am | 5 +- src/csi/service_manager.cpp | 2 +- include/mesos/csi/v0.hpp => src/csi/v0.cpp | 20 ++-- src/csi/{utils.cpp => v0_utils.cpp} | 117 ++++----------------- src/csi/{utils.hpp => v0_utils.hpp} | 61 +---------- src/csi/v0_volume_manager.cpp | 2 +- src/csi/v0_volume_manager_process.hpp | 2 +- src/examples/test_csi_plugin.cpp | 2 +- .../storage/uri_disk_profile_adaptor.cpp | 2 +- src/tests/csi_utils_tests.cpp | 2 +- 12 files changed, 110 insertions(+), 175 deletions(-) diff --git a/include/mesos/csi/v0.hpp b/include/mesos/csi/v0.hpp index 58b2c61..8be070b 100644 --- a/include/mesos/csi/v0.hpp +++ b/include/mesos/csi/v0.hpp @@ -17,12 +17,22 @@ #ifndef __MESOS_CSI_V0_HPP__ #define __MESOS_CSI_V0_HPP__ +#include <ostream> +#include <type_traits> + // ONLY USEFUL AFTER RUNNING PROTOC. #include <csi/v0/csi.pb.h> // ONLY USEFUL AFTER RUNNING PROTOC WITH GRPC CPP PLUGIN. #include <csi/v0/csi.grpc.pb.h> +#include <google/protobuf/message.h> + +#include <google/protobuf/util/json_util.h> +#include <google/protobuf/util/message_differencer.h> + +#include <stout/check.hpp> + namespace mesos { namespace csi { namespace v0 { @@ -33,4 +43,61 @@ using namespace ::csi::v0; } // namespace csi { } // namespace mesos { + +namespace csi { +namespace v0 { + +// Default implementation for comparing protobuf messages in namespace +// `csi::v0`. Note that any non-template overloading of the equality operator +// would take precedence over this function template. +template < + typename Message, + typename std::enable_if<std::is_convertible< + Message*, google::protobuf::Message*>::value, int>::type = 0> +bool operator==(const Message& left, const Message& right) +{ + // NOTE: `MessageDifferencer::Equivalent` would ignore unknown fields and load + // default values for unset fields (which are indistinguishable in proto3). + return google::protobuf::util::MessageDifferencer::Equivalent(left, right); +} + + +template < + typename Message, + typename std::enable_if<std::is_convertible< + Message*, google::protobuf::Message*>::value, int>::type = 0> +bool operator!=(const Message& left, const Message& right) +{ + return !(left == right); +} + + +std::ostream& operator<<( + std::ostream& stream, + const ControllerServiceCapability::RPC::Type& type); + + +// Default implementation for output protobuf messages in namespace `csi::v0`. +// Note that any non-template overloading of the output operator would take +// precedence over this function template. +template < + typename Message, + typename std::enable_if<std::is_convertible< + Message*, google::protobuf::Message*>::value, int>::type = 0> +std::ostream& operator<<(std::ostream& stream, const Message& message) +{ + // NOTE: We use Google's JSON utility functions for proto3. + std::string output; + google::protobuf::util::Status status = + google::protobuf::util::MessageToJsonString(message, &output); + + CHECK(status.ok()) + << "Could not convert messages to string: " << status.error_message(); + + return stream << output; +} + +} // namespace v0 { +} // namespace csi { + #endif // __MESOS_CSI_V0_HPP__ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2071576..6ef0c8c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -238,7 +238,8 @@ set(CSI_SRC csi/paths.cpp csi/rpc.cpp csi/service_manager.cpp - csi/utils.cpp + csi/v0.cpp + csi/v0_utils.cpp csi/v0_volume_manager.cpp csi/volume_manager.cpp) diff --git a/src/Makefile.am b/src/Makefile.am index d132d80..61ded56 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1573,8 +1573,9 @@ libcsi_la_SOURCES = \ csi/service_manager.hpp \ csi/state.hpp \ csi/state.proto \ - csi/utils.cpp \ - csi/utils.hpp \ + csi/v0.cpp \ + csi/v0_utils.cpp \ + csi/v0_utils.hpp \ csi/v0_volume_manager.cpp \ csi/v0_volume_manager.hpp \ csi/v0_volume_manager_process.hpp \ diff --git a/src/csi/service_manager.cpp b/src/csi/service_manager.cpp index 9f715d6..f8a42f6 100644 --- a/src/csi/service_manager.cpp +++ b/src/csi/service_manager.cpp @@ -51,7 +51,7 @@ #include "csi/client.hpp" #include "csi/paths.hpp" -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" #include "internal/devolve.hpp" #include "internal/evolve.hpp" diff --git a/include/mesos/csi/v0.hpp b/src/csi/v0.cpp similarity index 73% copy from include/mesos/csi/v0.hpp copy to src/csi/v0.cpp index 58b2c61..09c3fe6 100644 --- a/include/mesos/csi/v0.hpp +++ b/src/csi/v0.cpp @@ -14,23 +14,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef __MESOS_CSI_V0_HPP__ -#define __MESOS_CSI_V0_HPP__ +#include <mesos/csi/v0.hpp> -// ONLY USEFUL AFTER RUNNING PROTOC. -#include <csi/v0/csi.pb.h> +using std::ostream; -// ONLY USEFUL AFTER RUNNING PROTOC WITH GRPC CPP PLUGIN. -#include <csi/v0/csi.grpc.pb.h> - -namespace mesos { namespace csi { namespace v0 { -using namespace ::csi::v0; +ostream& operator<<( + ostream& stream, + const ControllerServiceCapability::RPC::Type& type) +{ + return stream << ControllerServiceCapability::RPC::Type_Name(type); +} } // namespace v0 { } // namespace csi { -} // namespace mesos { - -#endif // __MESOS_CSI_V0_HPP__ diff --git a/src/csi/utils.cpp b/src/csi/v0_utils.cpp similarity index 62% rename from src/csi/utils.cpp rename to src/csi/v0_utils.cpp index 872527c..a95d240 100644 --- a/src/csi/utils.cpp +++ b/src/csi/v0_utils.cpp @@ -14,102 +14,27 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" #include <stout/unreachable.hpp> -using std::ostream; - -namespace csi { -namespace v0 { - -bool operator==( - const ControllerServiceCapability::RPC& left, - const ControllerServiceCapability::RPC& right) -{ - return left.type() == right.type(); -} - - -bool operator==( - const ControllerServiceCapability& left, - const ControllerServiceCapability& right) -{ - return left.has_rpc() == right.has_rpc() && - (!left.has_rpc() || left.rpc() == right.rpc()); -} - - -bool operator==(const VolumeCapability& left, const VolumeCapability& right) -{ - // NOTE: This enumeration is set when `block` or `mount` are set and - // covers the case where neither are set. - if (left.access_type_case() != right.access_type_case()) { - return false; - } - - // NOTE: No need to check `block` for equality as that object is empty. - - if (left.has_mount()) { - if (left.mount().fs_type() != right.mount().fs_type()) { - return false; - } - - if (left.mount().mount_flags_size() != right.mount().mount_flags_size()) { - return false; - } - - // NOTE: Ordering may or may not matter for these flags, but this helper - // only checks for complete equality. - for (int i = 0; i < left.mount().mount_flags_size(); i++) { - if (left.mount().mount_flags(i) != right.mount().mount_flags(i)) { - return false; - } - } - } - - if (left.has_access_mode() != right.has_access_mode()) { - return false; - } - - if (left.has_access_mode()) { - if (left.access_mode().mode() != right.access_mode().mode()) { - return false; - } - } - - return true; -} - - -ostream& operator<<( - ostream& stream, - const ControllerServiceCapability::RPC::Type& type) -{ - return stream << ControllerServiceCapability::RPC::Type_Name(type); -} - -} // namespace v0 { -} // namespace csi { - - namespace mesos { namespace csi { namespace v0 { types::VolumeCapability::BlockVolume devolve( - const VolumeCapability::BlockVolume& blockVolume) + const VolumeCapability::BlockVolume& block) { return types::VolumeCapability::BlockVolume(); } types::VolumeCapability::MountVolume devolve( - const VolumeCapability::MountVolume& mountVolume) + const VolumeCapability::MountVolume& mount) { types::VolumeCapability::MountVolume result; - result.set_fs_type(mountVolume.fs_type()); - *result.mutable_mount_flags() = mountVolume.mount_flags(); + result.set_fs_type(mount.fs_type()); + *result.mutable_mount_flags() = mount.mount_flags(); return result; } @@ -161,17 +86,17 @@ types::VolumeCapability::AccessMode devolve( } -types::VolumeCapability devolve(const VolumeCapability& volumeCapability) +types::VolumeCapability devolve(const VolumeCapability& capability) { types::VolumeCapability result; - switch (volumeCapability.access_type_case()) { + switch (capability.access_type_case()) { case VolumeCapability::kBlock: { - *result.mutable_block() = devolve(volumeCapability.block()); + *result.mutable_block() = devolve(capability.block()); break; } case VolumeCapability::kMount: { - *result.mutable_mount() = devolve(volumeCapability.mount()); + *result.mutable_mount() = devolve(capability.mount()); break; } case VolumeCapability::ACCESS_TYPE_NOT_SET: { @@ -179,8 +104,8 @@ types::VolumeCapability devolve(const VolumeCapability& volumeCapability) } } - if (volumeCapability.has_access_mode()) { - *result.mutable_access_mode() = devolve(volumeCapability.access_mode()); + if (capability.has_access_mode()) { + *result.mutable_access_mode() = devolve(capability.access_mode()); } return result; @@ -188,18 +113,18 @@ types::VolumeCapability devolve(const VolumeCapability& volumeCapability) VolumeCapability::BlockVolume evolve( - const types::VolumeCapability::BlockVolume& blockVolume) + const types::VolumeCapability::BlockVolume& block) { return VolumeCapability::BlockVolume(); } VolumeCapability::MountVolume evolve( - const types::VolumeCapability::MountVolume& mountVolume) + const types::VolumeCapability::MountVolume& mount) { VolumeCapability::MountVolume result; - result.set_fs_type(mountVolume.fs_type()); - *result.mutable_mount_flags() = mountVolume.mount_flags(); + result.set_fs_type(mount.fs_type()); + *result.mutable_mount_flags() = mount.mount_flags(); return result; } @@ -247,17 +172,17 @@ VolumeCapability::AccessMode evolve( } -VolumeCapability evolve(const types::VolumeCapability& volumeCapability) +VolumeCapability evolve(const types::VolumeCapability& capability) { VolumeCapability result; - switch (volumeCapability.access_type_case()) { + switch (capability.access_type_case()) { case types::VolumeCapability::kBlock: { - *result.mutable_block() = evolve(volumeCapability.block()); + *result.mutable_block() = evolve(capability.block()); break; } case types::VolumeCapability::kMount: { - *result.mutable_mount() = evolve(volumeCapability.mount()); + *result.mutable_mount() = evolve(capability.mount()); break; } case types::VolumeCapability::ACCESS_TYPE_NOT_SET: { @@ -265,8 +190,8 @@ VolumeCapability evolve(const types::VolumeCapability& volumeCapability) } } - if (volumeCapability.has_access_mode()) { - *result.mutable_access_mode() = evolve(volumeCapability.access_mode()); + if (capability.has_access_mode()) { + *result.mutable_access_mode() = evolve(capability.access_mode()); } return result; diff --git a/src/csi/utils.hpp b/src/csi/v0_utils.hpp similarity index 74% rename from src/csi/utils.hpp rename to src/csi/v0_utils.hpp index 30b579e..46a5f13 100644 --- a/src/csi/utils.hpp +++ b/src/csi/v0_utils.hpp @@ -14,70 +14,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef __CSI_UTILS_HPP__ -#define __CSI_UTILS_HPP__ - -#include <ostream> -#include <type_traits> - -#include <google/protobuf/map.h> - -#include <google/protobuf/util/json_util.h> - -#include <mesos/mesos.hpp> +#ifndef __CSI_V0_UTILS_HPP__ +#define __CSI_V0_UTILS_HPP__ #include <mesos/csi/types.hpp> #include <mesos/csi/v0.hpp> #include <stout/foreach.hpp> -#include <stout/try.hpp> #include <stout/unreachable.hpp> -#include "csi/state.hpp" - -namespace csi { -namespace v0 { - -bool operator==( - const ControllerServiceCapability& left, - const ControllerServiceCapability& right); - - -bool operator==(const VolumeCapability& left, const VolumeCapability& right); - - -inline bool operator!=( - const VolumeCapability& left, - const VolumeCapability& right) -{ - return !(left == right); -} - - -std::ostream& operator<<( - std::ostream& stream, - const ControllerServiceCapability::RPC::Type& type); - - -// Default imprementation for output protobuf messages in namespace -// `csi`. Note that any non-template overloading of the output operator -// would take precedence over this function template. -template < - typename Message, - typename std::enable_if<std::is_convertible< - Message*, google::protobuf::Message*>::value, int>::type = 0> -std::ostream& operator<<(std::ostream& stream, const Message& message) -{ - // NOTE: We use Google's JSON utility functions for proto3. - std::string output; - google::protobuf::util::MessageToJsonString(message, &output); - return stream << output; -} - -} // namespace v0 { -} // namespace csi { - - namespace mesos { namespace csi { namespace v0 { @@ -189,4 +134,4 @@ VolumeCapability evolve(const types::VolumeCapability& capability); } // namespace csi { } // namespace mesos { -#endif // __CSI_UTILS_HPP__ +#endif // __CSI_V0_UTILS_HPP__ diff --git a/src/csi/v0_volume_manager.cpp b/src/csi/v0_volume_manager.cpp index aeddb5d..bf9f00e 100644 --- a/src/csi/v0_volume_manager.cpp +++ b/src/csi/v0_volume_manager.cpp @@ -42,7 +42,7 @@ #include "csi/client.hpp" #include "csi/paths.hpp" -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" #include "csi/v0_volume_manager_process.hpp" #include "slave/state.hpp" diff --git a/src/csi/v0_volume_manager_process.hpp b/src/csi/v0_volume_manager_process.hpp index 214fc1f..170c3a6 100644 --- a/src/csi/v0_volume_manager_process.hpp +++ b/src/csi/v0_volume_manager_process.hpp @@ -47,7 +47,7 @@ #include "csi/rpc.hpp" #include "csi/service_manager.hpp" #include "csi/state.hpp" -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" #include "csi/v0_volume_manager.hpp" #include "csi/volume_manager.hpp" diff --git a/src/examples/test_csi_plugin.cpp b/src/examples/test_csi_plugin.cpp index 5b3ba4b..4321f8f 100644 --- a/src/examples/test_csi_plugin.cpp +++ b/src/examples/test_csi_plugin.cpp @@ -41,7 +41,7 @@ #include <stout/os/mkdir.hpp> #include <stout/os/rmdir.hpp> -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" #include "linux/fs.hpp" diff --git a/src/resource_provider/storage/uri_disk_profile_adaptor.cpp b/src/resource_provider/storage/uri_disk_profile_adaptor.cpp index 25b789d..215f7f9 100644 --- a/src/resource_provider/storage/uri_disk_profile_adaptor.cpp +++ b/src/resource_provider/storage/uri_disk_profile_adaptor.cpp @@ -39,7 +39,7 @@ #include <stout/result.hpp> #include <stout/strings.hpp> -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" #include "resource_provider/storage/disk_profile_utils.hpp" diff --git a/src/tests/csi_utils_tests.cpp b/src/tests/csi_utils_tests.cpp index db58b49..b9d0ca8 100644 --- a/src/tests/csi_utils_tests.cpp +++ b/src/tests/csi_utils_tests.cpp @@ -24,7 +24,7 @@ #include <mesos/csi/types.hpp> #include <mesos/csi/v0.hpp> -#include "csi/utils.hpp" +#include "csi/v0_utils.hpp" namespace util = google::protobuf::util;