Skip to content

Commit 594b597

Browse files
fabioyfabioy
authored
Merge pull request kubernetes#29746 from pmorie/automated-cherry-pick-of-#29673-upstream-release-1.3
Automated cherry pick of kubernetes#29673
2 parents 1c45e26 + 7a266c2 commit 594b597

File tree

9 files changed

+312
-88
lines changed

9 files changed

+312
-88
lines changed

‎pkg/controller/volume/attachdetach/cache/actual_state_of_world.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ type nodeToUpdateStatusFor struct {
224224
}
225225

226226
func (asw *actualStateOfWorld) MarkVolumeAsAttached(
227-
volumeSpec *volume.Spec, nodeName string, devicePath string) error {
227+
_ api.UniqueVolumeName, volumeSpec *volume.Spec, nodeName string, devicePath string) error {
228228
_, err := asw.AddVolumeNode(volumeSpec, nodeName, devicePath)
229229
return err
230230
}

‎pkg/kubelet/volumemanager/cache/actual_state_of_world.go

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,6 @@ type ActualStateOfWorld interface {
5151
// operationexecutor to interact with it.
5252
operationexecutor.ActualStateOfWorldAttacherUpdater
5353

54-
// AddVolume adds the given volume to the cache indicating the specified
55-
// volume is attached to this node. A unique volume name is generated from
56-
// the volumeSpec and returned on success.
57-
// If a volume with the same generated name already exists, this is a noop.
58-
// If no volume plugin can support the given volumeSpec or more than one
59-
// plugin can support it, an error is returned.
60-
AddVolume(volumeSpec *volume.Spec, devicePath string) (api.UniqueVolumeName, error)
61-
6254
// AddPodToVolume adds the given pod to the given volume in the cache
6355
// indicating the specified volume has been successfully mounted to the
6456
// specified pod.
@@ -274,9 +266,8 @@ type mountedPod struct {
274266
}
275267

276268
func (asw *actualStateOfWorld) MarkVolumeAsAttached(
277-
volumeSpec *volume.Spec, nodeName string, devicePath string) error {
278-
_, err := asw.AddVolume(volumeSpec, devicePath)
279-
return err
269+
volumeName api.UniqueVolumeName, volumeSpec *volume.Spec, _, devicePath string) error {
270+
return asw.addVolume(volumeName, volumeSpec, devicePath)
280271
}
281272

282273
func (asw *actualStateOfWorld) MarkVolumeAsDetached(
@@ -315,27 +306,34 @@ func (asw *actualStateOfWorld) MarkDeviceAsUnmounted(
315306
return asw.SetVolumeGloballyMounted(volumeName, false /* globallyMounted */)
316307
}
317308

318-
func (asw *actualStateOfWorld) AddVolume(
319-
volumeSpec *volume.Spec, devicePath string) (api.UniqueVolumeName, error) {
309+
// addVolume adds the given volume to the cache indicating the specified
310+
// volume is attached to this node. If no volume name is supplied, a unique
311+
// volume name is generated from the volumeSpec and returned on success. If a
312+
// volume with the same generated name already exists, this is a noop. If no
313+
// volume plugin can support the given volumeSpec or more than one plugin can
314+
// support it, an error is returned.
315+
func (asw *actualStateOfWorld) addVolume(
316+
volumeName api.UniqueVolumeName, volumeSpec *volume.Spec, devicePath string) error {
320317
asw.Lock()
321318
defer asw.Unlock()
322319

323320
volumePlugin, err := asw.volumePluginMgr.FindPluginBySpec(volumeSpec)
324321
if err != nil || volumePlugin == nil {
325-
return "", fmt.Errorf(
322+
return fmt.Errorf(
326323
"failed to get Plugin from volumeSpec for volume %q err=%v",
327324
volumeSpec.Name(),
328325
err)
329326
}
330327

331-
volumeName, err :=
332-
volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec)
333-
if err != nil {
334-
return "", fmt.Errorf(
335-
"failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v",
336-
volumeSpec.Name(),
337-
volumePlugin.GetPluginName(),
338-
err)
328+
if len(volumeName) == 0 {
329+
volumeName, err = volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec)
330+
if err != nil {
331+
return fmt.Errorf(
332+
"failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v",
333+
volumeSpec.Name(),
334+
volumePlugin.GetPluginName(),
335+
err)
336+
}
339337
}
340338

341339
pluginIsAttachable := false
@@ -357,7 +355,7 @@ func (asw *actualStateOfWorld) AddVolume(
357355
asw.attachedVolumes[volumeName] = volumeObj
358356
}
359357

360-
return volumeObj.volumeName, nil
358+
return nil
361359
}
362360

363361
func (asw *actualStateOfWorld) AddPodToVolume(

‎pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go

Lines changed: 76 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ import (
2626
"k8s.io/kubernetes/pkg/volume/util/volumehelper"
2727
)
2828

29-
// Calls AddVolume() once to add volume
29+
var emptyVolumeName = api.UniqueVolumeName("")
30+
31+
// Calls MarkVolumeAsAttached() once to add volume
3032
// Verifies newly added volume exists in GetUnmountedVolumes()
3133
// Verifies newly added volume doesn't exist in GetGloballyMountedVolumes()
32-
func Test_AddVolume_Positive_NewVolume(t *testing.T) {
34+
func Test_MarkVolumeAsAttached_Positive_NewVolume(t *testing.T) {
3335
// Arrange
34-
volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
36+
volumePluginMgr, plugin := volumetesting.GetTestVolumePluginMgr(t)
3537
asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr)
3638
pod := &api.Pod{
3739
ObjectMeta: api.ObjectMeta{
@@ -53,28 +55,29 @@ func Test_AddVolume_Positive_NewVolume(t *testing.T) {
5355
}
5456
volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]}
5557
devicePath := "fake/device/path"
58+
generatedVolumeName, _ := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec)
5659

5760
// Act
58-
generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath)
61+
err := asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath)
5962

6063
// Assert
6164
if err != nil {
62-
t.Fatalf("AddVolume failed. Expected: <no error> Actual: <%v>", err)
65+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
6366
}
6467

6568
verifyVolumeExistsAsw(t, generatedVolumeName, true /* shouldExist */, asw)
6669
verifyVolumeExistsInUnmountedVolumes(t, generatedVolumeName, asw)
6770
verifyVolumeDoesntExistInGloballyMountedVolumes(t, generatedVolumeName, asw)
6871
}
6972

70-
// Calls AddVolume() twice to add the same volume
71-
// Verifies second call doesn't fail
73+
// Calls MarkVolumeAsAttached() once to add volume, specifying a name --
74+
// establishes that the supplied volume name is used to register the volume
75+
// rather than the generated one.
7276
// Verifies newly added volume exists in GetUnmountedVolumes()
7377
// Verifies newly added volume doesn't exist in GetGloballyMountedVolumes()
74-
func Test_AddVolume_Positive_ExistingVolume(t *testing.T) {
78+
func Test_MarkVolumeAsAttached_SuppliedVolumeName_Positive_NewVolume(t *testing.T) {
7579
// Arrange
7680
volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
77-
devicePath := "fake/device/path"
7881
asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr)
7982
pod := &api.Pod{
8083
ObjectMeta: api.ObjectMeta{
@@ -94,19 +97,64 @@ func Test_AddVolume_Positive_ExistingVolume(t *testing.T) {
9497
},
9598
},
9699
}
100+
volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]}
101+
devicePath := "fake/device/path"
102+
volumeName := api.UniqueVolumeName("this-would-never-be-a-volume-name")
103+
104+
// Act
105+
err := asw.MarkVolumeAsAttached(volumeName, volumeSpec, "" /* nodeName */, devicePath)
106+
107+
// Assert
108+
if err != nil {
109+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
110+
}
97111

