Skip to content

Commit 976cb49

Browse files
authored
Enabling logs in critical paths (#62)
<!-- Thanks for sending a pull request! Here are some tips for you: 1. Ensure you have added the unit tests for your changes. 2. Ensure you have included output of manual testing done in the Testing section. 3. Ensure number of lines of code for new or existing methods are within the reasonable limit. 4. Ensure your change works on existing clusters after upgrade. --> **What type of PR is this?** <!-- Add one of the following: bug cleanup documentation feature --> cleanup **Which issue does this PR fix**: **What does this PR do / Why do we need it**: Since we are running this controller in EKS managed control plane, we should enable critical logs by default. **If an issue # is not available please add steps to reproduce and the controller logs**: **Testing done on this change**: <!-- output of manual testing/integration tests results and also attach logs showing the fix being resolved --> Ran the controller locally and confirmed. **Automation added to e2e**: <!-- List the e2e tests you added as part of this PR. If no, create an issue with enhancement/testing label --> **Will this PR introduce any new dependencies?**: <!-- e.g. new K8s API --> **Will this break upgrades or downgrades. Has updating a running cluster been tested?**: **Does this PR introduce any user-facing change?**: <!-- If yes, a release note update is required: Enter your extended release note in the block below. If the PR requires additional actions from users switching to the new release, include the string "action required". --> ```release-note ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
2 parents 885b675 + 03e22b5 commit 976cb49

File tree

4 files changed

+22
-11
lines changed

4 files changed

+22
-11
lines changed

internal/controllers/policy_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (r *policyReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage
122122
func (r *policyReconciler) reconcile(ctx context.Context, request reconcile.Request) error {
123123
policy := &networking.NetworkPolicy{}
124124
if err := r.k8sClient.Get(ctx, request.NamespacedName, policy); err != nil {
125-
r.logger.V(1).Info("Unable to get policy", "resource", policy, "err", err)
125+
r.logger.Info("Unable to get policy", "resource", policy, "err", err)
126126
return client.IgnoreNotFound(err)
127127
}
128128
if !policy.DeletionTimestamp.IsZero() {

pkg/resolvers/endpoints.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ func (r *defaultEndpointsResolver) resolveNetworkPeers(ctx context.Context, poli
167167
// However, in this case we are not able to resolve any of the ports from the CIDR list alone. In this
168168
// case we do not add the CIDR to the list of resolved peers to prevent allow all ports.
169169
if len(ports) != 0 && len(portList) == 0 {
170+
r.logger.Info("Couldn't resolve ports from given CIDR list and will skip this rule", "peer", peer)
170171
continue
171172
}
172173
networkPeers = append(networkPeers, policyinfo.EndpointInfo{
@@ -215,7 +216,7 @@ func (r *defaultEndpointsResolver) getIngressRulesPorts(ctx context.Context, pol
215216
var portList []policyinfo.Port
216217
for _, pod := range podList.Items {
217218
portList = append(portList, r.getPortList(pod, ports)...)
218-
r.logger.Info("got ingress port", "port", portList, "pod", pod)
219+
r.logger.Info("Got ingress port", "port", portList, "pod", pod)
219220
}
220221

221222
return portList
@@ -260,7 +261,7 @@ func (r *defaultEndpointsResolver) resolveServiceClusterIPs(ctx context.Context,
260261
return nil, err
261262
}
262263
}
263-
r.logger.V(1).Info("Namespaces for service clusterIP lookup", "list", namespaces)
264+
r.logger.Info("Populated namespaces for service clusterIP lookup", "list", namespaces)
264265
for _, ns := range namespaces {
265266
networkPeers = append(networkPeers, r.getMatchingServiceClusterIPs(ctx, peer.PodSelector, ns, ports)...)
266267
}
@@ -323,11 +324,12 @@ func (r *defaultEndpointsResolver) getMatchingPodAddresses(ctx context.Context,
323324
for _, pod := range podList.Items {
324325
podIP := k8s.GetPodIP(&pod)
325326
if len(podIP) == 0 {
326-
r.logger.V(1).Info("pod IP not assigned yet", "pod", k8s.NamespacedName(&pod))
327+
r.logger.Info("pod IP not assigned yet", "pod", k8s.NamespacedName(&pod))
327328
continue
328329
}
329330
portList := r.getPortList(pod, ports)
330331
if len(ports) != len(portList) && len(portList) == 0 {
332+
r.logger.Info("Couldn't get matched port list from the pod", "pod", k8s.NamespacedName(&pod), "expectedPorts", ports)
331333
continue
332334
}
333335
addresses = append(addresses, policyinfo.EndpointInfo{
@@ -382,16 +384,24 @@ func (r *defaultEndpointsResolver) getMatchingServiceClusterIPs(ctx context.Cont
382384
return nil
383385
}
384386
for _, svc := range svcList.Items {
385-
if k8s.IsServiceHeadless(&svc) || !svcSelector.Matches(labels.Set(svc.Spec.Selector)) {
387+
// do not add headless services to policy endpoints
388+
if k8s.IsServiceHeadless(&svc) {
389+
r.logger.Info("skipping headless service when populating EndpointInfo", "serviceName", svc.Name, "serviceNamespace", svc.Namespace)
386390
continue
387391
}
392+
// do not add services if their pod selector is not matching with the pod selector defined in the network policy
393+
if !svcSelector.Matches(labels.Set(svc.Spec.Selector)) {
394+
r.logger.Info("skipping pod selector mismatched service when populating EndpointInfo", "serviceName", svc.Name, "serviceNamespace", svc.Namespace, "expectedPS", svcSelector)
395+
continue
396+
}
397+
388398
var portList []policyinfo.Port
389399
for _, port := range ports {
390400
var portPtr *int32
391401
if port.Port != nil {
392402
portVal, err := r.getMatchingServicePort(ctx, &svc, port.Port, *port.Protocol)
393403
if err != nil {
394-
r.logger.V(1).Info("Unable to lookup service port", "err", err)
404+
r.logger.Info("Unable to lookup service port", "err", err)
395405
continue
396406
}
397407
portPtr = &portVal
@@ -403,6 +413,7 @@ func (r *defaultEndpointsResolver) getMatchingServiceClusterIPs(ctx context.Cont
403413
})
404414
}
405415
if len(ports) != len(portList) && len(portList) == 0 {
416+
r.logger.Info("Couldn't find matching port for the service", "service", k8s.NamespacedName(&svc))
406417
continue
407418
}
408419
networkPeers = append(networkPeers, policyinfo.EndpointInfo{
@@ -420,7 +431,7 @@ func (r *defaultEndpointsResolver) getMatchingServicePort(ctx context.Context, s
420431
if portVal, err := k8s.LookupServiceListenPort(svc, *port, protocol); err == nil {
421432
return portVal, nil
422433
} else {
423-
r.logger.V(1).Info("Unable to lookup service port", "err", err)
434+
r.logger.Info("Unable to lookup service port", "err", err)
424435
}
425436
// List pods matching the svc selector
426437
podSelector, err := metav1.LabelSelectorAsSelector(metav1.SetAsLabelSelector(svc.Spec.Selector))
@@ -438,8 +449,9 @@ func (r *defaultEndpointsResolver) getMatchingServicePort(ctx context.Context, s
438449
for i := range podList.Items {
439450
if portVal, err := k8s.LookupListenPortFromPodSpec(svc, &podList.Items[i], *port, protocol); err == nil {
440451
return portVal, nil
452+
} else {
453+
r.logger.Info("The pod doesn't have port matched", "err", err, "pod", podList.Items[i])
441454
}
442-
break
443455
}
444-
return 0, errors.Errorf("unable to find matching service listen port %s", port.String())
456+
return 0, errors.Errorf("unable to find matching service listen port %s for service %s", port.String(), k8s.NamespacedName(svc))
445457
}

pkg/resolvers/endpoints_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,6 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) {
830830
)
831831

832832
for _, rule := range policy.Spec.Egress {
833-
resolver.logger.V(1).Info("computing egress addresses", "peers", rule.To)
834833
if rule.To == nil {
835834
egressEndpoints = append(egressEndpoints, resolver.getAllowAllNetworkPeers(rule.Ports)...)
836835
continue

pkg/resolvers/policies_for_service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
// getReferredPoliciesForService returns the list of policies that refer to the service.
1818
func (r *defaultPolicyReferenceResolver) getReferredPoliciesForService(ctx context.Context, svc, svcOld *corev1.Service) ([]networking.NetworkPolicy, error) {
1919
if k8s.IsServiceHeadless(svc) {
20-
r.logger.V(1).Info("Ignoring headless service", "svc", k8s.NamespacedName(svc))
20+
r.logger.Info("Ignoring headless service", "svc", k8s.NamespacedName(svc))
2121
return nil, nil
2222
}
2323
policiesWithEgressRules := r.policyTracker.GetPoliciesWithEgressRules()

0 commit comments

Comments
 (0)