Skip to content
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

fix nri removeMount does not work bug #87

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

liangjingtao11
Copy link

nri needs to pass the del mount info to containerd, so that containerd can handle the mount records that need to be deleted correctly

fix: #80

nri needs to pass the del mount info to containerd,
so that containerd can handle the mount records that
need to be deleted correctly

Signed-off-by: jingtao.liang <jingtao.liang@easystack.cn>
@liangjingtao11
Copy link
Author

/retest

@mikebrow
Copy link
Member

@mikebrow
Copy link
Member

mikebrow commented May 29, 2024

Additional consideration.. the removeMount test, linked below, only does the removeMount when the adjustment is set to "overwrite"= true..

If we are to support removal of a mount that is not an overwrite we should probably have another test for it.. https://github.com/containerd/nri/blob/main/pkg/adaptation/adaptation_suite_test.go#L453-L456

fix nri removeMount does not work bug
@@ -322,6 +322,11 @@ func (r *result) adjustMounts(mounts []*Mount, plugin string) error {
r.reply.adjust.Mounts = append(r.reply.adjust.Mounts, m)
}

// need add del mount, containerd needs to process this del mounts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I would change 'containerd' in the above comment to 'runtime'. Maybe rephrase to something like ' add deleted mounts, the runtime needs them to properly remove deleted mounts'.

@klihub
Copy link
Member

klihub commented Jun 7, 2024

@liangjingtao11 It would be nice to have a test case for this.

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.

RemoveMount does not work
5 participants