-
Notifications
You must be signed in to change notification settings - Fork 300
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: simplify context states update #9443
base: main
Are you sure you want to change the base?
Conversation
unit tests and type check is failing maybe a draft pr ? |
8746886
to
3b9dd33
Compare
this.startResourceInformer(this.currentContext.name, resourceName); | ||
// start secondary informers only if current context is reachable | ||
const currentContextState = this.states.getCurrentContextGeneralState(this.currentContext.name); | ||
if (currentContextState.reachable) { |
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'm having trouble following the async states - how do we know the reachable state of the current context will be valid / updated when this function is called, and isn't that basically the same as the call to .isReachable() a few lines up?
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, you are right this is the same, I'll change to simplify
} | ||
|
||
// Add informers for new contexts (only current context if we are checking only it) | ||
const added: string[] = []; | ||
// Add primary informers for contexts (only current context if we are checking only it) |
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.
Double use of 'only' is confusing to read here
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
Signed-off-by: Philippe Martin <[email protected]>
466fbe0
to
4c06761
Compare
Signed-off-by: Philippe Martin [email protected]
What does this PR do?
Working on #6114, informers need to be restarted in more circumstances, to be able to change their backoff configuration.
After a few iterations working on these informers, the logic to detect which informers need to be started/stopped is more and more complex, and there are still missing conditions.
With this PR, I propose to simplify the logic, with:
when the kubeconfig file changes:
when the current context becomes reachable:
when the current context becomes unreachable:
when the frontend subscribes to the secondary informers:
What issues does this PR fix or reference?
Part of #6114
How to test this PR?