112+
verifyVolumeExistsAsw(t, volumeName, true /* shouldExist */, asw)
113+
verifyVolumeExistsInUnmountedVolumes(t, volumeName, asw)
114+
verifyVolumeDoesntExistInGloballyMountedVolumes(t, volumeName, asw)
115+
}
116+
117+
// Calls MarkVolumeAsAttached() twice to add the same volume
118+
// Verifies second call doesn't fail
119+
// Verifies newly added volume exists in GetUnmountedVolumes()
120+
// Verifies newly added volume doesn't exist in GetGloballyMountedVolumes()
121+
func Test_MarkVolumeAsAttached_Positive_ExistingVolume(t *testing.T) {
122+
// Arrange
123+
volumePluginMgr, plugin := volumetesting.GetTestVolumePluginMgr(t)
124+
devicePath := "fake/device/path"
125+
asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr)
126+
pod := &api.Pod{
127+
ObjectMeta: api.ObjectMeta{
128+
Name: "pod1",
129+
UID: "pod1uid",
130+
},
131+
Spec: api.PodSpec{
132+
Volumes: []api.Volume{
133+
{
134+
Name: "volume-name",
135+
VolumeSource: api.VolumeSource{
136+
GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{
137+
PDName: "fake-device1",
138+
},
139+
},
140+
},
141+
},
142+
},
143+
}
98144
volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]}
99-
generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath)
145+
generatedVolumeName, _ := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec)
146+
147+
err := asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath)
100148
if err != nil {
101-
t.Fatalf("AddVolume failed. Expected: <no error> Actual: <%v>", err)
149+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
102150
}
103151

