-
Notifications
You must be signed in to change notification settings - Fork 69
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
Update dependencies #481
Update dependencies #481
Conversation
- golang.org/x/net to v0.19.0 - k8s.io/klog/v2 to v2.110.1 - k8s.io/utils to v0.0.0-20231127182322-b307cd553661 Signed-off-by: Hidde Beydals <[email protected]>
golang.org/x/text v0.14.0 // indirect | ||
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect | ||
gopkg.in/inf.v0 v0.9.1 // indirect | ||
gopkg.in/yaml.v2 v2.4.0 // indirect | ||
k8s.io/klog/v2 v2.100.1 // indirect | ||
k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect | ||
k8s.io/klog/v2 v2.110.1 // indirect |
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.
This is not Ok, 110 breaks backwards compatibly
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.
Hu, I couldn't find an obvious commit in kubernetes/klog@v2.100.1...v2.110.1 that would do that.
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.
It appears to not break backwards compatibility when updated in conjunction with logr to v1.3.0. As my helm-controller build (which contains this bump) is running just fine.
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.
We could force controller-runtime to the new version if we bump logr also in pkg/runtime, the version we are using is on klog 100 and logr 1.2 https://github.com/kubernetes-sigs/controller-runtime/blob/v0.16.3/go.mod
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 look into this in combination with a weird issue I observed earlier when log-level
is set to trace
:
{"level":"Level(-2)","ts":"2023-12-11T10:45:58.333Z","msg":"getting last revision of \"podinfo\"","controller":"helmrelease","controllerGroup":"helm.toolkit.fluxcd.io","controllerKind":"HelmRelease","HelmRelease":{"name":"podinfo","namespace":"podinfo"},"namespace":"podinfo","name":"podinfo","reconcileID":"daa23d2f-7b8a-4428-a61e-a816fe7d2846"}
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.
Can someone please clarify on the backwards-incompatibility we're talking about here? The code compiles and tests fine so is it about the actual output it produces that's different with the new version?
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.
"Backwards compatibility" being that klog v2.110 is only compatible with logr >=v1.3.0
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.
The backwards-incompatibility is about klog - logr, controller-runtime comes with logr 1.2 which doesn't work with klog 110. As I said above, we could force logr in pkg/runtime.
- github.com/fluxcd/pkg/oci to v0.33.3 - github.com/fluxcd/pkg/runtime to v0.43.2 Signed-off-by: Hidde Beydals <[email protected]>
Releated to #479