Skip to content

Commit c9d0286

Browse files
committed
container: fix concurrent map acccess
GetSpec() can be called concurrently in manager/container.go.updateSpec() results into a concurrent map access on the labels map because we're directly updating the map inside GetSpec(). The labels map from the container handler is not a copy of the map itself, just a reference, that's why we're getting the concurrent map access. Fix this by moving the label update with restartcount to the handler's initialization method which is not called concurrently. Signed-off-by: Antonio Murdaca <[email protected]>
1 parent 5382012 commit c9d0286

File tree

2 files changed

+8
-16
lines changed

2 files changed

+8
-16
lines changed

container/crio/handler.go

+4-8
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,6 @@ type crioContainerHandler struct {
8181
ipAddress string
8282

8383
ignoreMetrics container.MetricSet
84-
85-
// container restart count
86-
restartCount int
8784
}
8885

8986
var _ container.ContainerHandler = &crioContainerHandler{}
@@ -175,7 +172,10 @@ func newCrioContainerHandler(
175172
// ignore err and get zero as default, this happens with sandboxes, not sure why...
176173
// kube isn't sending restart count in labels for sandboxes.
177174
restartCount, _ := strconv.Atoi(cInfo.Annotations["io.kubernetes.container.restartCount"])
178-
handler.restartCount = restartCount
175+
// Only adds restartcount label if it's greater than 0
176+
if restartCount > 0 {
177+
handler.labels["restartcount"] = strconv.Itoa(restartCount)
178+
}
179179

180180
handler.ipAddress = cInfo.IP
181181

@@ -225,10 +225,6 @@ func (self *crioContainerHandler) GetSpec() (info.ContainerSpec, error) {
225225
spec, err := common.GetSpec(self.cgroupPaths, self.machineInfoFactory, self.needNet(), hasFilesystem)
226226

227227
spec.Labels = self.labels
228-
// Only adds restartcount label if it's greater than 0
229-
if self.restartCount > 0 {
230-
spec.Labels["restartcount"] = strconv.Itoa(self.restartCount)
231-
}
232228
spec.Envs = self.envs
233229
spec.Image = self.image
234230

container/docker/handler.go

+4-8
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,6 @@ type dockerContainerHandler struct {
114114

115115
// zfs watcher
116116
zfsWatcher *zfs.ZfsWatcher
117-
118-
// container restart count
119-
restartCount int
120117
}
121118

122119
var _ container.ContainerHandler = &dockerContainerHandler{}
@@ -249,7 +246,10 @@ func newDockerContainerHandler(
249246
handler.image = ctnr.Config.Image
250247
handler.networkMode = ctnr.HostConfig.NetworkMode
251248
handler.deviceID = ctnr.GraphDriver.Data["DeviceId"]
252-
handler.restartCount = ctnr.RestartCount
249+
// Only adds restartcount label if it's greater than 0
250+
if ctnr.RestartCount > 0 {
251+
handler.labels["restartcount"] = strconv.Itoa(ctnr.RestartCount)
252+
}
253253

254254
// Obtain the IP address for the contianer.
255255
// If the NetworkMode starts with 'container:' then we need to use the IP address of the container specified.
@@ -385,10 +385,6 @@ func (self *dockerContainerHandler) GetSpec() (info.ContainerSpec, error) {
385385
spec, err := common.GetSpec(self.cgroupPaths, self.machineInfoFactory, self.needNet(), hasFilesystem)
386386

387387
spec.Labels = self.labels
388-
// Only adds restartcount label if it's greater than 0
389-
if self.restartCount > 0 {
390-
spec.Labels["restartcount"] = strconv.Itoa(self.restartCount)
391-
}
392388
spec.Envs = self.envs
393389
spec.Image = self.image
394390

0 commit comments

Comments
 (0)