From 080d0eb6c05d558ed4be53fb0ce54a7736c31c6c Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 30 Mar 2018 14:04:47 -0400 Subject: [PATCH 1/7] Added test for ComputePodAssociations --- test/kubernetes_unittest.cc | 75 +++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 79e4ac94..64283333 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -1,3 +1,5 @@ +#include + #include "../src/configuration.h" #include "../src/kubernetes.h" #include "../src/updater.h" @@ -13,6 +15,16 @@ class KubernetesTest : public ::testing::Test { throw(json::Exception) { return reader.GetNodeMetadata(node, collected_at, is_deleted); } + + json::value ComputePodAssociations(const json::Object* pod, + const KubernetesReader& reader) { + return reader.ComputePodAssociations(pod); + } + + void InsertIntoOwners(const std::string& key, json::value& value, + KubernetesReader* reader) { + reader->owners_[key] = value->Clone(); + } }; TEST_F(KubernetesTest, GetNodeMetadata) { @@ -68,4 +80,67 @@ TEST_F(KubernetesTest, GetNodeMetadata) { }); EXPECT_EQ(big->ToString(), m.metadata.metadata->ToString()); } + +TEST_F(KubernetesTest, ComputePodAssociations) { + const std::string api_version = "1.2.3"; + const std::string kind = "TestKind"; + const std::string uid1 = "TestUID1"; + Configuration config(std::stringstream( + "KubernetesClusterName: TestClusterName\n" + "KubernetesClusterLocation: TestClusterLocation\n" + "MetadataIngestionRawContentVersion: TestVersion\n" + "InstanceZone: TestZone\n" + "InstanceId: TestID\n" + )); + Environment environment(config); + KubernetesReader reader(config, nullptr); // Don't need HealthChecker. + const std::string encoded_ref = boost::algorithm::join( + std::vector{api_version, kind, uid1}, "/"); + json::value controller = json::object({ + {"controller", json::boolean(true)}, + {"apiVersion", json::string(api_version)}, + {"kind", json::string(kind)}, + {"name", json::string("TestName")}, + {"uid", json::string(uid1)}, + {"metadata", json::object({ + {"name", json::string("InnerTestName")}, + {"kind", json::string("InnerTestKind")}, + {"uid", json::string("InnerTestUID1")}, + })}, + }); + InsertIntoOwners(encoded_ref, controller, &reader); + json::value pod = json::object({ + {"metadata", json::object({ + {"namespace", json::string("TestNamespace")}, + {"uid", json::string("TestUID0")}, + {"ownerReferences", json::array({ + json::object({{"no_controller", json::boolean(true)}}), + controller->Clone(), + })}, + })}, + {"spec", json::object({ + {"nodeName", json::string("TestSpecNodeName")}, + })}, + }); + + auto associations = ComputePodAssociations(pod->As(), reader); + + EXPECT_EQ(associations->ToString(), json::object({ + {"raw", json::object({ + {"controllers", json::object({ + {"topLevelControllerName", json::string("InnerTestName")}, + {"topLevelControllerType", json::string(kind)}, + })}, + {"infrastructureResource", json::object({ + {"labels", json::object({ + {"instance_id", json::string("TestID")}, + {"zone", json::string("TestZone")} + })}, + {"type", json::string("gce_instance")}, + })}, + {"nodeName", json::string("TestSpecNodeName")}, + })}, + {"version", json::string("TestVersion")}, + })->ToString()); +} } // namespace google From 201c3bf7a66cbe621fc4b2c5784f2edbb80de49d Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 30 Mar 2018 15:23:19 -0400 Subject: [PATCH 2/7] Addressed code reivew comments --- test/kubernetes_unittest.cc | 55 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 64283333..15a6ddd3 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -17,12 +17,12 @@ class KubernetesTest : public ::testing::Test { } json::value ComputePodAssociations(const json::Object* pod, - const KubernetesReader& reader) { + const KubernetesReader& reader) { return reader.ComputePodAssociations(pod); } - void InsertIntoOwners(const std::string& key, json::value& value, - KubernetesReader* reader) { + void UpdateOwnersCache(const std::string& key, json::value& value, + KubernetesReader* reader) { reader->owners_[key] = value->Clone(); } }; @@ -94,28 +94,27 @@ TEST_F(KubernetesTest, ComputePodAssociations) { )); Environment environment(config); KubernetesReader reader(config, nullptr); // Don't need HealthChecker. - const std::string encoded_ref = boost::algorithm::join( - std::vector{api_version, kind, uid1}, "/"); + const std::string encoded_ref = "1.2.3/TestKind/TestUID1"; json::value controller = json::object({ - {"controller", json::boolean(true)}, - {"apiVersion", json::string(api_version)}, - {"kind", json::string(kind)}, - {"name", json::string("TestName")}, - {"uid", json::string(uid1)}, - {"metadata", json::object({ - {"name", json::string("InnerTestName")}, - {"kind", json::string("InnerTestKind")}, - {"uid", json::string("InnerTestUID1")}, - })}, + {"controller", json::boolean(true)}, + {"apiVersion", json::string(api_version)}, + {"kind", json::string(kind)}, + {"name", json::string("TestName")}, + {"uid", json::string(uid1)}, + {"metadata", json::object({ + {"name", json::string("InnerTestName")}, + {"kind", json::string("InnerTestKind")}, + {"uid", json::string("InnerTestUID1")}, + })}, }); - InsertIntoOwners(encoded_ref, controller, &reader); + UpdateOwnersCache(encoded_ref, controller, &reader); json::value pod = json::object({ {"metadata", json::object({ {"namespace", json::string("TestNamespace")}, {"uid", json::string("TestUID0")}, {"ownerReferences", json::array({ json::object({{"no_controller", json::boolean(true)}}), - controller->Clone(), + controller->Clone(), })}, })}, {"spec", json::object({ @@ -123,24 +122,24 @@ TEST_F(KubernetesTest, ComputePodAssociations) { })}, }); - auto associations = ComputePodAssociations(pod->As(), reader); - - EXPECT_EQ(associations->ToString(), json::object({ + json::value expected_associations = json::object({ {"raw", json::object({ {"controllers", json::object({ - {"topLevelControllerName", json::string("InnerTestName")}, - {"topLevelControllerType", json::string(kind)}, + {"topLevelControllerName", json::string("InnerTestName")}, + {"topLevelControllerType", json::string(kind)}, })}, {"infrastructureResource", json::object({ - {"labels", json::object({ - {"instance_id", json::string("TestID")}, - {"zone", json::string("TestZone")} - })}, - {"type", json::string("gce_instance")}, + {"labels", json::object({ + {"instance_id", json::string("TestID")}, + {"zone", json::string("TestZone")} + })}, + {"type", json::string("gce_instance")}, })}, {"nodeName", json::string("TestSpecNodeName")}, })}, {"version", json::string("TestVersion")}, - })->ToString()); + }); + const auto associations = ComputePodAssociations(pod->As(), reader); + EXPECT_EQ(expected_associations->ToString(), associations->ToString()); } } // namespace google From c65cfaf6fbd08c542d7ffa2c2fdb8d90b380a29f Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 30 Mar 2018 15:37:01 -0400 Subject: [PATCH 3/7] Line too long fix --- test/kubernetes_unittest.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 15a6ddd3..2879933e 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -139,7 +139,8 @@ TEST_F(KubernetesTest, ComputePodAssociations) { })}, {"version", json::string("TestVersion")}, }); - const auto associations = ComputePodAssociations(pod->As(), reader); + const auto associations = + ComputePodAssociations(pod->As(), reader); EXPECT_EQ(expected_associations->ToString(), associations->ToString()); } } // namespace google From 8b9eb102cb1a9fcab39e883a10231240f0e8ce0e Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 30 Mar 2018 16:03:19 -0400 Subject: [PATCH 4/7] Addressed further code review comments --- test/kubernetes_unittest.cc | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 2879933e..655ff81a 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -16,13 +16,13 @@ class KubernetesTest : public ::testing::Test { return reader.GetNodeMetadata(node, collected_at, is_deleted); } - json::value ComputePodAssociations(const json::Object* pod, - const KubernetesReader& reader) { + json::value ComputePodAssociations(const KubernetesReader& reader, + const json::Object* pod) { return reader.ComputePodAssociations(pod); } - void UpdateOwnersCache(const std::string& key, json::value& value, - KubernetesReader* reader) { + void UpdateOwnersCache(KubernetesReader* reader, const std::string& key, + const json::value& value) { reader->owners_[key] = value->Clone(); } }; @@ -82,9 +82,6 @@ TEST_F(KubernetesTest, GetNodeMetadata) { } TEST_F(KubernetesTest, ComputePodAssociations) { - const std::string api_version = "1.2.3"; - const std::string kind = "TestKind"; - const std::string uid1 = "TestUID1"; Configuration config(std::stringstream( "KubernetesClusterName: TestClusterName\n" "KubernetesClusterLocation: TestClusterLocation\n" @@ -97,17 +94,17 @@ TEST_F(KubernetesTest, ComputePodAssociations) { const std::string encoded_ref = "1.2.3/TestKind/TestUID1"; json::value controller = json::object({ {"controller", json::boolean(true)}, - {"apiVersion", json::string(api_version)}, - {"kind", json::string(kind)}, + {"apiVersion", json::string("1.2.3")}, + {"kind", json::string("TestKind")}, {"name", json::string("TestName")}, - {"uid", json::string(uid1)}, + {"uid", json::string("TestUID1")}, {"metadata", json::object({ {"name", json::string("InnerTestName")}, {"kind", json::string("InnerTestKind")}, {"uid", json::string("InnerTestUID1")}, })}, }); - UpdateOwnersCache(encoded_ref, controller, &reader); + UpdateOwnersCache(&reader, encoded_ref, controller); json::value pod = json::object({ {"metadata", json::object({ {"namespace", json::string("TestNamespace")}, @@ -126,12 +123,12 @@ TEST_F(KubernetesTest, ComputePodAssociations) { {"raw", json::object({ {"controllers", json::object({ {"topLevelControllerName", json::string("InnerTestName")}, - {"topLevelControllerType", json::string(kind)}, + {"topLevelControllerType", json::string("TestKind")}, })}, {"infrastructureResource", json::object({ {"labels", json::object({ {"instance_id", json::string("TestID")}, - {"zone", json::string("TestZone")} + {"zone", json::string("TestZone")}, })}, {"type", json::string("gce_instance")}, })}, @@ -140,7 +137,7 @@ TEST_F(KubernetesTest, ComputePodAssociations) { {"version", json::string("TestVersion")}, }); const auto associations = - ComputePodAssociations(pod->As(), reader); + ComputePodAssociations(reader, pod->As()); EXPECT_EQ(expected_associations->ToString(), associations->ToString()); } } // namespace google From 85dd98848d46c557e786c395f22a417172c27524 Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 30 Mar 2018 16:04:14 -0400 Subject: [PATCH 5/7] Remove unused import --- test/kubernetes_unittest.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 655ff81a..207ca887 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -1,5 +1,3 @@ -#include - #include "../src/configuration.h" #include "../src/kubernetes.h" #include "../src/updater.h" From a8b50e2307ad073347747e440138f89d71f25c27 Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 30 Mar 2018 16:34:40 -0400 Subject: [PATCH 6/7] Inlined unneeded variable --- test/kubernetes_unittest.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 207ca887..1a955bad 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -89,7 +89,6 @@ TEST_F(KubernetesTest, ComputePodAssociations) { )); Environment environment(config); KubernetesReader reader(config, nullptr); // Don't need HealthChecker. - const std::string encoded_ref = "1.2.3/TestKind/TestUID1"; json::value controller = json::object({ {"controller", json::boolean(true)}, {"apiVersion", json::string("1.2.3")}, @@ -102,7 +101,7 @@ TEST_F(KubernetesTest, ComputePodAssociations) { {"uid", json::string("InnerTestUID1")}, })}, }); - UpdateOwnersCache(&reader, encoded_ref, controller); + UpdateOwnersCache(&reader, "1.2.3/TestKind/TestUID1", controller); json::value pod = json::object({ {"metadata", json::object({ {"namespace", json::string("TestNamespace")}, From fb12d03187f4680b5ba11e430f24a3995152c8c1 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Sat, 31 Mar 2018 17:03:42 -0400 Subject: [PATCH 7/7] Avoid an unnecessary Clone(). --- test/kubernetes_unittest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/kubernetes_unittest.cc b/test/kubernetes_unittest.cc index 1a955bad..23777536 100644 --- a/test/kubernetes_unittest.cc +++ b/test/kubernetes_unittest.cc @@ -108,7 +108,7 @@ TEST_F(KubernetesTest, ComputePodAssociations) { {"uid", json::string("TestUID0")}, {"ownerReferences", json::array({ json::object({{"no_controller", json::boolean(true)}}), - controller->Clone(), + std::move(controller), })}, })}, {"spec", json::object({