Skip to content

Commit

Permalink
[pkg/volume] Changed to use sets.Set[string] instead of sets.String
Browse files Browse the repository at this point in the history
  • Loading branch information
bells17 committed May 11, 2024
1 parent 925cb2b commit 14bd183
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 64 deletions.
10 changes: 10 additions & 0 deletions kind-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# necessary for conformance
kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
networking:
ipFamily: ipv4
nodes:
# the control plane node
- role: control-plane
- role: worker
- role: worker
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func (dswp *desiredStateOfWorldPopulator) deleteProcessedPod(
// specified volume. It dereference any PVC to get PV objects, if needed.
// Returns an error if unable to obtain the volume at this time.
func (dswp *desiredStateOfWorldPopulator) createVolumeSpec(
podVolume v1.Volume, pod *v1.Pod, mounts, devices sets.String) (*v1.PersistentVolumeClaim, *volume.Spec, string, error) {
podVolume v1.Volume, pod *v1.Pod, mounts, devices sets.Set[string]) (*v1.PersistentVolumeClaim, *volume.Spec, string, error) {
pvcSource := podVolume.VolumeSource.PersistentVolumeClaim
isEphemeral := pvcSource == nil && podVolume.VolumeSource.Ephemeral != nil
if isEphemeral {
Expand Down
16 changes: 8 additions & 8 deletions pkg/volume/csi/nodeinfomanager/nodeinfomanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,16 +474,16 @@ func setMigrationAnnotation(migratedPlugins map[string](func() bool), nodeInfo *
nodeInfoAnnotations = map[string]string{}
}

var oldAnnotationSet sets.String
var oldAnnotationSet sets.Set[string]
mpa := nodeInfoAnnotations[v1.MigratedPluginsAnnotationKey]
tok := strings.Split(mpa, ",")
if len(mpa) == 0 {
oldAnnotationSet = sets.NewString()
oldAnnotationSet = sets.New[string]()
} else {
oldAnnotationSet = sets.NewString(tok...)
oldAnnotationSet = sets.New[string](tok...)
}

newAnnotationSet := sets.NewString()
newAnnotationSet := sets.New[string]()
for pluginName, migratedFunc := range migratedPlugins {
if migratedFunc() {
newAnnotationSet.Insert(pluginName)
Expand All @@ -494,7 +494,7 @@ func setMigrationAnnotation(migratedPlugins map[string](func() bool), nodeInfo *
return false
}

nas := strings.Join(newAnnotationSet.List(), ",")
nas := strings.Join(sets.List[string](newAnnotationSet), ",")
if len(nas) != 0 {
nodeInfoAnnotations[v1.MigratedPluginsAnnotationKey] = nas
} else {
Expand Down Expand Up @@ -526,15 +526,15 @@ func (nim *nodeInfoManager) installDriverToCSINode(
return fmt.Errorf("error getting CSI client")
}

topologyKeys := sets.StringKeySet(topology)
topologyKeys := sets.KeySet[string, string](topology)

specModified := true
// Clone driver list, omitting the driver that matches the given driverName
newDriverSpecs := []storagev1.CSINodeDriver{}
for _, driverInfoSpec := range nodeInfo.Spec.Drivers {
if driverInfoSpec.Name == driverName {
if driverInfoSpec.NodeID == driverNodeID &&
sets.NewString(driverInfoSpec.TopologyKeys...).Equal(topologyKeys) &&
sets.New[string](driverInfoSpec.TopologyKeys...).Equal(topologyKeys) &&
keepAllocatableCount(driverInfoSpec, maxAttachLimit) {
specModified = false
}
Expand All @@ -554,7 +554,7 @@ func (nim *nodeInfoManager) installDriverToCSINode(
driverSpec := storagev1.CSINodeDriver{
Name: driverName,
NodeID: driverNodeID,
TopologyKeys: topologyKeys.List(),
TopologyKeys: sets.List[string](topologyKeys),
}

if maxAttachLimit > 0 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ type VolumePluginMgr struct {
plugins map[string]VolumePlugin
prober DynamicPluginProber
probedPlugins map[string]VolumePlugin
loggedDeprecationWarnings sets.String
loggedDeprecationWarnings sets.Set[string]
Host VolumeHost
}

Expand Down Expand Up @@ -567,7 +567,7 @@ func (pm *VolumePluginMgr) InitPlugins(plugins []VolumePlugin, prober DynamicPlu
defer pm.mutex.Unlock()

pm.Host = host
pm.loggedDeprecationWarnings = sets.NewString()
pm.loggedDeprecationWarnings = sets.New[string]()

if prober == nil {
// Use a dummy prober to prevent nil deference.
Expand Down
6 changes: 3 additions & 3 deletions pkg/volume/testing/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (plugin *FakeVolumePlugin) getFakeVolume(list *[]*FakeVolume) *FakeVolume {
WaitForAttachHook: plugin.WaitForAttachHook,
UnmountDeviceHook: plugin.UnmountDeviceHook,
}
volume.VolumesAttached = make(map[string]sets.String)
volume.VolumesAttached = make(map[string]sets.Set[string])
volume.DeviceMountState = make(map[string]string)
volume.VolumeMountState = make(map[string]string)
if list != nil {
Expand Down Expand Up @@ -679,7 +679,7 @@ type FakeVolume struct {
VolName string
Plugin *FakeVolumePlugin
volume.MetricsNil
VolumesAttached map[string]sets.String
VolumesAttached map[string]sets.Set[string]
DeviceMountState map[string]string
VolumeMountState map[string]string

Expand Down Expand Up @@ -1021,7 +1021,7 @@ func (fv *FakeVolume) Attach(spec *volume.Spec, nodeName types.NodeName) (string
return "", fmt.Errorf("volume %q trying to attach to node %q is already attached to node %q", volumeName, nodeName, volumeNodes)
}

fv.VolumesAttached[volumeName] = sets.NewString(string(nodeName))
fv.VolumesAttached[volumeName] = sets.New[string](string(nodeName))
if nodeName == UncertainAttachNode || nodeName == TimeoutAttachNode {
return "", fmt.Errorf("timed out to attach volume %q to node %q", volumeName, nodeName)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/testing/volume_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ func enableMigrationOnNode(csiNode *storagev1.CSINode, pluginName string) {
nodeInfoAnnotations = map[string]string{}
}

newAnnotationSet := sets.NewString()
newAnnotationSet := sets.New[string]()
newAnnotationSet.Insert(pluginName)
nas := strings.Join(newAnnotationSet.List(), ",")
nas := strings.Join(sets.List(newAnnotationSet), ",")
nodeInfoAnnotations[v1.MigratedPluginsAnnotationKey] = nas

csiNode.Annotations = nodeInfoAnnotations
Expand Down
14 changes: 7 additions & 7 deletions pkg/volume/util/atomic_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (w *AtomicWriter) Write(payload map[string]FileProjection, setPerms func(su
}
oldTsPath := filepath.Join(w.targetDir, oldTsDir)

var pathsToRemove sets.String
var pathsToRemove sets.Set[string]
shouldWrite := true
// if there was no old version, there's nothing to remove
if len(oldTsDir) != 0 {
Expand Down Expand Up @@ -355,8 +355,8 @@ func shouldWriteFile(path string, content []byte) (bool, error) {
// pathsToRemove walks the current version of the data directory and
// determines which paths should be removed (if any) after the payload is
// written to the target directory.
func (w *AtomicWriter) pathsToRemove(payload map[string]FileProjection, oldTsDir string) (sets.String, error) {
paths := sets.NewString()
func (w *AtomicWriter) pathsToRemove(payload map[string]FileProjection, oldTsDir string) (sets.Set[string], error) {
paths := sets.New[string]()
visitor := func(path string, info os.FileInfo, err error) error {
relativePath := strings.TrimPrefix(path, oldTsDir)
relativePath = strings.TrimPrefix(relativePath, string(os.PathSeparator))
Expand All @@ -374,9 +374,9 @@ func (w *AtomicWriter) pathsToRemove(payload map[string]FileProjection, oldTsDir
} else if err != nil {
return nil, err
}
klog.V(5).Infof("%s: current paths: %+v", w.targetDir, paths.List())
klog.V(5).Infof("%s: current paths: %+v", w.targetDir, sets.List(paths))

newPaths := sets.NewString()
newPaths := sets.New[string]()
for file := range payload {
// add all subpaths for the payload to the set of new paths
// to avoid attempting to remove non-empty dirs
Expand All @@ -386,7 +386,7 @@ func (w *AtomicWriter) pathsToRemove(payload map[string]FileProjection, oldTsDir
subPath = strings.TrimSuffix(subPath, string(os.PathSeparator))
}
}
klog.V(5).Infof("%s: new paths: %+v", w.targetDir, newPaths.List())
klog.V(5).Infof("%s: new paths: %+v", w.targetDir, sets.List(newPaths))

result := paths.Difference(newPaths)
klog.V(5).Infof("%s: paths to remove: %+v", w.targetDir, result)
Expand Down Expand Up @@ -487,7 +487,7 @@ func (w *AtomicWriter) createUserVisibleFiles(payload map[string]FileProjection)

// removeUserVisiblePaths removes the set of paths from the user-visible
// portion of the writer's target directory.
func (w *AtomicWriter) removeUserVisiblePaths(paths sets.String) error {
func (w *AtomicWriter) removeUserVisiblePaths(paths sets.Set[string]) error {
ps := string(os.PathSeparator)
var lasterr error
for p := range paths {
Expand Down
26 changes: 13 additions & 13 deletions pkg/volume/util/atomic_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestPathsToRemove(t *testing.T) {
name string
payload1 map[string]FileProjection
payload2 map[string]FileProjection
expected sets.String
expected sets.Set[string]
}{
{
name: "simple",
Expand All @@ -144,7 +144,7 @@ func TestPathsToRemove(t *testing.T) {
payload2: map[string]FileProjection{
"foo.txt": {Mode: 0644, Data: []byte("foo")},
},
expected: sets.NewString("bar.txt"),
expected: sets.New[string]("bar.txt"),
},
{
name: "simple 2",
Expand All @@ -155,7 +155,7 @@ func TestPathsToRemove(t *testing.T) {
payload2: map[string]FileProjection{
"foo.txt": {Mode: 0644, Data: []byte("foo")},
},
expected: sets.NewString("zip/bar.txt", "zip"),
expected: sets.New[string]("zip/bar.txt", "zip"),
},
{
name: "subdirs 1",
Expand All @@ -166,7 +166,7 @@ func TestPathsToRemove(t *testing.T) {
payload2: map[string]FileProjection{
"foo.txt": {Mode: 0644, Data: []byte("foo")},
},
expected: sets.NewString("zip/zap/bar.txt", "zip", "zip/zap"),
expected: sets.New[string]("zip/zap/bar.txt", "zip", "zip/zap"),
},
{
name: "subdirs 2",
Expand All @@ -177,7 +177,7 @@ func TestPathsToRemove(t *testing.T) {
payload2: map[string]FileProjection{
"foo.txt": {Mode: 0644, Data: []byte("foo")},
},
expected: sets.NewString("zip/1/2/3/4/bar.txt", "zip", "zip/1", "zip/1/2", "zip/1/2/3", "zip/1/2/3/4"),
expected: sets.New[string]("zip/1/2/3/4/bar.txt", "zip", "zip/1", "zip/1/2", "zip/1/2/3", "zip/1/2/3/4"),
},
{
name: "subdirs 3",
Expand All @@ -189,7 +189,7 @@ func TestPathsToRemove(t *testing.T) {
payload2: map[string]FileProjection{
"foo.txt": {Mode: 0644, Data: []byte("foo")},
},
expected: sets.NewString("zip/1/2/3/4/bar.txt", "zip", "zip/1", "zip/1/2", "zip/1/2/3", "zip/1/2/3/4", "zap", "zap/a", "zap/a/b", "zap/a/b/c", "zap/a/b/c/bar.txt"),
expected: sets.New[string]("zip/1/2/3/4/bar.txt", "zip", "zip/1", "zip/1/2", "zip/1/2/3", "zip/1/2/3/4", "zap", "zap/a", "zap/a/b", "zap/a/b/c", "zap/a/b/c/bar.txt"),
},
{
name: "subdirs 4",
Expand All @@ -203,7 +203,7 @@ func TestPathsToRemove(t *testing.T) {
"foo.txt": {Mode: 0644, Data: []byte("foo")},
"zap/1/2/magic.txt": {Mode: 0644, Data: []byte("indigo")},
},
expected: sets.NewString("zap/1/2/3/4/bar.txt", "zap/1/2/3", "zap/1/2/3/4", "zap/1/2/3/4/bar.txt", "zap/1/2/c", "zap/1/2/c/bar.txt"),
expected: sets.New[string]("zap/1/2/3/4/bar.txt", "zap/1/2/3", "zap/1/2/3/4", "zap/1/2/3/4/bar.txt", "zap/1/2/c", "zap/1/2/c/bar.txt"),
},
{
name: "subdirs 5",
Expand All @@ -216,7 +216,7 @@ func TestPathsToRemove(t *testing.T) {
"foo.txt": {Mode: 0644, Data: []byte("foo")},
"zap/1/2/magic.txt": {Mode: 0644, Data: []byte("indigo")},
},
expected: sets.NewString("zap/1/2/3/4/bar.txt", "zap/1/2/3", "zap/1/2/3/4", "zap/1/2/3/4/bar.txt", "zap/1/2/c", "zap/1/2/c/bar.txt"),
expected: sets.New[string]("zap/1/2/3/4/bar.txt", "zap/1/2/3", "zap/1/2/3/4", "zap/1/2/3/4/bar.txt", "zap/1/2/c", "zap/1/2/c/bar.txt"),
},
}

Expand Down Expand Up @@ -818,7 +818,7 @@ func TestValidatePayload(t *testing.T) {
cases := []struct {
name string
payload map[string]FileProjection
expected sets.String
expected sets.Set[string]
valid bool
}{
{
Expand All @@ -828,7 +828,7 @@ func TestValidatePayload(t *testing.T) {
"bar": {},
},
valid: true,
expected: sets.NewString("foo", "bar"),
expected: sets.New[string]("foo", "bar"),
},
{
name: "payload with path length > 4096 is invalid",
Expand Down Expand Up @@ -871,11 +871,11 @@ func TestValidatePayload(t *testing.T) {
"foo////bar": {},
},
valid: true,
expected: sets.NewString("foo/bar"),
expected: sets.New[string]("foo/bar"),
},
}
getPayloadPaths := func(payload map[string]FileProjection) sets.String {
paths := sets.NewString()
getPayloadPaths := func(payload map[string]FileProjection) sets.Set[string] {
paths := sets.New[string]()
for path := range payload {
paths.Insert(path)
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/volume/util/nested_volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"path/filepath"
"testing"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
Expand All @@ -30,7 +30,7 @@ import (
type testCases struct {
name string
err bool
expected sets.String
expected sets.Set[string]
volname string
pod v1.Pod
}
Expand All @@ -45,7 +45,7 @@ func TestGetNestedMountpoints(t *testing.T) {
{
name: "Simple Pod",
err: false,
expected: sets.NewString(),
expected: sets.New[string](),
volname: "vol1",
pod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -66,7 +66,7 @@ func TestGetNestedMountpoints(t *testing.T) {
{
name: "Simple Nested Pod",
err: false,
expected: sets.NewString("nested"),
expected: sets.New[string]("nested"),
volname: "vol1",
pod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -88,7 +88,7 @@ func TestGetNestedMountpoints(t *testing.T) {
{
name: "Unsorted Nested Pod",
err: false,
expected: sets.NewString("nested", "nested2", "nested-vol", "nested.vol"),
expected: sets.New[string]("nested", "nested2", "nested-vol", "nested.vol"),
volname: "vol1",
pod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestGetNestedMountpoints(t *testing.T) {
{
name: "Multiple vol1 mounts Pod",
err: false,
expected: sets.NewString("nested", "nested2"),
expected: sets.New[string]("nested", "nested2"),
volname: "vol1",
pod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -143,7 +143,7 @@ func TestGetNestedMountpoints(t *testing.T) {
name: "Big Pod",
err: false,
volname: "vol1",
expected: sets.NewString(filepath.Join("sub1", "sub2", "sub3"), filepath.Join("sub1", "sub2", "sub4"), filepath.Join("sub1", "sub2", "sub6"), "sub"),
expected: sets.New[string](filepath.Join("sub1", "sub2", "sub3"), filepath.Join("sub1", "sub2", "sub4"), filepath.Join("sub1", "sub2", "sub6"), "sub"),
pod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Expand Down Expand Up @@ -227,7 +227,7 @@ func TestGetNestedMountpoints(t *testing.T) {
continue
}
}
actual := sets.NewString(dirs...)
actual := sets.New[string](dirs...)
if !test.expected.Equal(actual) {
t.Errorf("%v: unexpected nested directories created:\nexpected: %v\n got: %v", test.name, test.expected, actual)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/volume/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func MountOptionFromSpec(spec *volume.Spec, options ...string) []string {

// JoinMountOptions joins mount options eliminating duplicates
func JoinMountOptions(userOptions []string, systemOptions []string) []string {
allMountOptions := sets.NewString()
allMountOptions := sets.New[string]()

for _, mountOption := range userOptions {
if len(mountOption) > 0 {
Expand All @@ -270,7 +270,7 @@ func JoinMountOptions(userOptions []string, systemOptions []string) []string {
for _, mountOption := range systemOptions {
allMountOptions.Insert(mountOption)
}
return allMountOptions.List()
return sets.List(allMountOptions)
}

// ContainsAccessMode returns whether the requested mode is contained by modes
Expand Down Expand Up @@ -612,9 +612,9 @@ func GetLocalPersistentVolumeNodeNames(pv *v1.PersistentVolume) []string {
// GetPodVolumeNames returns names of volumes that are used in a pod,
// either as filesystem mount or raw block device, together with list
// of all SELinux contexts of all containers that use the volumes.
func GetPodVolumeNames(pod *v1.Pod) (mounts sets.String, devices sets.String, seLinuxContainerContexts map[string][]*v1.SELinuxOptions) {
mounts = sets.NewString()
devices = sets.NewString()
func GetPodVolumeNames(pod *v1.Pod) (mounts sets.Set[string], devices sets.Set[string], seLinuxContainerContexts map[string][]*v1.SELinuxOptions) {
mounts = sets.New[string]()
devices = sets.New[string]()
seLinuxContainerContexts = make(map[string][]*v1.SELinuxOptions)

podutil.VisitContainers(&pod.Spec, podutil.AllFeatureEnabledContainers(), func(container *v1.Container, containerType podutil.ContainerType) bool {
Expand Down

0 comments on commit 14bd183

Please sign in to comment.