From 6705868b24aadeedc9c685fe27d16f21e4cbdea0 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Wed, 20 Dec 2017 19:19:21 -0500 Subject: [PATCH 1/2] Since containerStatuses and containerSpecs may not be in the same order, look up containers by name, rather than by index. --- src/kubernetes.cc | 79 +++++++++++++++++------------------------------ src/kubernetes.h | 5 +-- 2 files changed, 32 insertions(+), 52 deletions(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index 04125295..346d1876 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -250,7 +250,8 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetPodMetadata( } MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( - const json::Object* pod, int container_index, json::value associations, + const json::Object* pod, const json::Object* container_status, + const json::Object* container_spec, json::value associations, Timestamp collected_at) const throw(json::Exception) { const std::string zone = environment_.InstanceZone(); const std::string cluster_name = environment_.KubernetesClusterName(); @@ -267,34 +268,20 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( const json::Object* spec = pod->Get("spec"); const std::string node_name = spec->Get("nodeName"); - const json::Object* status = pod->Get("status"); - - const json::Array* container_specs = spec->Get("containers"); - const json::Array* container_statuses = - status->Get("containerStatuses"); - - const json::value& c_status = (*container_statuses)[container_index]; - const json::value& c_spec = (*container_specs)[container_index]; - if (config_.VerboseLogging()) { - LOG(INFO) << "Container: " << *c_status; - } - const json::Object* container = c_status->As(); - const json::Object* container_spec = c_spec->As(); - const std::string container_name = container->Get("name"); + const std::string container_name = container_spec->Get("name"); std::size_t docker_prefix_end = sizeof(kDockerIdPrefix) - 1; - if (container->Get("containerID").compare( - 0, docker_prefix_end, kDockerIdPrefix) != 0) { + const std::string docker_id = + container_status->Get("containerID"); + if (docker_id.compare(0, docker_prefix_end, kDockerIdPrefix) != 0) { LOG(ERROR) << "ContainerID " - << container->Get("containerID") + << docker_id << " does not start with " << kDockerIdPrefix << " (" << docker_prefix_end << " chars)"; docker_prefix_end = 0; } - const std::string container_id = - container->Get("containerID").substr( - docker_prefix_end); + const std::string container_id = docker_id.substr(docker_prefix_end); // TODO: find is_deleted. - //const json::Object* state = container->Get("state"); + //const json::Object* state = container_status->Get("state"); bool is_deleted = false; const MonitoredResource k8s_container("k8s_container", { @@ -315,7 +302,7 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( })}, {"status", json::object({ {"version", json::string(kKubernetesApiVersion)}, - {"raw", container->Clone()}, + {"raw", container_status->Clone()}, })}, {"labels", json::object({ {"version", json::string(kKubernetesApiVersion)}, @@ -350,7 +337,7 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( } MetadataUpdater::ResourceMetadata KubernetesReader::GetLegacyResource( - const json::Object* pod, int container_index) const + const json::Object* pod, const std::string& container_name) const throw(json::Exception) { const std::string instance_id = environment_.InstanceId(); const std::string zone = environment_.InstanceZone(); @@ -361,18 +348,6 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetLegacyResource( const std::string pod_name = metadata->Get("name"); const std::string pod_id = metadata->Get("uid"); - const json::Object* status = pod->Get("status"); - - const json::Array* container_statuses = - status->Get("containerStatuses"); - - const json::value& c_status = (*container_statuses)[container_index]; - if (config_.VerboseLogging()) { - LOG(INFO) << "Container: " << *c_status; - } - const json::Object* container = c_status->As(); - const std::string container_name = container->Get("name"); - const MonitoredResource gke_container("gke_container", { {"cluster_name", cluster_name}, {"namespace_id", namespace_name}, @@ -414,27 +389,31 @@ KubernetesReader::GetPodAndContainerMetadata( const json::Array* container_specs = spec->Get("containers"); const json::Array* container_statuses = status->Get("containerStatuses"); - std::size_t num_containers = std::min( - container_statuses->size(), container_specs->size()); - for (int i = 0; i < num_containers; ++i) { - const json::value& c_status = (*container_statuses)[i]; - const json::value& c_spec = (*container_specs)[i]; + // Move the container specs into a map from name to spec. + std::map container_spec_by_name; + for (const json::value& c_spec : *container_specs) { + const json::Object* container_spec = c_spec->As(); + const std::string name = container_spec->Get("name"); + container_spec_by_name.emplace(name, container_spec); + } + + for (const json::value& c_status : *container_statuses) { if (config_.VerboseLogging()) { LOG(INFO) << "Container: " << *c_status; } const json::Object* container = c_status->As(); - const json::Object* container_spec = c_spec->As(); - const std::string container_name = container->Get("name"); - const std::string spec_name = container_spec->Get("name"); - if (container_name != spec_name) { - LOG(ERROR) << "Internal error; container name " << container_name - << " not the same as spec name " << spec_name - << " at index " << i; + const std::string name = container->Get("name"); + auto spec_it = container_spec_by_name.find(name); + if (spec_it == container_spec_by_name.end()) { + LOG(ERROR) << "Internal error; spec not found for container " << name; + continue; } - result.emplace_back(GetLegacyResource(pod, i)); + const json::Object* container_spec = spec_it->second; + result.emplace_back(GetLegacyResource(pod, name)); result.emplace_back( - GetContainerMetadata(pod, i, associations->Clone(), collected_at)); + GetContainerMetadata(pod, container, container_spec, + associations->Clone(), collected_at)); } result.emplace_back( diff --git a/src/kubernetes.h b/src/kubernetes.h index d669be92..88bdd980 100644 --- a/src/kubernetes.h +++ b/src/kubernetes.h @@ -89,12 +89,13 @@ class KubernetesReader { const throw(json::Exception); // Given a pod object and container index, return the container metadata. MetadataUpdater::ResourceMetadata GetContainerMetadata( - const json::Object* pod, int container_index, json::value associations, + const json::Object* pod, const json::Object* container_status, + const json::Object* container_spec, json::value associations, Timestamp collected_at) const throw(json::Exception); // Given a pod object and container index, return the legacy resource. // The returned "metadata" field will be Metadata::IGNORED. MetadataUpdater::ResourceMetadata GetLegacyResource( - const json::Object* pod, int container_index) const + const json::Object* pod, const std::string& container_name) const throw(json::Exception); // Given a pod object, return the associated pod and container metadata. std::vector GetPodAndContainerMetadata( From e0eded959a7771a70d35c40c4f8ac568b5e28e00 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 29 Dec 2017 13:28:07 -0500 Subject: [PATCH 2/2] Address feedback: - Rename variable. - Update method comments. --- src/kubernetes.cc | 6 +++--- src/kubernetes.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index 346d1876..a8fc5a73 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -402,8 +402,8 @@ KubernetesReader::GetPodAndContainerMetadata( if (config_.VerboseLogging()) { LOG(INFO) << "Container: " << *c_status; } - const json::Object* container = c_status->As(); - const std::string name = container->Get("name"); + const json::Object* container_status = c_status->As(); + const std::string name = container_status->Get("name"); auto spec_it = container_spec_by_name.find(name); if (spec_it == container_spec_by_name.end()) { LOG(ERROR) << "Internal error; spec not found for container " << name; @@ -412,7 +412,7 @@ KubernetesReader::GetPodAndContainerMetadata( const json::Object* container_spec = spec_it->second; result.emplace_back(GetLegacyResource(pod, name)); result.emplace_back( - GetContainerMetadata(pod, container, container_spec, + GetContainerMetadata(pod, container_status, container_spec, associations->Clone(), collected_at)); } diff --git a/src/kubernetes.h b/src/kubernetes.h index 88bdd980..25f0aaa0 100644 --- a/src/kubernetes.h +++ b/src/kubernetes.h @@ -87,12 +87,12 @@ class KubernetesReader { MetadataUpdater::ResourceMetadata GetPodMetadata( json::value raw_pod, json::value associations, Timestamp collected_at) const throw(json::Exception); - // Given a pod object and container index, return the container metadata. + // Given a pod object and container info, return the container metadata. MetadataUpdater::ResourceMetadata GetContainerMetadata( const json::Object* pod, const json::Object* container_status, const json::Object* container_spec, json::value associations, Timestamp collected_at) const throw(json::Exception); - // Given a pod object and container index, return the legacy resource. + // Given a pod object and container name, return the legacy resource. // The returned "metadata" field will be Metadata::IGNORED. MetadataUpdater::ResourceMetadata GetLegacyResource( const json::Object* pod, const std::string& container_name) const