Skip to content

Commit cd78965

Browse files
gnufiedgnufied
committed
Change UpdateStatus to ApplyStatus
1 parent 3438cd2 commit cd78965

6 files changed

Lines changed: 87 additions & 172 deletions

File tree

‎pkg/common/provisioner_utils.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func CreateLocalPV(ctx context.Context, args CreateLocalPVArgs) error {
263263
return fmt.Errorf("error creating pv %s: %w", pvName, err)
264264
}
265265

266-
_, err = deviceHandler.UpdateStatus(ctx, pvName, runtimeConfig.Namespace, args.KName, args.DevicePath, obj)
266+
_, err = deviceHandler.ApplyStatus(ctx, pvName, runtimeConfig.Namespace, args.KName, args.DevicePath, obj)
267267
if err != nil {
268268
return fmt.Errorf("error updating localvolumedevicelink object: %w", err)
269269
}

‎pkg/diskmaker/controllers/lv/create_pv_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ func TestCreatePV(t *testing.T) {
317317
// TestCreateLocalPV_DeviceLinkArgOrder verifies that CreateLocalPV passes KName
318318
// to the by-id symlink matcher and DevicePath to blkid — and not the other way
319319
// around. A previous bug had these two arguments swapped in the
320-
// UpdateStatusAndPV call inside CreateLocalPV.
320+
// ApplyStatus call inside CreateLocalPV.
321321
//
322322
// The test uses deliberately distinct values for KName and DevicePath so that
323323
// swapping them would produce wrong results:

‎pkg/diskmaker/controllers/lv/reconcile.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ func (r *LocalVolumeReconciler) processRejectedDevicesForDeviceLinks(ctx context
521521
source, _, _, err := getSymlinkSourceAndTarget(diskLocation, symLinkDirPath)
522522
if err != nil {
523523
klog.ErrorS(err, "failed to get symlink source and target", "deviceNameLocation", diskLocation)
524-
break
524+
continue
525525
}
526526

527527
blockDevice := diskLocation.BlockDevice
@@ -540,7 +540,7 @@ func (r *LocalVolumeReconciler) processRejectedDevicesForDeviceLinks(ctx context
540540

541541
pvName := common.GeneratePVName(existingSymlink, r.runtimeConfig.Node.Name, storageClassName)
542542
deviceHandler := internal.NewDeviceLinkHandler(source, preferredSymLink, r.Client)
543-
_, err = deviceHandler.UpdateStatus(ctx, pvName, r.runtimeConfig.Namespace, blockDevice.KName, devicePath, r.localVolume)
543+
_, err = deviceHandler.ApplyStatus(ctx, pvName, r.runtimeConfig.Namespace, blockDevice.KName, devicePath, r.localVolume)
544544
if err != nil {
545545
klog.ErrorS(err, "error updating LocalVolumeDeviceLink", "device", blockDevice.Name)
546546
}

‎pkg/diskmaker/controllers/lvset/reconcile.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ func (r *LocalVolumeSetReconciler) processRejectedDevicesForDeviceLinks(ctx cont
297297

298298
pvName := common.GeneratePVName(existingSymlink, r.runtimeConfig.Node.Name, storageClassName)
299299
deviceHandler := internal.NewDeviceLinkHandler(symlinkSourcePath, preferredSymLink, r.Client)
300-
_, err = deviceHandler.UpdateStatus(ctx, pvName, r.runtimeConfig.Namespace, blockDevice.KName, devicePath, lvset)
300+
_, err = deviceHandler.ApplyStatus(ctx, pvName, r.runtimeConfig.Namespace, blockDevice.KName, devicePath, lvset)
301301
if err != nil {
302302
klog.ErrorS(err, "error updating LocalVolumeDeviceLink", "device", blockDevice.Name)
303303
}

‎pkg/internal/device_link_handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func isNilOwnerObject(ownerObj runtime.Object) bool {
145145
return value.Kind() == reflect.Ptr && value.IsNil()
146146
}
147147

148-
func (dl *DeviceLinkHandler) UpdateStatus(ctx context.Context, pvName, namespace, kName, devicePath string, ownerObj runtime.Object) (*v1.LocalVolumeDeviceLink, error) {
148+
func (dl *DeviceLinkHandler) ApplyStatus(ctx context.Context, pvName, namespace, kName, devicePath string, ownerObj runtime.Object) (*v1.LocalVolumeDeviceLink, error) {
149149
klog.V(2).Infof("updating lvdl with currentSymlink: %s, preferredSymlink: %s, devicePath: %s, kname: %s", dl.currentSymlink, dl.preferredSymlink, devicePath, kName)
150150

151151
// Update is best-effort and independent from Create: if either the PV or

‎pkg/internal/device_link_handler_test.go‎

Lines changed: 81 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,53 @@ import (
1717
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1818
)
1919

20+
func newLocalVolume(name, namespace string, uid types.UID) *v1.LocalVolume {
21+
return &v1.LocalVolume{
22+
TypeMeta: metav1.TypeMeta{
23+
Kind: v1.LocalVolumeKind,
24+
APIVersion: v1.GroupVersion.String(),
25+
},
26+
ObjectMeta: metav1.ObjectMeta{
27+
Name: name,
28+
Namespace: namespace,
29+
UID: uid,
30+
},
31+
}
32+
}
33+
34+
func newLocalVolumeSet(name, namespace string, uid types.UID) *v1alpha1.LocalVolumeSet {
35+
return &v1alpha1.LocalVolumeSet{
36+
TypeMeta: metav1.TypeMeta{
37+
Kind: v1.LocalVolumeSetKind,
38+
APIVersion: v1alpha1.GroupVersion.String(),
39+
},
40+
ObjectMeta: metav1.ObjectMeta{
41+
Name: name,
42+
Namespace: namespace,
43+
UID: uid,
44+
},
45+
}
46+
}
47+
48+
func newLVDL(name, namespace, pvName string) *v1.LocalVolumeDeviceLink {
49+
return &v1.LocalVolumeDeviceLink{
50+
ObjectMeta: metav1.ObjectMeta{
51+
Name: name,
52+
Namespace: namespace,
53+
},
54+
Spec: v1.LocalVolumeDeviceLinkSpec{
55+
PersistentVolumeName: pvName,
56+
Policy: v1.DeviceLinkPolicyNone,
57+
},
58+
}
59+
}
60+
61+
func newPV(name string) *corev1.PersistentVolume {
62+
return &corev1.PersistentVolume{
63+
ObjectMeta: metav1.ObjectMeta{Name: name},
64+
}
65+
}
66+
2067
// newFakeDeviceLinkClient builds a controller-runtime fake client registered
2168
// with the v1 scheme (which includes LocalVolumeDeviceLink).
2269
// WithStatusSubresource is set so that the fake client enforces the status
@@ -88,93 +135,39 @@ func TestDeviceLinkHandler_Create(t *testing.T) {
88135
namespace: "default",
89136
currentSymlink: "/dev/disk/by-id/wwn-current",
90137
preferredSymlink: "/dev/disk/by-id/wwn-preferred",
91-
ownerObj: &v1.LocalVolume{
92-
TypeMeta: metav1.TypeMeta{
93-
Kind: v1.LocalVolumeKind,
94-
APIVersion: v1.GroupVersion.String(),
95-
},
96-
ObjectMeta: metav1.ObjectMeta{
97-
Name: "lv-a",
98-
Namespace: "default",
99-
UID: types.UID("11111111-2222-3333-4444-555555555555"),
100-
},
101-
},
102-
expectedPolicy: v1.DeviceLinkPolicyNone,
103-
expectedListLen: 1,
138+
ownerObj: newLocalVolume("lv-a", "default", "11111111-2222-3333-4444-555555555555"),
139+
expectedPolicy: v1.DeviceLinkPolicyNone,
140+
expectedListLen: 1,
104141
},
105142
{
106143
name: "creates new lvdl with localvolumeset ownerref",
107144
pvName: "local-pv-lvset-owner",
108145
namespace: "default",
109146
currentSymlink: "/dev/disk/by-id/scsi-current",
110147
preferredSymlink: "/dev/disk/by-id/scsi-preferred",
111-
ownerObj: &v1alpha1.LocalVolumeSet{
112-
TypeMeta: metav1.TypeMeta{
113-
Kind: v1.LocalVolumeSetKind,
114-
APIVersion: v1alpha1.GroupVersion.String(),
115-
},
116-
ObjectMeta: metav1.ObjectMeta{
117-
Name: "lvset-a",
118-
Namespace: "default",
119-
UID: types.UID("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"),
120-
},
121-
},
122-
expectedPolicy: v1.DeviceLinkPolicyNone,
123-
expectedListLen: 1,
148+
ownerObj: newLocalVolumeSet("lvset-a", "default", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"),
149+
expectedPolicy: v1.DeviceLinkPolicyNone,
150+
expectedListLen: 1,
124151
},
125152
{
126-
name: "idempotent when object already exists",
127-
pvName: "local-pv-existing",
128-
namespace: "default",
129-
existing: &v1.LocalVolumeDeviceLink{
130-
ObjectMeta: metav1.ObjectMeta{
131-
Name: "local-pv-existing",
132-
Namespace: "default",
133-
},
134-
Spec: v1.LocalVolumeDeviceLinkSpec{
135-
PersistentVolumeName: "local-pv-existing",
136-
Policy: v1.DeviceLinkPolicyNone,
137-
},
138-
},
139-
ownerObj: &v1.LocalVolume{
140-
TypeMeta: metav1.TypeMeta{
141-
Kind: v1.LocalVolumeKind,
142-
APIVersion: v1.GroupVersion.String(),
143-
},
144-
ObjectMeta: metav1.ObjectMeta{
145-
Name: "lv-existing",
146-
Namespace: "default",
147-
UID: types.UID("bbbbbbbb-2222-3333-4444-555555555555"),
148-
},
149-
},
153+
name: "idempotent when object already exists",
154+
pvName: "local-pv-existing",
155+
namespace: "default",
156+
existing: newLVDL("local-pv-existing", "default", "local-pv-existing"),
157+
ownerObj: newLocalVolume("lv-existing", "default", "bbbbbbbb-2222-3333-4444-555555555555"),
150158
expectedPolicy: v1.DeviceLinkPolicyNone,
151159
expectedListLen: 1,
152160
},
153161
{
154162
name: "updates stale persistent volume name and preserves policy",
155163
pvName: "local-pv-new",
156164
namespace: "default",
157-
existing: &v1.LocalVolumeDeviceLink{
158-
ObjectMeta: metav1.ObjectMeta{
159-
Name: "local-pv-new",
160-
Namespace: "default",
161-
},
162-
Spec: v1.LocalVolumeDeviceLinkSpec{
163-
PersistentVolumeName: "local-pv-old",
164-
Policy: v1.DeviceLinkPolicyCurrentLinkTarget,
165-
},
166-
},
167-
ownerObj: &v1.LocalVolume{
168-
TypeMeta: metav1.TypeMeta{
169-
Kind: v1.LocalVolumeKind,
170-
APIVersion: v1.GroupVersion.String(),
171-
},
172-
ObjectMeta: metav1.ObjectMeta{
173-
Name: "lv-new",
174-
Namespace: "default",
175-
UID: types.UID("cccccccc-2222-3333-4444-555555555555"),
176-
},
177-
},
165+
existing: func() *v1.LocalVolumeDeviceLink {
166+
lvdl := newLVDL("local-pv-new", "default", "local-pv-old")
167+
lvdl.Spec.Policy = v1.DeviceLinkPolicyCurrentLinkTarget
168+
return lvdl
169+
}(),
170+
ownerObj: newLocalVolume("lv-new", "default", "cccccccc-2222-3333-4444-555555555555"),
178171
expectedPolicy: v1.DeviceLinkPolicyCurrentLinkTarget,
179172
expectedListLen: 1,
180173
},
@@ -255,26 +248,11 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
255248
preferredSymlink: "/dev/disk/by-id/wwn-preferred",
256249
kname: "sda",
257250
devPath: "/dev/sda",
258-
ownerObj: &v1.LocalVolume{
259-
TypeMeta: metav1.TypeMeta{
260-
Kind: v1.LocalVolumeKind,
261-
APIVersion: v1.GroupVersion.String(),
262-
},
263-
ObjectMeta: metav1.ObjectMeta{
264-
Name: "lv-statustest",
265-
Namespace: "default",
266-
UID: types.UID("11111111-aaaa-bbbb-cccc-111111111111"),
267-
},
268-
},
269-
existing: &v1.LocalVolumeDeviceLink{
270-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-statustest", Namespace: "default"},
271-
Spec: v1.LocalVolumeDeviceLinkSpec{PersistentVolumeName: "local-pv-statustest"},
272-
},
273-
existingPV: &corev1.PersistentVolume{
274-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-statustest"},
275-
},
276-
globLinks: []string{"/tmp/wwn-0x1234", "/tmp/scsi-abcde"},
277-
filesystemUUID: "550e8400-e29b-41d4-a716-446655440000",
251+
ownerObj: newLocalVolume("lv-statustest", "default", "11111111-aaaa-bbbb-cccc-111111111111"),
252+
existing: newLVDL("local-pv-statustest", "default", "local-pv-statustest"),
253+
existingPV: newPV("local-pv-statustest"),
254+
globLinks: []string{"/tmp/wwn-0x1234", "/tmp/scsi-abcde"},
255+
filesystemUUID: "550e8400-e29b-41d4-a716-446655440000",
278256
expectedLVDL: &v1.LocalVolumeDeviceLink{
279257
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-statustest", Namespace: "default"},
280258
Spec: v1.LocalVolumeDeviceLinkSpec{PersistentVolumeName: "local-pv-statustest"},
@@ -294,24 +272,9 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
294272
preferredSymlink: "",
295273
kname: "sdb",
296274
devPath: "/dev/sdb",
297-
ownerObj: &v1.LocalVolume{
298-
TypeMeta: metav1.TypeMeta{
299-
Kind: v1.LocalVolumeKind,
300-
APIVersion: v1.GroupVersion.String(),
301-
},
302-
ObjectMeta: metav1.ObjectMeta{
303-
Name: "lv-nolinks",
304-
Namespace: "default",
305-
UID: types.UID("22222222-aaaa-bbbb-cccc-222222222222"),
306-
},
307-
},
308-
existing: &v1.LocalVolumeDeviceLink{
309-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-nolinks", Namespace: "default"},
310-
Spec: v1.LocalVolumeDeviceLinkSpec{PersistentVolumeName: "local-pv-nolinks"},
311-
},
312-
existingPV: &corev1.PersistentVolume{
313-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-nolinks"},
314-
},
275+
ownerObj: newLocalVolume("lv-nolinks", "default", "22222222-aaaa-bbbb-cccc-222222222222"),
276+
existing: newLVDL("local-pv-nolinks", "default", "local-pv-nolinks"),
277+
existingPV: newPV("local-pv-nolinks"),
315278
expectedLVDL: &v1.LocalVolumeDeviceLink{
316279
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-nolinks", Namespace: "default"},
317280
Spec: v1.LocalVolumeDeviceLinkSpec{PersistentVolumeName: "local-pv-nolinks"},
@@ -331,21 +294,9 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
331294
preferredSymlink: "/dev/disk/by-id/wwn-preferred",
332295
kname: "sdc",
333296
devPath: "/dev/sdc",
334-
ownerObj: &v1.LocalVolume{
335-
TypeMeta: metav1.TypeMeta{
336-
Kind: v1.LocalVolumeKind,
337-
APIVersion: v1.GroupVersion.String(),
338-
},
339-
ObjectMeta: metav1.ObjectMeta{
340-
Name: "lv-fullflow",
341-
Namespace: "openshift-local-storage",
342-
UID: types.UID("33333333-aaaa-bbbb-cccc-333333333333"),
343-
},
344-
},
345-
preCreate: true,
346-
existingPV: &corev1.PersistentVolume{
347-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-fullflow"},
348-
},
297+
ownerObj: newLocalVolume("lv-fullflow", "openshift-local-storage", "33333333-aaaa-bbbb-cccc-333333333333"),
298+
preCreate: true,
299+
existingPV: newPV("local-pv-fullflow"),
349300
expectedLVDL: &v1.LocalVolumeDeviceLink{
350301
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-fullflow", Namespace: "openshift-local-storage"},
351302
Spec: v1.LocalVolumeDeviceLinkSpec{PersistentVolumeName: "local-pv-fullflow"},
@@ -365,21 +316,8 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
365316
preferredSymlink: "",
366317
kname: "sdd",
367318
devPath: "/dev/sdd",
368-
ownerObj: &v1.LocalVolume{
369-
TypeMeta: metav1.TypeMeta{
370-
Kind: v1.LocalVolumeKind,
371-
APIVersion: v1.GroupVersion.String(),
372-
},
373-
ObjectMeta: metav1.ObjectMeta{
374-
Name: "lv-missing-pv",
375-
Namespace: "default",
376-
UID: types.UID("44444444-aaaa-bbbb-cccc-444444444444"),
377-
},
378-
},
379-
existing: &v1.LocalVolumeDeviceLink{
380-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-missing", Namespace: "default"},
381-
Spec: v1.LocalVolumeDeviceLinkSpec{PersistentVolumeName: "local-pv-missing"},
382-
},
319+
ownerObj: newLocalVolume("lv-missing-pv", "default", "44444444-aaaa-bbbb-cccc-444444444444"),
320+
existing: newLVDL("local-pv-missing", "default", "local-pv-missing"),
383321
},
384322
{
385323
name: "creates lvdl during status update when pv exists and create was skipped",
@@ -389,20 +327,8 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
389327
preferredSymlink: "",
390328
kname: "sde",
391329
devPath: "/dev/sde",
392-
ownerObj: &v1.LocalVolume{
393-
TypeMeta: metav1.TypeMeta{
394-
Kind: v1.LocalVolumeKind,
395-
APIVersion: v1.GroupVersion.String(),
396-
},
397-
ObjectMeta: metav1.ObjectMeta{
398-
Name: "lv-create-on-update",
399-
Namespace: "default",
400-
UID: types.UID("55555555-aaaa-bbbb-cccc-555555555555"),
401-
},
402-
},
403-
existingPV: &corev1.PersistentVolume{
404-
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-lvdl-missing"},
405-
},
330+
ownerObj: newLocalVolume("lv-create-on-update", "default", "55555555-aaaa-bbbb-cccc-555555555555"),
331+
existingPV: newPV("local-pv-lvdl-missing"),
406332
expectedLVDL: &v1.LocalVolumeDeviceLink{
407333
ObjectMeta: metav1.ObjectMeta{Name: "local-pv-lvdl-missing", Namespace: "default"},
408334
Spec: v1.LocalVolumeDeviceLinkSpec{
@@ -421,7 +347,6 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
421347
}
422348

423349
for _, tc := range testCases {
424-
tc := tc
425350
t.Run(tc.name, func(t *testing.T) {
426351
var runtimeObjects []runtime.Object
427352
if tc.existing != nil {
@@ -435,17 +360,7 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
435360
handler := NewDeviceLinkHandler(tc.currentSymlink, tc.preferredSymlink, fakeClient)
436361

437362
if tc.preCreate {
438-
lvdl, err := handler.Create(context.TODO(), tc.pvName, tc.namespace, &v1.LocalVolume{
439-
TypeMeta: metav1.TypeMeta{
440-
Kind: v1.LocalVolumeKind,
441-
APIVersion: v1.GroupVersion.String(),
442-
},
443-
ObjectMeta: metav1.ObjectMeta{
444-
Name: "owner-precreate",
445-
Namespace: tc.namespace,
446-
UID: types.UID("dddddddd-2222-3333-4444-555555555555"),
447-
},
448-
})
363+
lvdl, err := handler.Create(t.Context(), tc.pvName, tc.namespace, tc.ownerObj)
449364
if err != nil {
450365
t.Fatalf("Create failed: %v", err)
451366
}
@@ -474,7 +389,7 @@ func TestDeviceLinkHandler_UpdateStatusAndPV(t *testing.T) {
474389
ExecCommand = helperCommandBlkid(tc.filesystemUUID)
475390
}
476391

477-
updated, err := handler.UpdateStatus(context.TODO(), tc.pvName, tc.namespace, tc.kname, tc.devPath, tc.ownerObj)
392+
updated, err := handler.ApplyStatus(context.TODO(), tc.pvName, tc.namespace, tc.kname, tc.devPath, tc.ownerObj)
478393
if err != nil {
479394
t.Fatalf("UpdateStatusAndPV returned unexpected error: %v", err)
480395
}

0 commit comments

Comments
 (0)