Skip to content

fix potential hang on containerd client.LoadContainer#3672

Merged
cwangVT merged 1 commit intogoogle:masterfrom
lianghao208:client-timeout
Mar 14, 2025
Merged

fix potential hang on containerd client.LoadContainer#3672
cwangVT merged 1 commit intogoogle:masterfrom
lianghao208:client-timeout

Conversation

@lianghao208
Copy link
Contributor

fix: #3671

add timeout context for containerd factory during starting process.

Signed-off-by: lianghao208 <roylizard3@gmail.com>
@lianghao208
Copy link
Contributor Author

@dims @SuperQ PTAL, thanks:)

@lianghao208
Copy link
Contributor Author

kindly ping @cwangVT , might need your approval for pending workflow, thanks

@lianghao208
Copy link
Contributor Author

@bobbypage can you review this? thanks:)

@lianghao208
Copy link
Contributor Author

@dims @bobbypage @cwangVT @SuperQ Can this PR get merged by any chance? It's had a pretty big impact on kubelet. If cadvisor hangs on LoadClient/createContainerLocked, it helds the containersLock lock indefinitely, which leads to dead lock and serious goroutine leak. Much appreciated for the review:)

// Create a container.
func (m *manager) createContainer(containerName string, watchSource watcher.ContainerWatchSource) error {
	m.containersLock.Lock()
	defer m.containersLock.Unlock()

	return m.createContainerLocked(containerName, watchSource)
}

@lianghao208
Copy link
Contributor Author

Could relate to this: kubernetes/kubernetes#102481

@SuperQ
Copy link
Contributor

SuperQ commented Mar 14, 2025

Sorry, I have no authority here. I am not a Google employee.

@cwangVT cwangVT self-requested a review March 14, 2025 15:52
@lianghao208
Copy link
Contributor Author

@cwangVT thanks for the approval. Can you skip the pull-cadvisor-e2e test by updating the list of required checks? It seems this check is pending and can't merge.

@cwangVT cwangVT merged commit f6e31a3 into google:master Mar 14, 2025
7 checks passed
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.

Potential hang on containerd client.LoadContainer

3 participants