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/04/03 23:14:11 UTC

[mesos] 14/15: Moved CSI v0 type helpers to the `mesos/csi/v0.hpp` header.

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 <ch...@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;