Skip to content

Commit e090d1d

Browse files
committed
policy: clean up logs and error messages
Use NetworkPolicyResource and NetworkPolicy consistently for the two typeURLs to make grepping logs more meaningful. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
1 parent 18261d2 commit e090d1d

2 files changed

Lines changed: 57 additions & 56 deletions

File tree

cilium/network_policy.cc

Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,12 @@ class ResourceMapOverlay {
249249
const auto* entry = findEntry(selector);
250250
if (entry == nullptr) {
251251
throw EnvoyException(fmt::format(
252-
"Delta Network Policy rule references missing selector resource '{}'", selector));
252+
"NetworkPolicyResource rule references missing selector resource '{}'", selector));
253253
}
254254
const auto* selector_entry = entry->selectorResourceEntry();
255255
if (selector_entry == nullptr || selector_entry->handle == nullptr) {
256256
throw EnvoyException(
257-
fmt::format("Delta Network Policy rule references non-selector resource '{}'", selector));
257+
fmt::format("NetworkPolicyResource rule references non-selector resource '{}'", selector));
258258
}
259259
return selector_entry->handle;
260260
}
@@ -951,7 +951,7 @@ class PortNetworkPolicyRule : public Logger::Loggable<Logger::Id::config> {
951951
if (resource_map) {
952952
if (rule.remote_policies_size()) {
953953
throw EnvoyException(
954-
"Delta Network Policy rule must use selectors instead of remote_policies");
954+
"NetworkPolicyResource rule must use selectors instead of remote_policies");
955955
}
956956
selectors_.reserve(rule.selectors_size());
957957
for (const auto& selector : rule.selectors()) {
@@ -961,7 +961,7 @@ class PortNetworkPolicyRule : public Logger::Loggable<Logger::Id::config> {
961961
}
962962
} else {
963963
if (rule.selectors_size()) {
964-
throw EnvoyException("State-of-the-world Network Policy rule must not use selectors");
964+
throw EnvoyException("NetworkPolicy rule must not use selectors");
965965
}
966966
for (const auto remote : rule.remote_policies()) {
967967
ENVOY_LOG(trace, "Cilium L7 PortNetworkPolicyRule(): {} remote {} by rule: {}", verdict_,
@@ -1594,7 +1594,7 @@ struct PortRangeCompare {
15941594

15951595
// PolicySnapshot is keyed by port ranges, and contains a list of PortNetworkPolicyRules's
15961596
// applicable to this range. A list is needed as rules may come from multiple sources (e.g.,
1597-
// resulting from use of named ports and numbered ports in Cilium Network Policy at the same time).
1597+
// resulting from use of named ports and numbered ports in Cilium NetworkPolicy at the same time).
15981598
class PolicySnapshot : public absl::btree_map<PortRange, PortNetworkPolicyRules, PortRangeCompare> {
15991599
public:
16001600
using absl::btree_map<PortRange, PortNetworkPolicyRules, PortRangeCompare>::btree_map;
@@ -2256,7 +2256,7 @@ NetworkPolicyMapImpl::NetworkPolicyMapImpl(Server::Configuration::FactoryContext
22562256
context_(context.serverFactoryContext()), map_ptr_(nullptr),
22572257
npds_stats_scope_(context_.serverScope().createScope("cilium.npds.")),
22582258
policy_stats_scope_(context_.serverScope().createScope("cilium.policy.")),
2259-
init_target_(fmt::format("Cilium Network Policy subscription start"),
2259+
init_target_(fmt::format("Cilium NetworkPolicy subscription start"),
22602260
[this]() {
22612261
// production subscription is allowed to start from now on
22622262
subscription_should_start_ = true;
@@ -2479,16 +2479,16 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
24792479
// policy_stream_state_ gets updated on first successful update,
24802480
// so 'is_new_stream' remains 'true' as long as the stream has not had a successful update yet.
24812481
const bool is_new_stream = stream_generation != policy_stream_state_->streamGeneration();
2482-
ENVOY_LOG(debug, "NetworkPolicyMapImpl::onConfigUpdate({}), {} resources, version: {}",
2483-
instance_id_, resources.size(), version_info);
2482+
ENVOY_LOG(debug, "NetworkPolicyMapImpl::onConfigUpdate({}), {} resources, version: {}, stream {}",
2483+
instance_id_, resources.size(), version_info, stream_generation);
24842484
stats_.updates_total_.inc();
24852485

24862486
// Reopen IPcache for every new stream. Cilium agent re-creates IP cache on restart,
24872487
// and that is also when the old stream terminates and a new one is created.
24882488
// New security identities (e.g., for FQDN policies) only get inserted to the new IP cache,
24892489
// so open it before the workers get a chance to enforce policy on the new IDs.
24902490
if (is_new_stream) {
2491-
ENVOY_LOG(info, "New NetworkPolicy stream");
2491+
ENVOY_LOG(info, "New NetworkPolicy stream {}", stream_generation);
24922492

24932493
reopenIpcache();
24942494
}
@@ -2510,12 +2510,12 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
25102510
for (const auto& resource : resources) {
25112511
const auto& config = dynamic_cast<const cilium::NetworkPolicy&>(resource.get().resource());
25122512
const std::string& resource_name = resource.get().name();
2513-
validateResourceName(resource_name, "Network Policy resource name");
2513+
validateResourceName(resource_name, "NetworkPolicy resource name");
25142514
if (config.endpoint_ips().empty()) {
2515-
throw EnvoyException("Network Policy has no endpoint ips");
2515+
throw EnvoyException("NetworkPolicy has no endpoint ips");
25162516
}
25172517
ENVOY_LOG(debug,
2518-
"Received Network Policy for endpoint {}, endpoint_ip {} in onConfigUpdate() "
2518+
"Received NetworkPolicy for endpoint {}, endpoint_ip {} in onConfigUpdate() "
25192519
"version {}",
25202520
config.endpoint_id(), config.endpoint_ips()[0], version_info);
25212521

@@ -2560,7 +2560,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
25602560
bool updates_policies = false;
25612561
bool updates_selectors = false;
25622562
for (const auto& removed_resource : removed_resources) {
2563-
validateResourceName(removed_resource, "Network Policy delta removed resource name");
2563+
validateResourceName(removed_resource, "NetworkPolicyResource removed resource name");
25642564
auto resource_it = resource_map_.find(removed_resource);
25652565
if (resource_it == resource_map_.end()) {
25662566
continue;
@@ -2576,9 +2576,9 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
25762576
dynamic_cast<const cilium::NetworkPolicyResource&>(resource.get().resource());
25772577
const std::string& resource_name = resource.get().name();
25782578
if (resource_name.empty()) {
2579-
throw EnvoyException("Network Policy delta resource has no name");
2579+
throw EnvoyException("NetworkPolicyResource has no name");
25802580
}
2581-
validateResourceName(resource_name, "Network Policy delta resource name");
2581+
validateResourceName(resource_name, "NetworkPolicyResource added resource name");
25822582
switch (typed_resource.resource_case()) {
25832583
case cilium::NetworkPolicyResource::kPolicy:
25842584
updates_policies = true;
@@ -2593,17 +2593,17 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
25932593

25942594
ENVOY_LOG(debug,
25952595
"NetworkPolicyMapImpl::onConfigUpdate({}), {} added resources, {} removed resources, "
2596-
"version: {}, updates_selectors: {}, updates_policies: {}",
2596+
"version: {}, stream {}, updates_selectors: {}, updates_policies: {}",
25972597
instance_id_, added_resources.size(), removed_resources.size(), system_version_info,
2598-
updates_selectors, updates_policies);
2598+
stream_generation, updates_selectors, updates_policies);
25992599
stats_.updates_total_.inc();
26002600

26012601
// Reopen IPcache for every new stream. Cilium agent re-creates IP cache on restart,
26022602
// and that is also when the old stream terminates and a new one is created.
26032603
// New security identities (e.g., for FQDN policies) only get inserted to the new IP cache,
26042604
// so open it before the workers get a chance to enforce policy on the new IDs.
26052605
if (is_new_stream) {
2606-
ENVOY_LOG(info, "New NetworkPolicy stream");
2606+
ENVOY_LOG(info, "New NetworkPolicyResource stream {}", stream_generation);
26072607
reopenIpcache();
26082608
}
26092609
removeInitManager();
@@ -2615,24 +2615,24 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
26152615
const auto selector_update_version = selector_map_.prepareNextVersion();
26162616

26172617
for (const auto& resource : removed_resources) {
2618-
ENVOY_LOG(trace, "Cilium removing network policy selector resource {}", resource);
2618+
ENVOY_LOG(trace, "Cilium removing NetworkPolicyResource selector {}", resource);
26192619
const auto* resource_entry = pending_resource_map.findEntry(resource);
26202620
if (resource_entry == nullptr) {
26212621
ENVOY_LOG(
26222622
debug,
2623-
"NetworkPolicy delta removed selector resource name '{}' not found from resource map",
2623+
"NetworkPolicyResource removed selector name '{}' not found from resource map",
26242624
resource);
26252625
continue;
26262626
}
26272627
if (resource_entry->isPolicyEndpointIpEntry()) {
2628-
throw EnvoyException(fmt::format("NetworkPolicy delta removed selector resource name "
2628+
throw EnvoyException(fmt::format("NetworkPolicyResource removed selector name "
26292629
"'{}' is a policy endpoint IP alias, "
26302630
"not a resource name",
26312631
resource));
26322632
}
26332633
if (resource_entry->policyResourceEntry()) {
26342634
throw EnvoyException(fmt::format(
2635-
"NetworkPolicy delta removed selector resource name '{}' refers to a policy resource",
2635+
"NetworkPolicyResource removed selector name '{}' refers to a policy resource",
26362636
resource));
26372637
}
26382638
selector_map_.clear(resource);
@@ -2657,15 +2657,15 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
26572657
switch (typed_resource.resource_case()) {
26582658
case cilium::NetworkPolicyResource::kSelector: {
26592659
ENVOY_LOG(debug,
2660-
"Received delta Network Policy selector resource {} in onConfigUpdate() "
2660+
"Received NetworkPolicyResource selector {} in onConfigUpdate() "
26612661
"version {}",
26622662
resource_name, system_version_info);
26632663
auto selector_handle = createOrReuseSelector(resource_name, typed_resource.selector(),
26642664
selector_update_version);
26652665
if (!pending_resource_map.emplace(resource_name,
26662666
ResourceKey::selectorResource(selector_handle))) {
26672667
throw EnvoyException(fmt::format(
2668-
"Network Policy delta selector update for version {} has duplicate resource key "
2668+
"NetworkPolicyResource selector update for version {} has duplicate resource key "
26692669
"'{}' on an old stream: "
26702670
"incoming selector resource '{}' collides with existing {}",
26712671
system_version_info, resource_name, resource_name,
@@ -2674,14 +2674,14 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
26742674
break;
26752675
}
26762676
case cilium::NetworkPolicyResource::kPolicy:
2677-
IS_ENVOY_BUG("Selector-only delta Network Policy update unexpectedly included a policy");
2677+
IS_ENVOY_BUG("Selector-only NetworkPolicyResource update unexpectedly included a policy");
26782678
break;
26792679
case cilium::NetworkPolicyResource::RESOURCE_NOT_SET:
2680-
throw EnvoyException("Network Policy delta resource has no payload");
2680+
throw EnvoyException("NetworkPolicyResource has no payload");
26812681
}
26822682
}
26832683
} catch (const EnvoyException& e) {
2684-
ENVOY_LOG(warn, "NetworkPolicy delta update for version {} failed: {}", system_version_info,
2684+
ENVOY_LOG(warn, "NetworkPolicyResource update for version {} failed: {}", system_version_info,
26852685
e.what());
26862686
stats_.updates_rejected_.inc();
26872687
scheduleSelectorDeferredDeletion(selector_map_.revert());
@@ -2717,7 +2717,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
27172717
const auto selector_update_version = selector_map_.prepareNextVersion();
27182718

27192719
for (const auto& removed_resource : removed_resources) {
2720-
ENVOY_LOG(trace, "Cilium removing network policy resource {}", removed_resource);
2720+
ENVOY_LOG(trace, "Cilium removing NetworkPolicyResource {}", removed_resource);
27212721
const auto* resource_entry = pending_resource_map.findEntry(removed_resource);
27222722
if (resource_entry == nullptr) {
27232723
continue;
@@ -2731,7 +2731,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
27312731
continue;
27322732
}
27332733
throw EnvoyException(
2734-
fmt::format("Network Policy delta removed resource '{}' is a policy endpoint IP alias, "
2734+
fmt::format("NetworkPolicyResource removed resource '{}' is a policy endpoint IP alias, "
27352735
"not a resource name",
27362736
removed_resource));
27372737
}
@@ -2767,15 +2767,15 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
27672767
}
27682768

27692769
ENVOY_LOG(debug,
2770-
"Received delta Network Policy selector resource {} in onConfigUpdate() "
2770+
"Received NetworkPolicyResource selector {} in onConfigUpdate() "
27712771
"version {}",
27722772
resource_name, system_version_info);
27732773
auto selector_handle =
27742774
createOrReuseSelector(resource_name, typed_resource.selector(), selector_update_version);
27752775
if (!pending_resource_map.emplace(resource_name,
27762776
ResourceKey::selectorResource(selector_handle))) {
27772777
throw EnvoyException(fmt::format(
2778-
"Network Policy delta update for version {} has duplicate resource key '{}' on {} "
2778+
"NetworkPolicyResource update for version {} has duplicate resource key '{}' on {} "
27792779
"stream: "
27802780
"incoming selector resource '{}' collides with existing {}",
27812781
system_version_info, resource_name, is_new_stream ? "a new" : "an old", resource_name,
@@ -2794,13 +2794,13 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
27942794
case cilium::NetworkPolicyResource::kPolicy: {
27952795
const auto& config = typed_resource.policy();
27962796
if (config.endpoint_ips().empty()) {
2797-
throw EnvoyException("Network Policy has no endpoint ips");
2797+
throw EnvoyException("NetworkPolicyResource has no endpoint ips");
27982798
}
27992799
if (config.endpoint_id() == 0) {
2800-
throw EnvoyException("Network Policy endpoint_id must be non-zero");
2800+
throw EnvoyException("NetworkPolicyResource endpoint_id must be non-zero");
28012801
}
28022802
ENVOY_LOG(debug,
2803-
"Received delta Network Policy resource {} for endpoint {}, endpoint_ip {} in "
2803+
"Received NetworkPolicyResource {} for endpoint {}, endpoint_ip {} in "
28042804
"onConfigUpdate() version {}",
28052805
resource_name, config.endpoint_id(), config.endpoint_ips()[0],
28062806
system_version_info);
@@ -2809,7 +2809,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
28092809
&pending_resource_map);
28102810
if (!pending_resource_map.emplace(resource_name, ResourceKey::policyResource(policy))) {
28112811
throw EnvoyException(fmt::format(
2812-
"Network Policy delta update for version {} has duplicate resource key '{}' on {} "
2812+
"NetworkPolicyResource update for version {} has duplicate resource key '{}' on {} "
28132813
"stream: "
28142814
"incoming {} collides with existing {}",
28152815
system_version_info, resource_name, is_new_stream ? "a new" : "an old",
@@ -2820,7 +2820,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
28202820
ENVOY_LOG(trace, "Cilium updating network policy for endpoint {}", endpoint_ip);
28212821
if (!pending_resource_map.emplace(endpoint_ip, ResourceKey::policyEndpointIp())) {
28222822
throw EnvoyException(fmt::format(
2823-
"Network Policy delta update for version {} has duplicate resource key '{}' on {} "
2823+
"NetworkPolicyResource update for version {} has duplicate resource key '{}' on {} "
28242824
"stream: "
28252825
"incoming {} collides with existing {}",
28262826
system_version_info, endpoint_ip, is_new_stream ? "a new" : "an old",
@@ -2829,7 +2829,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
28292829
}
28302830
if (!new_policy_map.emplace(endpoint_ip, policy).second) {
28312831
throw EnvoyException(fmt::format(
2832-
"Network Policy delta update for version {} has duplicate resource key '{}' on {} "
2832+
"NetworkPolicyResource update for version {} has duplicate resource key '{}' on {} "
28332833
"stream: "
28342834
"incoming {} collides with existing {}",
28352835
system_version_info, endpoint_ip, is_new_stream ? "a new" : "an old",
@@ -2840,11 +2840,11 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate(
28402840
break;
28412841
}
28422842
case cilium::NetworkPolicyResource::RESOURCE_NOT_SET:
2843-
throw EnvoyException("Network Policy delta resource has no payload");
2843+
throw EnvoyException("NetworkPolicyResource has no payload");
28442844
}
28452845
}
28462846
} catch (const EnvoyException& e) {
2847-
ENVOY_LOG(warn, "NetworkPolicy delta update for version {} failed: {}", system_version_info,
2847+
ENVOY_LOG(warn, "NetworkPolicyResource update for version {} failed: {}", system_version_info,
28482848
e.what());
28492849
stats_.updates_rejected_.inc();
28502850
removeInitManager();
@@ -2867,7 +2867,8 @@ void NetworkPolicyMapImpl::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailu
28672867
const EnvoyException*) {
28682868
// We need to allow server startup to continue, even if we have a bad
28692869
// config.
2870-
ENVOY_LOG(debug, "Network Policy Update failed, keeping existing policy.");
2870+
ENVOY_LOG(debug, "NetworkPolicy update on stream {} failed, keeping existing policy.",
2871+
streamGeneration());
28712872
}
28722873

28732874
ProtobufTypes::MessagePtr

0 commit comments

Comments
 (0)