104152
// Act
105-
generatedVolumeName, err = asw.AddVolume(volumeSpec, devicePath)
153+
err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath)
106154

107155
// Assert
108156
if err != nil {
109-
t.Fatalf("AddVolume failed. Expected: <no error> Actual: <%v>", err)
157+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
110158
}
111159

112160
verifyVolumeExistsAsw(t, generatedVolumeName, true /* shouldExist */, asw)
@@ -141,14 +189,12 @@ func Test_AddPodToVolume_Positive_ExistingVolumeNewNode(t *testing.T) {
141189
},
142190
},
143191
}
144-
145192
volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]}
146-
volumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(
147-
plugin, volumeSpec)
193+
generatedVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec)
148194

149-
generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath)
195+
err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath)
150196
if err != nil {
151-
t.Fatalf("AddVolume failed. Expected: <no error> Actual: <%v>", err)
197+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
152198
}
153199
podName := volumehelper.GetUniquePodName(pod)
154200

@@ -159,7 +205,7 @@ func Test_AddPodToVolume_Positive_ExistingVolumeNewNode(t *testing.T) {
159205

160206
// Act
161207
err = asw.AddPodToVolume(
162-
podName, pod.UID, volumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */)
208+
podName, pod.UID, generatedVolumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */)
163209

164210
// Assert
165211
if err != nil {
@@ -202,12 +248,12 @@ func Test_AddPodToVolume_Positive_ExistingVolumeExistingNode(t *testing.T) {
202248
}
203249

204250
volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]}
205-
volumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(
251+
generatedVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(
206252
plugin, volumeSpec)
207253

208-
generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath)
254+
err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath)
209255
if err != nil {
210-
t.Fatalf("AddVolume failed. Expected: <no error> Actual: <%v>", err)
256+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
211257
}
212258
podName := volumehelper.GetUniquePodName(pod)
213259

@@ -217,14 +263,14 @@ func Test_AddPodToVolume_Positive_ExistingVolumeExistingNode(t *testing.T) {
217263
}
218264

219265
err = asw.AddPodToVolume(
220-
podName, pod.UID, volumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */)
266+
podName, pod.UID, generatedVolumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */)
221267
if err != nil {
222268
t.Fatalf("AddPodToVolume failed. Expected: <no error> Actual: <%v>", err)
223269
}
224270

225271
// Act
226272
err = asw.AddPodToVolume(
227-
podName, pod.UID, volumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */)
273+
podName, pod.UID, generatedVolumeName, mounter, volumeSpec.Name(), "" /* volumeGidValue */)
228274

229275
// Assert
230276
if err != nil {
@@ -301,13 +347,13 @@ func Test_AddPodToVolume_Negative_VolumeDoesntExist(t *testing.T) {
301347
asw)
302348
}
303349

