Skip to content
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

main.go: Remove sleep during shutdown #777

Merged
merged 1 commit into from
Jul 21, 2017

Conversation

tomdee
Copy link
Contributor

@tomdee tomdee commented Jul 14, 2017

No description provided.

@tomdee tomdee requested a review from gunjan5 July 14, 2017 22:11
Copy link
Contributor

@gunjan5 gunjan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question about passing wg as reference

main.go Outdated
@@ -44,6 +44,8 @@ import (
"github.com/joho/godotenv"

// Backends need to be imported for their init() to get executed and them to register
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment should be moved to where the imports are

main.go Outdated
// Block waiting to renew the lease
_ = MonitorLease(ctx, sm, bn)
if !opts.kubeSubnetMgr {
err = MonitorLease(ctx, sm, bn, wg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't you be passing &wg to MonitorLease if you're passing the reference?

main.go Outdated
@@ -339,10 +360,16 @@ func getConfig(ctx context.Context, sm subnet.Manager) (*subnet.Config, error) {
}
}

func MonitorLease(ctx context.Context, sm subnet.Manager, bn backend.Network) error {
func MonitorLease(ctx context.Context, sm subnet.Manager, bn backend.Network, wg sync.WaitGroup) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, I think it should be wg *sync.WaitGroup

@tomdee
Copy link
Contributor Author

tomdee commented Jul 21, 2017

I amended the commits with the markups (since they were small). PTAL @gunjan5

Copy link
Contributor

@gunjan5 gunjan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tomdee tomdee merged commit 7ffbf9d into flannel-io:master Jul 21, 2017
@tomdee tomdee deleted the remove-shutdown-sleep branch July 21, 2017 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants