-
Notifications
You must be signed in to change notification settings - Fork 298
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
feat: make Kubernetes informers cancellable #9411
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
@@ -364,7 +366,7 @@ export class ContextsManager { | |||
}; | |||
} | |||
|
|||
private createPodInformer(kc: KubeConfig, namespace: string, context: KubeContext): Informer<V1Pod> { | |||
private createPodInformer(kc: KubeConfig, namespace: string, context: KubeContext): CancellableInformer { |
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.
Why are we losing the generic type, is it important ?
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'll check again, does it does not seem to be useful
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 don't think so:
The V1Pod
is necessary when calling this.createInformer<V1Pod>
, to have he onAdd
and other options have the matching type. But the result of createPodInformer
is stored in an Informer<KubernetesObject>
; the specific type is never used on this side.
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.
Okey thanks
Signed-off-by: Philippe Martin <[email protected]>
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 Codewise
Signed-off-by: Philippe Martin [email protected]
What does this PR do?
For #6114 , we will need to restart the informers every time we change the current context, to change their backoff configuration.
Working on this, I realized that the informers were not correctly stopped when not necessary anymore.
This PR makes the informers cancellable, so we can make them stop their infinite loop of retries.
What issues does this PR fix or reference?
Part of #6114
How to test this PR?