linkerd / linkerd2

Ultralight, security-first service mesh for Kubernetes. Main repo for Linkerd 2.x.
https://linkerd.io
Apache License 2.0
10.72k stars 1.28k forks source link

chore(policy): polish logging #13379

Closed olix0r closed 3 days ago

olix0r commented 6 days ago

There are a few things about the policy controller logging that can be cleaned up for consistency and clarity:

DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder has changed
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder has changed
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder has changed
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l
DEBUG status::Index: linkerd_policy_controller_k8s_status::index: Lease holder reconciling cluster index.name=linkerd-destination-74d7fdc45d-xfb8l

The "Lease holder has changed" message actually indicates that the lease has changed, though the holder may be unchanged.

To improve logging clarity, this change does the following:

olix0r commented 4 days ago

Reconciliation logs have been improved so that we log the number of resources being inspected, as well as the total patches in a reconciliation. Tracing contexts are set so we know which resource is being updated.

The awkward "Lease non-holder skipping controller update" DEBUG messages have been consolidated in a helper as

    fn reconcile_if_leader(&self) {
        let lease = self.claims.borrow();
        if !lease.is_current_for(&self.name) {
            tracing::trace!(%lease.holder, "Reconcilation skipped");
            return;
        }
        self.reconcile();
    } 

Example log output here.

cratelyn commented 3 days ago

note that two ci jobs appear to be failing, my understanding is that these are known to be flaky.