Skip to content

Commit 6530829

Browse files
synaretephlogistonjohn
authored andcommitted
resources: avoid implicit sharing of volMount between slices
In Go a slice is a fancy name for pointer and size, encapsulated within single sliceHeader struct[1]. When two (or more) distinct sliceHeaders refer to the same underlying array of objects, and modify it, the end result depends on the capacity of the original underlying array[2]. (Note that append may increase the capacity more then the number of elements added). In our case, in buildADPodSpec function, joinVols refer to the same underlying array as smbAllVols and smbServerVols, and thus when appending it over-write values. This, in turn, causes a containers to have configurations with undefined behaviour. Using a simple solution: duplicate volMount arrays so that each slice has its own (unique) copy. The performance penalty is minor. [1] https://go.dev/blog/slices [2] https://go.dev/ref/spec#Appending_and_copying_slices Signed-off-by: Shachar Sharon <[email protected]> Signed-off-by: Shachar Sharon <[email protected]>
1 parent 7fa74c2 commit 6530829

File tree

1 file changed

+35
-31
lines changed

1 file changed

+35
-31
lines changed

internal/resources/pods.go

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,19 @@ func buildADPodSpec(
6666
// for smb server containers (not init containers)
6767
wbSockVol := wbSocketsVolumeAndMount(planner)
6868
volumes = append(volumes, wbSockVol)
69-
// nolint:gocritic
70-
smbServerVols := append(smbAllVols, wbSockVol)
69+
smbServerVols := dupVolMounts(smbAllVols)
70+
smbServerVols = append(smbServerVols, wbSockVol)
7171

7272
// for smbd only
7373
shareVol := shareVolumeAndMount(planner, pvcName)
7474
volumes = append(volumes, shareVol)
75-
// nolint:gocritic
76-
smbdVols := append(smbServerVols, shareVol)
75+
smbdVols := dupVolMounts(smbServerVols)
76+
smbdVols = append(smbdVols, shareVol)
7777

7878
jsrc := getJoinSources(planner)
7979
volumes = append(volumes, jsrc.volumes...)
80-
// nolint:gocritic
81-
joinVols := append(smbAllVols, jsrc.volumes...)
80+
joinVols := dupVolMounts(smbAllVols)
81+
joinVols = append(joinVols, jsrc.volumes...)
8282

8383
podEnv := defaultPodEnv(planner)
8484
// nolint:gocritic
@@ -100,8 +100,8 @@ func buildADPodSpec(
100100
)
101101
volumes = append(volumes, watchVol)
102102
svcWatchVols := []volMount{watchVol}
103-
// nolint:gocritic
104-
dnsRegVols := append(smbServerVols, watchVol)
103+
dnsRegVols := dupVolMounts(smbServerVols)
104+
dnsRegVols = append(dnsRegVols, watchVol)
105105
containers = append(
106106
containers,
107107
buildSvcWatchCtr(planner, svcWatchEnv(planner), svcWatchVols),
@@ -227,9 +227,9 @@ func buildClusteredUserPodSpec(
227227
ctdbPeristentVol,
228228
)))
229229

230-
// nolint:gocritic
231-
ctdbInitVols := append(
232-
podCfgVols,
230+
ctdbInitVols := dupVolMounts(podCfgVols)
231+
ctdbInitVols = append(
232+
ctdbInitVols,
233233
stateVol,
234234
ctdbSharedVol,
235235
ctdbConfigVol,
@@ -240,9 +240,9 @@ func buildClusteredUserPodSpec(
240240
buildCTDBMustHaveNodeCtr(planner, ctdbEnv, ctdbInitVols),
241241
)
242242

243-
// nolint:gocritic
244-
ctdbdVols := append(
245-
podCfgVols,
243+
ctdbdVols := dupVolMounts(podCfgVols)
244+
ctdbdVols = append(
245+
ctdbdVols,
246246
ctdbConfigVol,
247247
ctdbPeristentVol,
248248
ctdbVolatileVol,
@@ -253,9 +253,9 @@ func buildClusteredUserPodSpec(
253253
containers,
254254
buildCTDBDaemonCtr(planner, ctdbEnv, ctdbdVols))
255255

256-
// nolint:gocritic
257-
ctdbManageNodesVols := append(
258-
podCfgVols,
256+
ctdbManageNodesVols := dupVolMounts(podCfgVols)
257+
ctdbManageNodesVols = append(
258+
ctdbManageNodesVols,
259259
ctdbConfigVol,
260260
ctdbSocketsVol,
261261
ctdbSharedVol,
@@ -361,9 +361,9 @@ func buildClusteredADPodSpec(
361361
ctdbPeristentVol,
362362
)))
363363

364-
// nolint:gocritic
365-
ctdbInitVols := append(
366-
podCfgVols,
364+
ctdbInitVols := dupVolMounts(podCfgVols)
365+
ctdbInitVols = append(
366+
ctdbInitVols,
367367
stateVol,
368368
ctdbSharedVol,
369369
ctdbConfigVol,
@@ -374,9 +374,9 @@ func buildClusteredADPodSpec(
374374
buildCTDBMustHaveNodeCtr(planner, ctdbEnv, ctdbInitVols),
375375
)
376376

377-
// nolint:gocritic
378-
ctdbdVols := append(
379-
podCfgVols,
377+
ctdbdVols := dupVolMounts(podCfgVols)
378+
ctdbdVols = append(
379+
ctdbdVols,
380380
ctdbConfigVol,
381381
ctdbPeristentVol,
382382
ctdbVolatileVol,
@@ -387,9 +387,9 @@ func buildClusteredADPodSpec(
387387
containers,
388388
buildCTDBDaemonCtr(planner, ctdbEnv, ctdbdVols))
389389

390-
// nolint:gocritic
391-
ctdbManageNodesVols := append(
392-
podCfgVols,
390+
ctdbManageNodesVols := dupVolMounts(podCfgVols)
391+
ctdbManageNodesVols = append(
392+
ctdbManageNodesVols,
393393
ctdbConfigVol,
394394
ctdbSocketsVol,
395395
ctdbSharedVol,
@@ -399,9 +399,9 @@ func buildClusteredADPodSpec(
399399
buildCTDBManageNodesCtr(planner, ctdbEnv, ctdbManageNodesVols))
400400

401401
// winbindd
402-
// nolint:gocritic
403-
wbVols := append(
404-
podCfgVols,
402+
wbVols := dupVolMounts(podCfgVols)
403+
wbVols = append(
404+
wbVols,
405405
stateVol,
406406
wbSockVol,
407407
ctdbConfigVol,
@@ -426,8 +426,8 @@ func buildClusteredADPodSpec(
426426
)
427427
volumes = append(volumes, watchVol)
428428
svcWatchVols := []volMount{watchVol}
429-
// nolint:gocritic
430-
dnsRegVols := append(wbVols, watchVol)
429+
dnsRegVols := dupVolMounts(wbVols)
430+
dnsRegVols = append(dnsRegVols, watchVol)
431431
containers = append(
432432
containers,
433433
buildSvcWatchCtr(planner, svcWatchEnv(planner), svcWatchVols),
@@ -792,3 +792,7 @@ func getJoinSources(planner *pln.Planner) joinSources {
792792
func joinEnvPaths(p []string) string {
793793
return strings.Join(p, ":")
794794
}
795+
796+
func dupVolMounts(vols []volMount) []volMount {
797+
return append(make([]volMount, 0, len(vols)), vols...)
798+
}

0 commit comments

Comments
 (0)