From 94f101920790f74d7c6a709d4011f77cb1231a33 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 17 Nov 2017 11:36:31 -0500 Subject: [PATCH 1/2] Memoize owner objects instead of querying the master all the time. --- src/kubernetes.cc | 23 ++++++++++++++++++----- src/kubernetes.h | 2 ++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index 6d902bd8..f68b9e19 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -527,13 +527,26 @@ json::value KubernetesReader::GetOwner( const std::string api_version = owner_ref->Get("apiVersion"); const std::string kind = owner_ref->Get("kind"); const std::string name = owner_ref->Get("name"); - const auto path_component = KindPath(api_version, kind); + + const std::string encoded_ref = boost::algorithm::join( + std::vector{api_version, kind, ns, name}, "/"); + + std::lock_guard lock(mutex_); + auto found = owners_.emplace(std::piecewise_construct, + std::forward_as_tuple(encoded_ref), + std::forward_as_tuple()); + json::value& owner = found.first->second; + if (found.second) { // Not found, inserted new. + const auto path_component = KindPath(api_version, kind); #ifdef VERBOSE - LOG(DEBUG) << "KindPath returned {" << path_component.first << ", " - << path_component.second << "}"; + LOG(DEBUG) << "KindPath returned {" << path_component.first << ", " + << path_component.second << "}"; #endif - return QueryMaster(path_component.first + "/namespaces/" + ns + "/" + - path_component.second + "/" + name); + owner = std::move( + QueryMaster(path_component.first + "/namespaces/" + ns + "/" + + path_component.second + "/" + name)); + } + return owner->Clone(); } json::value KubernetesReader::FindTopLevelOwner( diff --git a/src/kubernetes.h b/src/kubernetes.h index d0f5d452..6f739e54 100644 --- a/src/kubernetes.h +++ b/src/kubernetes.h @@ -85,6 +85,8 @@ class KubernetesReader { // A memoized map from version to a map from kind to name. mutable std::map> version_to_kind_to_name_; + // A memoized map from an encoded owner reference to the owner object. + mutable std::map owners_; const MetadataAgentConfiguration& config_; Environment environment_; From 7dce675326e7deb87d5aad57d81faf645105221c Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Wed, 20 Dec 2017 11:33:12 -0500 Subject: [PATCH 2/2] Store and look up owner references by uid instead of by name. --- src/kubernetes.cc | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index f68b9e19..e5a42346 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -527,9 +527,12 @@ json::value KubernetesReader::GetOwner( const std::string api_version = owner_ref->Get("apiVersion"); const std::string kind = owner_ref->Get("kind"); const std::string name = owner_ref->Get("name"); + const std::string uid = owner_ref->Get("uid"); + // Even though we query by name, we should look up the owner by uid, + // to handle the case when an object is deleted and re-constructed. const std::string encoded_ref = boost::algorithm::join( - std::vector{api_version, kind, ns, name}, "/"); + std::vector{api_version, kind, uid}, "/"); std::lock_guard lock(mutex_); auto found = owners_.emplace(std::piecewise_construct, @@ -545,6 +548,19 @@ json::value KubernetesReader::GetOwner( owner = std::move( QueryMaster(path_component.first + "/namespaces/" + ns + "/" + path_component.second + "/" + name)); + // Sanity check: because we are looking up by name, the object we get + // back might have a different uid. + const json::Object* owner_obj = owner->As(); + const json::Object* metadata = owner_obj->Get("metadata"); + const std::string owner_uid = metadata->Get("uid"); + if (owner_uid != uid) { + LOG(WARNING) << "Owner " << kind << "'" << name << "' (id " << uid + << ") disappeared before we could query it. Found id " + << owner_uid << " instead."; + owner.reset(); + throw QueryException("Owner " + kind + " " + name + " (id " + uid + + ") disappeared"); + } } return owner->Clone(); }