Skip to content

Commit

Permalink
Protect must gather versions cache with mutex (#6078)
Browse files Browse the repository at this point in the history
Currently the code that accesses the must gather versions cache uses a
map without any protection agains concurrent writes. As a result in some
situations this results in a concurrent writes error:

```
2024-03-12 03:44:13
fatal error: concurrent map writes
github.com/openshift/assisted-service/internal/versions.(*handler).GetMustGatherImages(0xc000222c40, {0xc001dea8a0, 0xb}, {0xc001dea8d4, 0x5}, {0xc0037be000, 0xad2})
	/assisted-service/internal/versions/versions.go:95 +0x2e5
```

This patch uses a mutex to protect access to that map.

Note that this is a simple solution, but not ideal, as it means that
concurrent calls to the `GetMustGatherImages` method will be serialized.
I am not sure if the logic inside that method takes long to execute. If
it does this could impact performance.

Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
  • Loading branch information
jhernand committed Mar 12, 2024
1 parent 0eb2aee commit a2facc2
Showing 1 changed file with 20 additions and 15 deletions.
35 changes: 20 additions & 15 deletions internal/versions/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ func NewHandler(log logrus.FieldLogger, releaseHandler oc.Release, releaseImages
mustGatherVersions MustGatherVersions, releaseImageMirror string, kubeClient client.Client) (*handler, error) {

h := &handler{
mustGatherVersions: mustGatherVersions,
releaseImages: releaseImages,
releaseHandler: releaseHandler,
releaseImageMirror: releaseImageMirror,
log: log,
kubeClient: kubeClient,
sem: semaphore.NewWeighted(30),
mustGatherVersions: mustGatherVersions,
mustGatherVersionsLock: &sync.Mutex{},
releaseImages: releaseImages,
releaseHandler: releaseHandler,
releaseImageMirror: releaseImageMirror,
log: log,
kubeClient: kubeClient,
sem: semaphore.NewWeighted(30),
}

if err := h.validateVersions(); err != nil {
Expand All @@ -54,17 +55,21 @@ func NewHandler(log logrus.FieldLogger, releaseHandler oc.Release, releaseImages
}

type handler struct {
mustGatherVersions MustGatherVersions
releaseImages models.ReleaseImages
imagesLock sync.Mutex
sem *semaphore.Weighted
releaseHandler oc.Release
releaseImageMirror string
log logrus.FieldLogger
kubeClient client.Client
mustGatherVersions MustGatherVersions
mustGatherVersionsLock *sync.Mutex
releaseImages models.ReleaseImages
imagesLock sync.Mutex
sem *semaphore.Weighted
releaseHandler oc.Release
releaseImageMirror string
log logrus.FieldLogger
kubeClient client.Client
}

func (h *handler) GetMustGatherImages(openshiftVersion, cpuArchitecture, pullSecret string) (MustGatherVersion, error) {
h.mustGatherVersionsLock.Lock()
defer h.mustGatherVersionsLock.Unlock()

majMinorVersion, err := toMajorMinor(openshiftVersion)
if err != nil {
return nil, err
Expand Down

0 comments on commit a2facc2

Please sign in to comment.