-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Publish sandbox events #8602
Publish sandbox events #8602
Conversation
Skipping CI for Draft Pull Request. |
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
// Send CONTAINER_DELETED event with ContainerId equal to SandboxId. | ||
c.cri.GenerateAndSendContainerEvent(ctx, sandboxID, sandboxID, runtime.ContainerEventType_CONTAINER_DELETED_EVENT) | ||
|
||
c.sandboxStore.Delete(sandboxID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this delete done now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRI layer is more appropriate place to remove a sandbox from the sandbox store (so it works for all implementations).
We already handle this here:
containerd/pkg/cri/sbserver/sandbox_remove.go
Line 102 in c7b9a95
c.sandboxStore.Delete(id) |
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
podsandbox/
package, so sbserver should be less aware about pod sandbox containers./cc: @egernst