Skip to content

Commit

Permalink
pkg/operator/utils: Log diff on CredentialsRequest status change
Browse files Browse the repository at this point in the history
'status has changed, updating' shows that *something* is changing.
But without a diff, it's hard to figure out what.  For example in this
recent CI run [1]:

  $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cloud-credential-operator/789/pull-ci-openshift-cloud-credential-operator-master-e2e-aws-manual-oidc/1864768460005838848/artifacts/e2e-aws-manual-oidc/gather-extra/artifacts/pods/openshift-cloud-credential-operator_cloud-credential-operator-64bcbb54cc-vzs5s_cloud-credential-operator.log | grep -1 ingress | tail -n13
  --
  time="2024-12-05T21:44:49Z" level=info msg="status has changed, updating" controller=credreq cr=openshift-cloud-credential-operator/aws-ebs-csi-driver-operator secret=openshift-cluster-csi-drivers/ebs-cloud-credentials
  time="2024-12-05T21:44:49Z" level=info msg="operator detects timed access token enabled cluster (STS, Workload Identity, etc.)" controller=credreq cr=openshift-cloud-credential-operator/openshift-ingress
  time="2024-12-05T21:44:49Z" level=info msg="syncing credentials request" controller=credreq cr=openshift-cloud-credential-operator/openshift-ingress
  time="2024-12-05T21:44:49Z" level=info msg="status has changed, updating" controller=credreq cr=openshift-cloud-credential-operator/openshift-ingress secret=openshift-ingress-operator/cloud-credentials
  time="2024-12-05T21:44:49Z" level=info msg="reconciling clusteroperator status"
  --
  time="2024-12-05T21:44:51Z" level=info msg="reconciling clusteroperator status"
  time="2024-12-05T21:44:52Z" level=info msg="operator detects timed access token enabled cluster (STS, Workload Identity, etc.)" controller=credreq cr=openshift-cloud-credential-operator/openshift-ingress
  time="2024-12-05T21:44:52Z" level=info msg="syncing credentials request" controller=credreq cr=openshift-cloud-credential-operator/openshift-ingress
  time="2024-12-05T21:44:52Z" level=info msg="reconciling clusteroperator status"
  time="2024-12-05T21:44:52Z" level=info msg="status has changed, updating" controller=credreq cr=openshift-cloud-credential-operator/openshift-ingress secret=openshift-ingress-operator/cloud-credentials
  time="2024-12-05T21:44:52Z" level=info msg="operator detects timed access token enabled cluster (STS, Workload Identity, etc.)" controller=credreq cr=openshift-cloud-credential-operator/openshift-machine-api-aws

Is that a hot loop?  Is something really changing?  Hard to debug.

With this commit, we'll log the change we make.  It will increase the
log level when there's a hot loop, but it will also make it easier for
us to identify and fix hot loops, so overall log verbosity should go
down (more text per update * orders of magnitude fewer updates).

[1]: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cloud-credential-operator/789/pull-ci-openshift-cloud-credential-operator-master-e2e-aws-manual-oidc/1864768460005838848
  • Loading branch information
wking committed Dec 20, 2024
1 parent 2395cbc commit e61e7f0
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions pkg/operator/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package utils
import (
"context"
"fmt"
"reflect"
"strconv"
"strings"

Expand All @@ -17,6 +16,7 @@ import (

"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/google/go-cmp/cmp"
log "github.com/sirupsen/logrus"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -477,8 +477,8 @@ func UpdateStatus(client client.Client, origCR, newCR *minterv1.CredentialsReque
logger.Debug("updating credentials request status")

// Update Credentials Request status if changed:
if !reflect.DeepEqual(newCR.Status, origCR.Status) {
logger.Infof("status has changed, updating")
if diff := cmp.Diff(origCR.Status, newCR.Status); diff != "" {
logger.Infof("Updating CredentialsRequest %s/%s status due to diff: %v", newCR.Namespace, newCR.Name, diff)
err := client.Status().Update(context.TODO(), newCR)
if err != nil {
logger.WithError(err).Error("error updating credentials request")
Expand Down

0 comments on commit e61e7f0

Please sign in to comment.