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

getNestedMountpoints may not group mountpoints correctly #112570

Closed
claudiubelu opened this issue Sep 19, 2022 · 1 comment · Fixed by #112571
Closed

getNestedMountpoints may not group mountpoints correctly #112570

claudiubelu opened this issue Sep 19, 2022 · 1 comment · Fixed by #112571
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/windows Categorizes an issue or PR as relevant to SIG Windows. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@claudiubelu
Copy link
Contributor

claudiubelu commented Sep 19, 2022

What happened?

getNestedMountpoints [1] relies on sort.Strings to have all the mount points sorted and grouped. For example, the following strings are sorted in this exact order:

/dir/nested, /dir/nested-vol, /dir/nested.vol, /dir/nested/double, /dir/nested2

With the current implementation, getNestedMountpoints, an incorrect set of nested directories would be returned. For example, if we add the nested-vol and nested.vol volume mounts into the TestGetNestedMountpoints/Unsorted Nested Pod test, we would get:

=== RUN   TestGetNestedMountpoints
    nested_volumes_test.go:232: Unsorted Nested Pod: unexpected nested directories created:
        expected: map[nested:{} nested-vol:{} nested.vol:{} nested2:{}]
             got: map[nested:{} nested-vol:{} nested.vol:{} nested/double:{} nested2:{}]
--- FAIL: TestGetNestedMountpoints (0.00s)

This is worse on Windows, where paths usually have backslashes instead (with no test changes):

=== RUN   TestGetNestedMountpoints
    nested_volumes_test.go:230: Unsorted Nested Pod: unexpected nested directories created:
        expected: map[nested:{} nested2:{}]
             got: map[nested:{} nested2:{} nested\double:{}]
--- FAIL: TestGetNestedMountpoints (0.02s)

[1]

func getNestedMountpoints(name, baseDir string, pod v1.Pod) ([]string, error) {

What did you expect to happen?

getNestedMountpoints should group mount points correctly.

How can we reproduce it (as minimally and precisely as possible)?

git diff pkg/
diff --git a/pkg/volume/util/nested_volumes_test.go b/pkg/volume/util/nested_volumes_test.go
index f4f4add..e73730c 100644
--- a/pkg/volume/util/nested_volumes_test.go
+++ b/pkg/volume/util/nested_volumes_test.go
@@ -89,7 +89,7 @@ func TestGetNestedMountpoints(t *testing.T) {
                {
                        name:     "Unsorted Nested Pod",
                        err:      false,
-                       expected: sets.NewString("nested", "nested2"),
+                       expected: sets.NewString("nested", "nested2", "nested-vol", "nested.vol"),
                        volname:  "vol1",
                        pod: v1.Pod{
                                ObjectMeta: metav1.ObjectMeta{
@@ -106,6 +106,8 @@ func TestGetNestedMountpoints(t *testing.T) {
                                                                {MountPath: "/ignore2", Name: "vol5"},
                                                                {MountPath: "/dir", Name: "vol1"},
                                                                {MountPath: "/dir/nested2", Name: "vol3"},
+                                                               {MountPath: "/dir/nested-vol", Name: "vol6"},
+                                                               {MountPath: "/dir/nested.vol", Name: "vol7"},
                                                        },
                                                },
                                        },

Anything else we need to know?

/kind bug

/sig windows

Kubernetes version

Kubernetes master, Kubernetes v1.25 and older.

Cloud provider

N/A

OS version

N/A

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

@claudiubelu claudiubelu added the kind/bug Categorizes issue or PR as related to a bug. label Sep 19, 2022
@k8s-ci-robot k8s-ci-robot added sig/windows Categorizes an issue or PR as relevant to SIG Windows. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 19, 2022
@marosset
Copy link
Contributor

/triage accepted
/milestone v1.26

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Sep 29, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.26 milestone Sep 29, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/windows Categorizes an issue or PR as relevant to SIG Windows. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Archived in project
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants