pkg/config: make EditConnectionConfig race free

Add a lock to EditConnectionConfig to make it race free for parallel
modification. It is possible that several podman system connection
commands are called simultaneously so we should read/file in a
consistent way to ensure no modifications are "silently dropped".

A test is added that reliably fails without the lock.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This commit is contained in:
Paul Holzinger 2024-03-04 16:24:42 +01:00
parent 969e9c04a0
commit a94f7e6eb0
2 changed files with 49 additions and 0 deletions

View File

@ -9,6 +9,7 @@ import (
"path/filepath" "path/filepath"
"github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/lockfile"
) )
const connectionsFile = "podman-connections.json" const connectionsFile = "podman-connections.json"
@ -113,6 +114,15 @@ func EditConnectionConfig(callback func(cfg *ConnectionsFile) error) error {
if err != nil { if err != nil {
return err return err
} }
lockPath := path + ".lock"
lock, err := lockfile.GetLockFile(lockPath)
if err != nil {
return fmt.Errorf("obtain lock file: %w", err)
}
lock.Lock()
defer lock.Unlock()
conf, err := readConnectionConf(path) conf, err := readConnectionConf(path)
if err != nil { if err != nil {
return fmt.Errorf("read connections file: %w", err) return fmt.Errorf("read connections file: %w", err)

View File

@ -3,6 +3,8 @@ package config
import ( import (
"os" "os"
"path/filepath" "path/filepath"
"strconv"
"sync"
. "github.com/onsi/ginkgo/v2" . "github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega" "github.com/onsi/gomega"
@ -73,6 +75,43 @@ var _ = Describe("Connections conf", func() {
gomega.Expect(conf).To(gomega.Equal(orgConf)) gomega.Expect(conf).To(gomega.Equal(orgConf))
}) })
It("parallel EditConnectionConfig", func() {
// race test for EditConnectionConfig
// Basic idea spawn a bunch of goroutines and call EditConnectionConfig at the same time.
// We read a int from one field and then +1 one it each time so at the end we must have
// the number in the filed for how many times we called EditConnectionConfig. If it is
// less than it is racy.
count := 50
wg := sync.WaitGroup{}
wg.Add(count)
for i := 0; i < count; i++ {
go func() {
defer wg.Done()
err := EditConnectionConfig(func(cfg *ConnectionsFile) error {
if cfg.Connection.Default == "" {
cfg.Connection.Default = "1"
return nil
}
// basic idea just add 1
i, err := strconv.Atoi(cfg.Connection.Default)
if err != nil {
return err
}
i++
cfg.Connection.Default = strconv.Itoa(i)
return nil
})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
}()
}
wg.Wait()
path, err := connectionsConfigFile()
gomega.Expect(err).ToNot(gomega.HaveOccurred())
conf, err := readConnectionConf(path)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
gomega.Expect(conf.Connection.Default).To(gomega.Equal("50"))
})
Context("GetConnection/Farm", func() { Context("GetConnection/Farm", func() {
const defConnectionsConf = `{"Connection":{"Default":"test","Connections":{"test":{"URI":"ssh://podman.io"},"QA":{"URI":"ssh://test","Identity":".ssh/id","IsMachine":true}}},"farm":{"Default":"farm1","List":{"farm1":["test"]}}}` const defConnectionsConf = `{"Connection":{"Default":"test","Connections":{"test":{"URI":"ssh://podman.io"},"QA":{"URI":"ssh://test","Identity":".ssh/id","IsMachine":true}}},"farm":{"Default":"farm1","List":{"farm1":["test"]}}}`
const defContainersConf = ` const defContainersConf = `