-
Notifications
You must be signed in to change notification settings - Fork 32
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
refactor: controller-runtime v0.16.3 #444
Conversation
HealthProbeBindAddress: opts.healthProbeAddr, | ||
LeaderElection: false, | ||
} | ||
if opts.watchNamespace != "" { | ||
baseManagerOptions.Namespace = opts.watchNamespace | ||
baseManagerOptions.Cache.DefaultNamespaces = map[string]cache.Config{opts.watchNamespace: {}} |
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.
Interesting. So now we can supporting watching selected namespaces (plural)?
We need to test this with Authorino deployed in both modes (i.e. cluster-wide and namespaced.)
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.
Interesting. So now we can supporting watching selected namespaces (plural)?
Yeah, it seems the BaseOptions has moved the Namespace
option to Cache.Options.DefaultNamespaces
🤔
By default it will watch from all namespaces unless defined in this slice
So it seems possible to support multi namespace via this if we so choose at some point 🤔
We need to test this with Authorino deployed in both modes (i.e. cluster-wide and namespaced.
Agreed 👍
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.
return nil, err | ||
} | ||
if options.Metrics.BindAddress != "0" { | ||
options.Metrics.ExtraHandlers = map[string]http.Handler{"/server-metrics": promhttp.Handler()} |
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.
Defining extra metric handlers looks to have moved to this option instead:
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've tested a few scenarios and everything looks good. e2e tests also green.
Nice work, @KevFan!
Thanks @guicassolato for testing it out ! 👍 |
With Authorino now using Go 1.20 (Kuadrant/authorino-operator#156), we should bump controller runtime to v0.16.3 as Kuadrant Operator has also moved to this version for consistency (Kuadrant/kuadrant-operator#246)