You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/09/28 17:04:19 UTC
[GitHub] [geode-native] mreddington opened a new pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.
mreddington opened a new pull request #658:
URL: https://github.com/apache/geode-native/pull/658
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #658:
URL: https://github.com/apache/geode-native/pull/658#discussion_r497602977
##########
File path: cppcache/acceptance-test/SNITest.cpp
##########
@@ -174,4 +175,53 @@ TEST_F(SNITest, connectWithoutProxyFails) {
cache.close();
}
+TEST_F(SNITest, dropSNIProxyTest) {
+ const auto clientTruststore =
+ (clientSslKeysDir / boost::filesystem::path("/truststore_sni.pem"));
+
+ auto cache = CacheFactory()
+ .set("log-level", "debug")
+ .set("log-file", "SNITest.log")
+ .set("ssl-enabled", "true")
+ .set("ssl-truststore", clientTruststore.string())
+ .create();
+
+ auto portString = runDockerCommand("docker port haproxy");
+ auto portNumber = parseProxyPort(portString);
+
+ cache.getPoolManager()
+ .createFactory()
+ .setSniProxy("localhost", portNumber)
+ .addLocator("locator-maeve", 10334)
+ .create("pool");
+
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("pool")
+ .create("jellyfish");
+
+ auto systemRVal = std::system("docker pause haproxy");
+ if (systemRVal == -1) {
+ BOOST_LOG_TRIVIAL(error) << "std::system returned: " << systemRVal;
Review comment:
Where does this log statement go? We're using gtest as our framework, does it have a logging facility that would be more appropriate? Just curious, before we drop in another pseudo-dependency like this. Is it an error here if the system call fails? If so, shouldn't we just `EXPECT_NE(systemRVal, -1)` rather than log and move on?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mreddington commented on a change in pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.
Posted by GitBox <gi...@apache.org>.
mreddington commented on a change in pull request #658:
URL: https://github.com/apache/geode-native/pull/658#discussion_r497671089
##########
File path: cppcache/acceptance-test/SNITest.cpp
##########
@@ -174,4 +175,53 @@ TEST_F(SNITest, connectWithoutProxyFails) {
cache.close();
}
+TEST_F(SNITest, dropSNIProxyTest) {
+ const auto clientTruststore =
+ (clientSslKeysDir / boost::filesystem::path("/truststore_sni.pem"));
+
+ auto cache = CacheFactory()
+ .set("log-level", "debug")
+ .set("log-file", "SNITest.log")
+ .set("ssl-enabled", "true")
+ .set("ssl-truststore", clientTruststore.string())
+ .create();
+
+ auto portString = runDockerCommand("docker port haproxy");
+ auto portNumber = parseProxyPort(portString);
+
+ cache.getPoolManager()
+ .createFactory()
+ .setSniProxy("localhost", portNumber)
+ .addLocator("locator-maeve", 10334)
+ .create("pool");
+
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("pool")
+ .create("jellyfish");
+
+ auto systemRVal = std::system("docker pause haproxy");
+ if (systemRVal == -1) {
+ BOOST_LOG_TRIVIAL(error) << "std::system returned: " << systemRVal;
Review comment:
Honestly I don't know. This test is cobbled out of the existing SNI tests, this is the same structure we use to initiate the docker images - invoke the command, check the result, log the error. I figured when in Rome...
The pseudo-dependency is already there because this is how we start the proxy in the test fixture.
I don't think an expectation is appropriate here. We're not testing whether or not a system command works... You know? I'd love if GTest had an "Incomplete" state for tests that incur an error beyond the scope of the test, in the environment. But then again, we shouldn't be writing big tests like this in the first place with a unit test framework.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] echobravopapa commented on a change in pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.
Posted by GitBox <gi...@apache.org>.
echobravopapa commented on a change in pull request #658:
URL: https://github.com/apache/geode-native/pull/658#discussion_r497706092
##########
File path: cppcache/acceptance-test/SNITest.cpp
##########
@@ -174,4 +174,51 @@ TEST_F(SNITest, connectWithoutProxyFails) {
cache.close();
}
+#if defined(_WIN32)
+TEST_F(SNITest, DISABLED_dropSNIProxyTest) {
+#else
+TEST_F(SNITest, dropSNIProxyTest) {
+#endif
+ const auto clientTruststore =
+ (clientSslKeysDir / boost::filesystem::path("/truststore_sni.pem"));
+
+ auto cache = CacheFactory()
+ .set("log-level", "debug")
+ .set("log-file", "SNITest.log")
+ .set("ssl-enabled", "true")
+ .set("ssl-truststore", clientTruststore.string())
+ .create();
+
+ auto portString = runDockerCommand("docker port haproxy");
+ auto portNumber = parseProxyPort(portString);
+
+ cache.getPoolManager()
+ .createFactory()
+ .setSniProxy("localhost", portNumber)
+ .addLocator("locator-maeve", 10334)
+ .create("pool");
+
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("pool")
+ .create("jellyfish");
+
+ auto systemRVal = std::system("docker-compose pause");
+ if (systemRVal == -1) {
+ BOOST_LOG_TRIVIAL(error) << "std::system returned: " << systemRVal;
+ }
+
+ region->put("1", "one");
+
+ std::this_thread::sleep_for(std::chrono::seconds(17));
Review comment:
the magic of prime numbers?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell merged pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.
Posted by GitBox <gi...@apache.org>.
mmartell merged pull request #658:
URL: https://github.com/apache/geode-native/pull/658
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #658:
URL: https://github.com/apache/geode-native/pull/658#discussion_r498431745
##########
File path: cppcache/acceptance-test/SNITest.cpp
##########
@@ -174,4 +175,53 @@ TEST_F(SNITest, connectWithoutProxyFails) {
cache.close();
}
+TEST_F(SNITest, dropSNIProxyTest) {
+ const auto clientTruststore =
+ (clientSslKeysDir / boost::filesystem::path("/truststore_sni.pem"));
+
+ auto cache = CacheFactory()
+ .set("log-level", "debug")
+ .set("log-file", "SNITest.log")
+ .set("ssl-enabled", "true")
+ .set("ssl-truststore", clientTruststore.string())
+ .create();
+
+ auto portString = runDockerCommand("docker port haproxy");
+ auto portNumber = parseProxyPort(portString);
+
+ cache.getPoolManager()
+ .createFactory()
+ .setSniProxy("localhost", portNumber)
+ .addLocator("locator-maeve", 10334)
+ .create("pool");
+
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("pool")
+ .create("jellyfish");
+
+ auto systemRVal = std::system("docker pause haproxy");
+ if (systemRVal == -1) {
+ BOOST_LOG_TRIVIAL(error) << "std::system returned: " << systemRVal;
Review comment:
Meh - fine, it's just test code. If we ever need to debug it, we can figure out where BOOST_LOG_TRIVIAL output goes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org