-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
fixes the way the informers are started in sample controller pkg #69636
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,8 +65,10 @@ func main() { | |
kubeInformerFactory.Apps().V1().Deployments(), | ||
exampleInformerFactory.Samplecontroller().V1alpha1().Foos()) | ||
|
||
go kubeInformerFactory.Start(stopCh) | ||
go exampleInformerFactory.Start(stopCh) | ||
// notice that there is no need to run Start methods in a separate goroutine. (i.e. go kubeInformerFactory.Start(stopCh) | ||
// Start method is non-blocking and runs all registered informers in a dedicated goroutine. | ||
kubeInformerFactory.Start(stopCh) | ||
exampleInformerFactory.Start(stopCh) | ||
|
||
if err = controller.Run(2, stopCh); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be run as a go routine though? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is okay, the main func will stop at this method as it essentially runs the controller and blocks until the stopCh is closed. |
||
glog.Fatalf("Error running controller: %s", err.Error()) | ||
|
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.
👍
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 should add comments here explaining this doesnt need to go in a go routine for so and so reason, more so because lot of first timers come here to copy code
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.
sgtm