Skip to content

Cherrypick #1971, #1969 #1964, #1963 to release v0.30 #1972

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

Merged
merged 4 commits into from
Jun 21, 2018

Conversation

dashpole
Copy link
Collaborator

No description provided.

MaximilianMeister and others added 4 commits June 20, 2018 16:28
Earlier if the NVIDIA driver was not installed when cAdvisor was started
we would start a goroutine to try to initialize NVML every minute.

This resulted in a race. We can have a situation where:
- goroutine tries to initialize NVML but fails. So, it sleeps for a minute.
- the driver is installed.
- a container that uses NVIDIA devices is started.
This container would not get GPU stats because a minute has not passed
since the last failed initialization attempt and so NVML is not
initialized.
GetSpec() can be called concurrently in
manager/container.go.updateSpec()
results into a concurrent map access on the labels map because we're
directly updating the map inside GetSpec(). The labels map from the
container handler is not a copy of the map itself, just a reference,
that's why we're getting the concurrent map access.
Fix this by moving the label update with restartcount to the handler's
initialization method which is not called concurrently.

Signed-off-by: Antonio Murdaca <[email protected]>
@googlebot
Copy link
Collaborator

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@dashpole
Copy link
Collaborator Author

@rohitagarwal003
Copy link
Contributor

LGTM

@MaximilianMeister
Copy link
Contributor

@dashpole thanks! should I do the vendor update in k8s once this is released on v0.30 ? or will you take care about it anyways?

@dashpole
Copy link
Collaborator Author

I will do all of the godep updates in k/k

@dashpole dashpole merged commit 50ae2ef into google:release-v0.30 Jun 21, 2018
@dashpole dashpole deleted the cherrypick_things branch June 21, 2018 16:24
@MaximilianMeister
Copy link
Contributor

I will do all of the godep updates in k/k

thanks @dashpole 👍

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.

5 participants