Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syncStatus: Reduce verbosity when syncing nothing of interest #922

Conversation

petr-muller
Copy link
Member

Status is synchronized every 15 seconds, and each call is currently logged with a lot of metadata, including full status. The metadata include words error and Failure which usually have zero values, but the words get noticed by filters and people eyeballing the logs:

I0405 09:24:26.084143       1 status.go:170] Synchronizing status errs=field.ErrorList(nil) status=&cvo.SyncWorkerStatus{Generation:55, Failure:error(nil), Done:830, Total:830, Completed:107, Reconciling:true, Initial:false, VersionHash:"yroEV7BhWJo=", Architecture:"amd64", LastProgress:time.Date(2023, time.April, 5, 9, 21, 50, 486422587, time.Local), Actual:v1.Release{Version:"4.12.10", Image:"quay.io/openshift-release-dev/ocp-release@sha256:db976910d909373b1136261a5479ed18ec08c93971285ff760ce75c6217d3943", URL:"https://access.redhat.com/errata/RHBA-2023:1508", Channels:[]string(nil)}, Verified:false, loadPayloadStatus:cvo.LoadPayloadStatus{Step:"PayloadLoaded", Message:"Payload loaded version=\"4.12.10\" image=\"quay.io/openshift-release-dev/ocp-release@sha256:db976910d909373b1136261a5479ed18ec08c93971285ff760ce75c6217d3943\" architecture=\"amd64\"", AcceptedRisks:"", Failure:error(nil), Update:v1.Update{Version:"4.12.10", Image:"quay.io/openshift-release-dev/ocp-release@sha256:db976910d909373b1136261a5479ed18ec08c93971285ff760ce75c6217d3943", Force:false}, Verified:false, Local:true, LastTransitionTime:time.Time{wall:0xc102c34e44dd634f, ext:2230452990, loc:(*time.Location)(0x2f4afa0)}}, CapabilitiesStatus:cvo.CapabilityStatus{Status:v1.ClusterVersionCapabilitiesStatus{EnabledCapabilities:[]v1.ClusterVersionCapability{"CSISnapshot", "Console", "Insights", "Storage", "baremetal", "marketplace", "openshift-samples"}, KnownCapabilities:[]v1.ClusterVersionCapability{"CSISnapshot", "Console", "Insights", "Storage", "baremetal", "marketplace", "openshift-samples"}}, ImplicitlyEnabledCaps:[]v1.ClusterVersionCapability(nil)}}

Because most of the time this event is not interesting, we can only log it if we happen to sync something interesting, like a failure, an only log every event on higher verbosity.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2023
pkg/cvo/status.go Outdated Show resolved Hide resolved
Status is synchronized every 15 seconds, and each call is currently
logged with a lot of metadata, including full `status`. The metadata
include words `error` and `Failure` which usually have zero values, but
the words get noticed by filters and people eyeballing the logs.

Because most of the time this event is not interesting, we can only log
it if we happen to sync something interesting, like a failure, an only
log every event on higher verbosity.
@petr-muller petr-muller force-pushed the reduce-verbosity-on-business-as-usual branch from aab0fb2 to 2fd54d1 Compare April 5, 2023 14:27
Copy link
Member

@LalatenduMohanty LalatenduMohanty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 5, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LalatenduMohanty, petr-muller

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [LalatenduMohanty,petr-muller]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@petr-muller
Copy link
Member Author

/test e2e-agnostic-ovn-upgrade-out-of-change

looks like infra hiccup, who wrote that CI? :trollface:

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 73ee5e5 and 2 for PR HEAD 2fd54d1 in total

@petr-muller
Copy link
Member Author

So the failure makes me a little jumpy in context of #920 but we discussed that on Slack and apparently this is a different problem and the fix is the revert in openshift/operator-framework-olm#478

Also, I have verified via search.ci that our failure did not start with #920 but it started earlier.

@petr-muller
Copy link
Member Author

/test e2e-agnostic-ovn-upgrade-into-change

@petr-muller
Copy link
Member Author

/test e2e-agnostic-ovn-upgrade-into-change

Installation failed, apiserver failed to go up

@petr-muller
Copy link
Member Author

/override ci/prow/e2e-agnostic-ovn-upgrade-into-change

Nothing I have seen is related to a logging change ;)

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 6, 2023

@petr-muller: Overrode contexts on behalf of petr-muller: ci/prow/e2e-agnostic-ovn-upgrade-into-change

In response to this:

/override ci/prow/e2e-agnostic-ovn-upgrade-into-change

Nothing I have seen is related to a logging change ;)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 6, 2023

@petr-muller: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit cce97c2 into openshift:master Apr 6, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants