Fixes a race in deviceplugin/manager_test.go and a race in deviceplug…

[jiayingz]

…in/manager.go.

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
#52560

Special notes for your reviewer:
Tested with go test -count 50 -race k8s.io/kubernetes/pkg/kubelet/deviceplugin and all runs passed.

Release note:

[jiayingz]

/release-note-none

[jiayingz]

/assign @mindprince @dchen1107

[jiayingz]

/assign @RenaudWasTaken

[k8s-ci-robot]

@jiayingz: GitHub didn't allow me to assign the following users: RenaudWasTaken.

Note that only kubernetes members can be assigned.

In response to this:

/assign @RenaudWasTaken

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jiayingz]

/retest

[dchen1107]

/approve

[RenaudWasTaken]

We are still leaking a goroutine in device_plugin_stub.go by not closing the close channel during the Stop implementation

[dims]

@jiayingz @RenaudWasTaken Do we need this for 1.8?

[jiayingz]

/assign @vishh

[vishh]

/lgtm
/approve

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, jiayingz, vishh

Associated issue: 52560

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• pkg/kubelet/OWNERS [dchen1107,vishh]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[dims]

/test all

[jiayingz]

/retest

[dims]

/retest

[RenaudWasTaken]

This whole loop is useless. m.server.GetServiceInfo() returns the number of services as soon as RegisterDevicePluginServer is called.

[jiayingz]

I think it returns the number of services when they are ready, not as soon as RegisterDevicePluginServer is called.

[RenaudWasTaken]

https://github.com/grpc/grpc-go/blob/master/server.go#L376

You can also test this:

m.server = grpc.NewServer([]grpc.ServerOption{}...)
pluginapi.RegisterDevicePluginServer(m.server, m)
fmt.Println(m.server.GetServiceInfo())

[jiayingz]

Hmm, this is interesting to know. I thought GetServiceInfo returns ready-to-serve no. of methods, but looking at the code, it is indeed should return No. of methods right after registration. However, I did verify that if I removed this code block, running 'bazel test --runs_per_test=20 //pkg/kubelet/deviceplugin:go_default_test' became flaky (I think mostly on endpoint_test.go) but with the current code, the tests all passed. I think we will need to spend more time understand this code. For now, I would like to keep it this way to make sure the tests are not flaky.

[jiayingz]

/retest

[dims]

/retest

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here..