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.

Sign up for GitHub

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

Jump to bottom

server.Initialize() returns a nil callback on second call #2281

Closed
dgageot opened this issue Jun 18, 2019 · 1 comment
Closed

server.Initialize() returns a nil callback on second call #2281

dgageot opened this issue Jun 18, 2019 · 1 comment
Labels
area/eventing kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it.

Comments

@dgageot
Copy link
Contributor

dgageot commented Jun 18, 2019 •
edited
Loading

See https://github.com/GoogleContainerTools/skaffold/blob/master/pkg/skaffold/server/server.go#L87

The second time we call Initialize(), for e.g. when a runner is recreated, instead of getting a shutdown callback, we get a nil callback. It'll cause a panic here.

In fact, I think we should initialize/shutdown the gRPC server only once.

ping @nkubala
@dgageot dgageot added area/eventing kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it. labels Jun 18, 2019
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 18, 2019
@dgageot
Initialize event server only once.
46787fb 
And only stop it at the end.
Fix GoogleContainerTools#2281

Signed-off-by: David Gageot <david@gageot.net>
@dgageot dgageot mentioned this issue Jun 18, 2019
Merged
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 18, 2019
@dgageot
Initialize event server only once.
723e18f 
And only stop it at the end.
Fix GoogleContainerTools#2281

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 18, 2019
@dgageot
Initialize event server only once.
f0a595d 
And only stop it at the end.
Fix GoogleContainerTools#2281

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 18, 2019
@dgageot
Initialize event server only once
d89cca8 
And only stop it at the end.
Fix GoogleContainerTools#2281

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2019
@dgageot
Initialize event server only once
6d0d63b 
And only stop it at the end.
Fix GoogleContainerTools#2281

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2019
@dgageot
Initialize event server only once
c12b56d 
And only stop it at the end.
Fix GoogleContainerTools#2281

Signed-off-by: David Gageot <david@gageot.net>
@nkubala
Copy link
Contributor

nkubala commented Jun 19, 2019

I guess we're recreating the runner whenever the skaffold config changes? seems a little weird to me but I guess that's the way we have to handle it. could we memoize the shutdown() callback in the server package and return that once we've started up the first time?

@dgageot dgageot mentioned this issue Jul 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/eventing kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dgageot @nkubala