-
Notifications
You must be signed in to change notification settings - Fork 83
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
Use informer for startupTranslator #1075
Conversation
/test integration-tests passed once. |
Codecov Report
@@ Coverage Diff @@
## main #1075 +/- ##
=======================================
Coverage 80.95% 80.95%
=======================================
Files 18 18
Lines 1355 1355
=======================================
Hits 1097 1097
Misses 205 205
Partials 53 53 |
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.
Other than that:
/lgtm
/hold for the one comment. Feel free to unhold if this is ok.
startupTranslator := generator.NewIngressTranslator( | ||
func(ns, name string) (*corev1.Secret, error) { | ||
return kubernetesClient.CoreV1().Secrets(ns).Get(ctx, name, metav1.GetOptions{}) | ||
return secretInformer.Lister().Secrets(ns).Get(name) |
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.
Just to make sure, we are now using the filtered secretInformer instead of the unfiltered kubeclient. It seems that Kourier can have just the secret filtered here: https://github.com/knative-sandbox/net-kourier/blob/main/pkg/reconciler/informerfiltering/util.go#L44. So this should be fine, right?
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.
Yes, I think no problem.
Secrets for internal-encryption has networking.internal.knative.dev/certificate-uid
label in serving repo and Secrets for DomainMapping BYO should be added networking.internal.knative.dev/certificate-uid
by users so we just need to take care of the filtered secrets when users enabled the filtering.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nak3, ReToCode 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:
Approvers can indicate their approval by writing |
This patch uses informer for
startupTranslator
instead of k8s client.This is alternative of #1066