Commit Graph

14 Commits

Author SHA1 Message Date
Kubernetes Submit Queue
a4eb2f96d0
Merge pull request #57610 from vikaschoudhary16/remove-redundant-sleep
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Remove redundant sleep from ReRegistration unit test case

/kind cleanup
/sig node

**What this PR does / why we need it**:
Once upon a time, there was a race in the device plugin registration logic.  At that time, [list()](5cac9fc984/pkg/kubelet/deviceplugin/manager.go (L206)) and [listAndWatch()](5cac9fc984/pkg/kubelet/deviceplugin/manager.go (L224)) used to be separate functions. Race was there for taking manager.mutex lock from two places. [One, from within the m.addEndpoint()](5cac9fc984/pkg/kubelet/deviceplugin/manager.go (L214)) and the [second, from within m.Devices()](5cac9fc984/pkg/kubelet/deviceplugin/manager.go (L137)).  This race was making `TestDevicePluginReRegistration` flaky as explained below.
 	
```
1.     p1.Register(socketName, testResourceName)
2.  	// Wait for the first callback to be issued.
3.  	<-callbackChan
4.        devices := m.Devices()  
```
* L#1 leads to eventually **asynchronous** invocation of m.addEndpoint(), let say **thread1**.
* L#3 holds the test case execution till the [callback gets invoked](5cac9fc984/pkg/kubelet/deviceplugin/endpoint.go (L108)). This means test case execution waits on channel till the **thread1**  reaches the point where [e.list() call completes in the addEndpoint.](5cac9fc984/pkg/kubelet/deviceplugin/manager.go (L206)) 
* L#4 triggers a new thread. thread1 and this new thread are both racing for m.mutex.Lock(). Former, in the addEndpoint() and later one in the m.Devices(). If m.Devices wins the race, result is the test case failure because endpoint gets added in the manager only after taking mutex.Lock() in the addEndpoint().

To deal with this flake, we added `Sleep` between L#3 and L#4.  `Sleep` was getting some extra time to addEndpoint() and thus making thread1 win the race each time.

Above explained race scenario got fixed and merged sometime back in this PR:
[Deviceplugin refactoring: merge func list and listwatch in endpoint into one](https://github.com/kubernetes/kubernetes/pull/52149)
With the above PR, callback function is invoked from e.run() which makes sure that test case waits on channel till the endpoint is added and devices are updated
Above explained race scenario does not exist now, therefore removing redundant sleeps from the test case.

Tested:
go test -race -count 500 k8s.io/kubernetes/pkg/kubelet/cm/deviceplugin -run TestDevicePluginReRegistration  -timeout 5h

Related #52616 #56026 

**Special notes for your reviewer**:

**Release note**:

```release-note
None
```
/cc @vishh @derekwaynecarr @jiayingz @RenaudWasTaken @lichuqiang @ScorpioCPH @tengqm @mindprince @ConnorDoyle @jeremyeder
2017-12-27 14:53:21 -08:00
vikaschoudhary16
5d10dcd983 Remove redundant sleep from ReRegistration unit test case 2017-12-27 03:02:21 -05:00
vikaschoudhary16
8749c5c989 Revert back #57278 2017-12-22 18:55:53 -05:00
vikaschoudhary16
bf1fb46347 Look for requested resources in the Requests 2017-12-17 22:56:45 -05:00
vikaschoudhary16
8c51d235d6 Refactor TestPodContainerDeviceAllocation to make it readable and extensible 2017-12-16 20:32:08 -05:00
Kubernetes Submit Queue
a902959544
Merge pull request #56911 from WanLinghao/projected_test_fix
Automatic merge from submit-queue (batch tested with PRs 56650, 55813, 56911, 56921, 56871). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix deviceplugin test file create leak file problem

When execute make test, this test file will create a file named "kubelet_internal_checkpoint" in k8s directory and not delete it.

This patch fix this error




**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #56365

**Special notes for your reviewer**:

**Release note**:

```release-note

```
2017-12-16 12:10:48 -08:00
Kubernetes Submit Queue
578f3db8d5
Merge pull request #55382 from vikaschoudhary16/checkpoint
Automatic merge from submit-queue (batch tested with PRs 57172, 55382, 56147, 56146, 56158). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use file store utility for device plugin checkpointing

Partially address issue #54088
cc @sjenning @jeremyeder @jiayingz @vishh 

/sig node
2017-12-14 12:38:13 -08:00
WanLinghao
3e7e4ab397 old test file will create a leak file in current directory.
this patch fix this.
	modified:   pkg/kubelet/cm/deviceplugin/manager_test.go
2017-12-07 11:57:17 +08:00
Jiaying Zhang
d4244f3ded Re-uses device plugin resources allocated to init containers.
Implements option 2 mentioned in
https://github.com/kubernetes/kubernetes/issues/56022#issuecomment-348286184
2017-12-04 22:01:28 -08:00
vikaschoudhary16
de358fb21f Use file store utility for device plugin check-pointing 2017-11-24 08:41:11 -05:00
Jiaying Zhang
1eb4e79453 Extends deviceplugin to gracefully handle full device plugin lifecycle.
- Instead of using cm.capacity field to communicate device plugin resource
capacity, this PR changes to use an explicit cm.GetDevicePluginResourceCapacity()
function that returns device plugin resource capacity as well as any inactive
device plugin resource. Kubelet syncNodeStatus call this function during its
periodic run to update node status capacity and allocatable. After this call,
device plugin can remove the inactive device plugin resource from its allDevices
field as the update is already pushed to API server.
- Extends device plugin checkpoint data to record registered resources
so that we can finish resource removing even upon kubelet restarts.
- Passes sourcesReady from kubelet to device plugin to avoid removing
inactive pods during grace period of kubelet restart.
2017-11-20 23:40:14 -08:00
Niklas Q. Nielsen
b16bfc768d Merging handler into manager API 2017-11-20 21:37:46 +00:00
lichuqiang
4fa0fa5ad1 pass devices of previous endpoint into re-registered one to avoid potential orphaned devices upon re-registration 2017-11-14 16:43:19 +08:00
Jiaying Zhang
796f488789 Move device plugin related files under pkg/kubelet/cm/deviceplugin/. 2017-10-24 14:17:20 -07:00