-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Make logs format more consistent by using server logger for client-go, controller-runtime, requiring upgrade of k8s.io/ to v0.31.3 #8450
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
Conversation
b0e0b77 to
c230a3c
Compare
c230a3c to
74ec953
Compare
74ec953 to
2e2cdeb
Compare
|
fyi I need to fix go sum here before merging :D |
6757e8b to
cf4add2
Compare
1aeedba to
ac86ced
Compare
ac86ced to
b210870
Compare
kaovilai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time="2024-11-25T20:27:46Z" level=fatal msg="controller with name backup already exists. Controller names must be unique to avoid multiple controllers reporting to the same metricunable to create controllercontrollerbackup-operations" logSource="pkg/cmd/server/server.go:661"
f518555 to
17c2721
Compare
| return nil, errors.Wrap(err, "error to create client config") | ||
| } | ||
|
|
||
| ctrl.SetLogger(zap.New(zap.UseDevMode(true))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI zap is being removed in this PR.
It was meant to be removed here https://github.com/vmware-tanzu/velero/pull/2561/files#r432107463 and was kept here #8046 (comment)
This means all logs are going to be affected by the default logger flags, that means if user want all logs in json, they will get all logs in json.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8450 +/- ##
==========================================
- Coverage 59.51% 59.49% -0.02%
==========================================
Files 371 371
Lines 40179 40196 +17
==========================================
+ Hits 23911 23914 +3
- Misses 14775 14789 +14
Partials 1493 1493 ☔ View full report in Codecov by Sentry. |
ee7ceda to
8929b68
Compare
This comment was marked as outdated.
This comment was marked as outdated.
8929b68 to
3bad684
Compare
3bad684 to
f8ff11f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be better to break this PR in 2? one for dependencies update and one for logger change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can save logger change for later but logger change depends on dep bump
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaovilai @mateusoliveira43 If the logger change requires the dep bump, then it makes more sense to keep them together. Otherwise, we wouldn't necessarily need the dep bump (assuming we're not also fixing a CVE at the same time).
e6b85b0 to
f5db89f
Compare
|
reopening to clear 1.32.1 tests which are hitting #8648 |
|
@kaovilai please resolve the conflict |
… client-go Also bumped to support upgraded k8s.io/ deps. - controller-gen to v0.16.5 - sigs.k8s.io/controller-runtime v0.19.2 Signed-off-by: Tiger Kaovilai <[email protected]>
f5db89f to
7a3bca0
Compare
|
Resolved! |
reasonerjt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
… client-go (vmware-tanzu#8450) Also bumped to support upgraded k8s.io/ deps. - controller-gen to v0.16.5 - sigs.k8s.io/controller-runtime v0.19.2 Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai [email protected]
Thank you for contributing to Velero!
Please add a summary of your change
Upgrade k8s.io/ go.mod to v0.31.3 (minor version used by https://github.com/kubernetes-sigs/controller-tools/blob/v0.16.5/go.mod#L18) and set logger for client-go to use the same logger as the rest of the codebase for consistent logging.
Upgraded controller-gen to version that support the new deps.
Also bumped to support upgraded k8s.io/ deps.
Does your change fix a particular issue?
Fixes #3550 #8442
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.