From e661cde773ed3eaf6108ed2e9c698e11f6a881e4 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 12 Jan 2018 14:31:46 -0500 Subject: [PATCH 1/4] Use container spec to find the status, since the spec is always present. --- src/kubernetes.cc | 37 +++++++++++++++++++------------------ src/kubernetes.h | 4 ++-- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index 35801212..dbd846dc 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -249,8 +249,8 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetPodMetadata( } MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( - const json::Object* pod, const json::Object* container_status, - const json::Object* container_spec, json::value associations, + const json::Object* pod, const json::Object* container_spec, + const json::Object* container_status, json::value associations, Timestamp collected_at) const throw(json::Exception) { const std::string zone = environment_.InstanceZone(); const std::string cluster_name = environment_.KubernetesClusterName(); @@ -400,29 +400,30 @@ KubernetesReader::GetPodAndContainerMetadata( const json::Array* container_statuses = status->Get("containerStatuses"); - // 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); + // Move the container statuses into a map from name to status. + std::map container_status_by_name; + for (const json::value& c_status : *container_statuses) { + const json::Object* container_status = c_status->As(); + const std::string name = container_status->Get("name"); + container_status_by_name.emplace(name, container_status); } - for (const json::value& c_status : *container_statuses) { + for (const json::value& c_spec : *container_specs) { if (config_.VerboseLogging()) { - LOG(INFO) << "Container: " << *c_status; + LOG(INFO) << "Container: " << *c_spec; } - 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; - continue; + const json::Object* container_spec = c_spec->As(); + const std::string name = container_spec->Get("name"); + auto status_it = container_status_by_name.find(name); + const json::Object* container_status; + if (status_it == container_status_by_name.end()) { + container_status = nullptr; + } else { + container_status = status_it->second; } - const json::Object* container_spec = spec_it->second; result.emplace_back(GetLegacyResource(pod, name)); result.emplace_back( - GetContainerMetadata(pod, container_status, container_spec, + GetContainerMetadata(pod, container_spec, container_status, associations->Clone(), collected_at)); } diff --git a/src/kubernetes.h b/src/kubernetes.h index 25f0aaa0..601c6e7d 100644 --- a/src/kubernetes.h +++ b/src/kubernetes.h @@ -89,8 +89,8 @@ class KubernetesReader { const throw(json::Exception); // 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, + const json::Object* pod, const json::Object* container_spec, + const json::Object* container_status, json::value associations, Timestamp collected_at) const throw(json::Exception); // Given a pod object and container name, return the legacy resource. // The returned "metadata" field will be Metadata::IGNORED. From 744963cc5c825f19afb352d48e1d19900a733dbe Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 12 Jan 2018 14:37:24 -0500 Subject: [PATCH 2/4] Get rid of the unused container_id. --- src/kubernetes.cc | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index dbd846dc..77006766 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -60,8 +60,6 @@ constexpr const char kNodeSelectorPrefix[] = "?fieldSelector=spec.nodeName%3D"; constexpr const char kWatchParam[] = "watch=true"; -constexpr const char kDockerIdPrefix[] = "docker://"; - constexpr const char kServiceAccountDirectory[] = "/var/run/secrets/kubernetes.io/serviceaccount"; @@ -273,17 +271,6 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( const std::string node_name = spec->Get("nodeName"); const std::string container_name = container_spec->Get("name"); - std::size_t docker_prefix_end = sizeof(kDockerIdPrefix) - 1; - const std::string docker_id = - container_status->Get("containerID"); - if (docker_id.compare(0, docker_prefix_end, kDockerIdPrefix) != 0) { - LOG(ERROR) << "ContainerID " - << docker_id - << " does not start with " << kDockerIdPrefix - << " (" << docker_prefix_end << " chars)"; - docker_prefix_end = 0; - } - const std::string container_id = docker_id.substr(docker_prefix_end); // TODO: find is_deleted. //const json::Object* state = container_status->Get("state"); bool is_deleted = false; @@ -324,9 +311,6 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( LOG(INFO) << "Raw container metadata: " << *container_raw_metadata; } - const std::string k8s_container_id = boost::algorithm::join( - std::vector{kK8sContainerResourcePrefix, container_id}, - kResourceTypeSeparator); const std::string k8s_container_pod = boost::algorithm::join( std::vector{kK8sContainerResourcePrefix, pod_id, container_name}, kResourceTypeSeparator); @@ -334,7 +318,7 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( std::vector{kK8sContainerNameResourcePrefix, namespace_name, pod_name, container_name}, kResourceTypeSeparator); return MetadataUpdater::ResourceMetadata( - std::vector{k8s_container_id, k8s_container_pod, k8s_container_name}, + std::vector{k8s_container_pod, k8s_container_name}, k8s_container, #ifdef ENABLE_KUBERNETES_METADATA MetadataAgent::Metadata(kKubernetesApiVersion, From a1f0b096752fcea2f0af4807eeb7839af929960e Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 12 Jan 2018 14:21:25 -0500 Subject: [PATCH 3/4] Make container_status optional. --- src/kubernetes.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index 77006766..ff48851b 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -290,11 +290,16 @@ MetadataUpdater::ResourceMetadata KubernetesReader::GetContainerMetadata( {"version", json::string(kKubernetesApiVersion)}, {"raw", container_spec->Clone()}, })}, - {"status", json::object({ - {"version", json::string(kKubernetesApiVersion)}, - {"raw", container_status->Clone()}, - })}, })); + if (container_status) { + blobs->emplace(std::make_pair( + "status", + json::object({ + {"version", json::string(kKubernetesApiVersion)}, + {"raw", container_status->Clone()}, + }) + )); + } if (labels) { blobs->emplace(std::make_pair( "labels", From cbb874e3291df13fb212440c69715470e1dfc1d7 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Fri, 12 Jan 2018 17:20:49 -0500 Subject: [PATCH 4/4] Log container status as well. --- src/kubernetes.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/kubernetes.cc b/src/kubernetes.cc index ff48851b..266bd452 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -407,8 +407,14 @@ KubernetesReader::GetPodAndContainerMetadata( const json::Object* container_status; if (status_it == container_status_by_name.end()) { container_status = nullptr; + if (config_.VerboseLogging()) { + LOG(INFO) << "No status for container " << name; + } } else { container_status = status_it->second; + if (config_.VerboseLogging()) { + LOG(INFO) << "Container status: " << *container_status; + } } result.emplace_back(GetLegacyResource(pod, name)); result.emplace_back(