Skip to content

Conversation

@liuyuan10
Copy link
Contributor

Today it only logs and stop the rpc server. The plugin hangs after that.

@coveralls
Copy link
Collaborator

coveralls commented Sep 24, 2024

Pull Request Test Coverage Report for Build 14307870838

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 74.63%

Totals Coverage Status
Change from base Build 14237300611: 0.0%
Covered Lines: 2118
Relevant Lines: 2838

💛 - Coveralls

Today it only logs and stop the rpc server. The plugin hangs after that.

Signed-off-by: Yuan Liu <[email protected]>
@liuyuan10
Copy link
Contributor Author

@adrianchiris I wonder if you can take a quick look of this PR? it should be a small fix

Copy link
Collaborator

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

LGTM

@SchSeba SchSeba closed this Apr 7, 2025
@SchSeba SchSeba reopened this Apr 7, 2025
@SchSeba
Copy link
Collaborator

SchSeba commented Apr 7, 2025

I re-open to trigger the CI system again

@adrianchiris when you have time please take a look on this one

} else {
glog.Infof("Plugin: %s failed to be registered at Kubelet: %v; restarting.\n", rs.endPoint, regstat.Error)
rs.grpcServer.Stop()
glog.Fatalf("Plugin: %s failed to be registered at Kubelet: %v; restarting.\n", rs.endPoint, regstat.Error)
Copy link
Contributor

Choose a reason for hiding this comment

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

im not sure im OK with logging a msg then panic,

maybe we need some way to signal errors and exit gracefully ?
what others think of this ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sure we can switch to log.err and exit 1 or something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isn't log.Fatal the same as log and exit 1?

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.

4 participants