304-
// Calls AddVolume() once to add volume
350+
// Calls MarkVolumeAsAttached() once to add volume
305351
// Calls MarkDeviceAsMounted() to mark volume as globally mounted.
306352
// Verifies newly added volume exists in GetUnmountedVolumes()
307353
// Verifies newly added volume exists in GetGloballyMountedVolumes()
308354
func Test_MarkDeviceAsMounted_Positive_NewVolume(t *testing.T) {
309355
// Arrange
310-
volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
356+
volumePluginMgr, plugin := volumetesting.GetTestVolumePluginMgr(t)
311357
asw := NewActualStateOfWorld("mynode" /* nodeName */, volumePluginMgr)
312358
pod := &api.Pod{
313359
ObjectMeta: api.ObjectMeta{
@@ -329,9 +375,11 @@ func Test_MarkDeviceAsMounted_Positive_NewVolume(t *testing.T) {
329375
}
330376
volumeSpec := &volume.Spec{Volume: &pod.Spec.Volumes[0]}
331377
devicePath := "fake/device/path"
332-
generatedVolumeName, err := asw.AddVolume(volumeSpec, devicePath)
378+
generatedVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec(plugin, volumeSpec)
379+
380+
err = asw.MarkVolumeAsAttached(emptyVolumeName, volumeSpec, "" /* nodeName */, devicePath)
333381
if err != nil {
334-
t.Fatalf("AddVolume failed. Expected: <no error> Actual: <%v>", err)
382+
t.Fatalf("MarkVolumeAsAttached failed. Expected: <no error> Actual: <%v>", err)
335383
}
336384

337385
// Act

‎pkg/kubelet/volumemanager/cache/desired_state_of_world.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,22 +183,35 @@ func (dsw *desiredStateOfWorld) AddPodToVolume(
183183
err)
184184
}
185185

186-
volumeName, err :=
187-
volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec)
188-
if err != nil {
189-
return "", fmt.Errorf(
190-
"failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v",
191-
volumeSpec.Name(),
192-
volumePlugin.GetPluginName(),
193-
err)
186+
var volumeName api.UniqueVolumeName
187+
188+
// The unique volume name used depends on whether the volume is attachable
189+
// or not.
190+
attachable := dsw.isAttachableVolume(volumeSpec)
191+
if attachable {
192+
// For attachable volumes, use the unique volume name as reported by
193+
// the plugin.
194+
volumeName, err =
195+
volumehelper.GetUniqueVolumeNameFromSpec(volumePlugin, volumeSpec)
196+
if err != nil {
197+
return "", fmt.Errorf(
198+
"failed to GetUniqueVolumeNameFromSpec for volumeSpec %q using volume plugin %q err=%v",
199+
volumeSpec.Name(),
200+
volumePlugin.GetPluginName(),
201+
err)
202+
}
203+
} else {
204+
// For non-attachable volumes, generate a unique name based on the pod
205+
// namespace and name and the name of the volume within the pod.
206+
volumeName = volumehelper.GetUniqueVolumeNameForNonAttachableVolume(podName, volumePlugin, outerVolumeSpecName)
194207
}
195208

196209
volumeObj, volumeExists := dsw.volumesToMount[volumeName]
197210
if !volumeExists {
198211
volumeObj = volumeToMount{
199212
volumeName: volumeName,
200213
podsToMount: make(map[types.UniquePodName]podToMount),
201-
pluginIsAttachable: dsw.isAttachableVolume(volumeSpec),
214+
pluginIsAttachable: attachable,
202215
volumeGidValue: volumeGidValue,
203216
reportedInUse: false,
204217
}

‎pkg/volume/git_repo/git_repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (plugin *gitRepoPlugin) GetPluginName() string {
6363
func (plugin *gitRepoPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
6464
volumeSource, _ := getVolumeSource(spec)
6565
if volumeSource == nil {
66-
return "", fmt.Errorf("Spec does not reference a GCE volume type")
66+
return "", fmt.Errorf("Spec does not reference a Git repo volume type")
6767
}
6868

6969
return fmt.Sprintf(

0 commit comments

Comments
 (0)