Fix race conditions and add -race flag to test.sh

[cbuto]

While running unit tests locally, I noticed a race condition (without the -race flag set). After setting the -race flag while running unit tests, they fail with a few race conditions that we should look to resolve.

We should first fix the race conditions and set the -race flag in test.sh to avoid any new races.

One example of a race condition caught:

==================
WARNING: DATA RACE
Write at 0x00c0007acab0 by goroutine 57:
  runtime.mapassign_faststr()
      /usr/local/go/src/runtime/map_faststr.go:202 +0x0
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).saveCacheEntry()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:402 +0x4e7
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).startEventListener()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:546 +0xde9
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.NewMultiTenantServer·dwrap·9()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server.go:170 +0x39

Previous read at 0x00c0007acab0 by goroutine 110:
  runtime.mapaccess2_faststr()
      /usr/local/go/src/runtime/map_faststr.go:107 +0x0
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).initCacheEntry()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:351 +0xaca
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getIndexFile()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/index.go:33 +0x8b
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getIndexFileRequestHandler()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/handlers.go:100 +0xe4
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getIndexFileRequestHandler-fm()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/handlers.go:97 +0x44
  helm.sh/chartmuseum/pkg/chartmuseum/router.(*Router).rootHandler()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/router/router.go:249 +0x6e1
  helm.sh/chartmuseum/pkg/chartmuseum/router.(*Router).rootHandler-fm()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/router/router.go:212 +0x44
  github.com/gin-gonic/gin.(*Context).Next()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/context.go:168 +0x1e4
  github.com/gin-contrib/size.RequestSizeLimiter.func1()
      /Users/cbuto/go/pkg/mod/github.com/gin-contrib/size@v0.0.0-20220102055520-f75bacbc2df3/size.go:88 +0x17b
  github.com/gin-gonic/gin.(*Context).Next()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/context.go:168 +0x222
  helm.sh/chartmuseum/pkg/chartmuseum/router.requestWrapper.func1()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/router/middleware.go:48 +0x19c
  github.com/gin-gonic/gin.(*Context).Next()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/context.go:168 +0x15c
  github.com/gin-gonic/gin.CustomRecoveryWithWriter.func1()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/recovery.go:99 +0xc5
  github.com/gin-gonic/gin.(*Context).Next()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/context.go:168 +0x101
  github.com/gin-gonic/gin.serveError()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/gin.go:591 +0x8a
  github.com/gin-gonic/gin.(*Engine).handleHTTPRequest()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/gin.go:584 +0x8ab
  github.com/gin-gonic/gin.(*Engine).HandleContext()
      /Users/cbuto/go/pkg/mod/github.com/gin-gonic/gin@v1.7.7/gin.go:522 +0x368
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).doRequest()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:97 +0x6b7
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).testAllRoutes()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:1030 +0x2cd2
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).TestRoutes()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:914 +0x328
  runtime.call16()
      /usr/local/go/src/runtime/asm_amd64.s:625 +0x48
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:339 +0xd7
  github.com/stretchr/testify/suite.Run.func1()
      /Users/cbuto/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:158 +0x71c
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47

Goroutine 57 (running) created at:
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.NewMultiTenantServer()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server.go:170 +0xa77
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).SetupSuite()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:287 +0x1fba
  github.com/stretchr/testify/suite.Run()
      /Users/cbuto/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:118 +0x5b7
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.TestMultiTenantServerTestSuite()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:1153 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47

Goroutine 110 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1306 +0x726
  github.com/stretchr/testify/suite.runTests()
      /Users/cbuto/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:203 +0x18f
  github.com/stretchr/testify/suite.Run()
      /Users/cbuto/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:176 +0x994
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.TestMultiTenantServerTestSuite()
      /Users/cbuto/Documents/projects/cbuto/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:1153 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
==================

[scbizu]

Yeah , it races at startEventListener while we operate the internal cache , the similar situation is uploading chart while CM is not fully start , the possible fix is refactor map[string]*cacheEntry to sync.map based structure , for further maintenance , I think it is a good idea XD .

[nerdeveloper]

/assign

[nerdeveloper]

Hey, @cbuto
Please, I am trying to reproduce this error. Do you mind sharing the commands used?

[cbuto]

hey 👋 @nerdeveloper, yep! Just add the -race to the scripts/test.sh script and run make test.

https://github.com/helm/chartmuseum/blob/main/scripts/test.sh#L11

[cbuto]

@scbizu sync.Map will likely work, it might be worth seeing if a RWMutex offers better performance as well!

[nerdeveloper]

Please in the makefile CGO_ENABLED is set to 0 and -race needs it to be enabled, how did you do this?

[cbuto]

Ah yep, you can set it before the command CGO_ENABLED=1 go test or export it.

[nerdeveloper]

this is the PR to this fix: #549

[scbizu]

Why -race needs cgo , we can see golang/go#9918 .

[nerdeveloper]

@scbizu Please refer to this

#546 (comment)

if we don't set the CGO, it does not run with the race flag