From ad5970ce1ef7555e9045c8c0e835a380a4b74f17 Mon Sep 17 00:00:00 2001 From: dougbtv Date: Thu, 28 Mar 2024 15:59:06 -0400 Subject: [PATCH] Configuration writes should be atomic This change introduces a method `atomicFileWrite(...)` which writes to a temporary file and then renames it to avoid partial reads, e.g. ``` Mar 29 04:13:05 labkubedualhost-node-1 crio[33593]: time="2024-03-29 04:13:05.977398607+09:00" level=info msg="CNI monitoring event CREATE \"/etc/cni/net.d/00-multus.conf\"" Mar 29 04:13:05 labkubedualhost-node-1 crio[33593]: time="2024-03-29 04:13:05.977767189+09:00" level=error msg="Error loading CNI config file /etc/cni/net.d/00-multus.conf: error parsing configuration: unexpected end of JSON input" ``` --- pkg/server/config/manager.go | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/pkg/server/config/manager.go b/pkg/server/config/manager.go index 6b450c13a..7e5db30e2 100644 --- a/pkg/server/config/manager.go +++ b/pkg/server/config/manager.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "fmt" + "io/ioutil" "os" "path/filepath" "sync" @@ -64,6 +65,32 @@ func NewManager(config MultusConf) (*Manager, error) { return newManager(config, defaultPluginName) } +// atomicFileWrite writes our configs atomically to avoid partial reads +func atomicFileWrite(filename string, data []byte, perm os.FileMode) error { + // Create a temporary file in the same directory as the destination file + dir := filepath.Dir(filename) + tmpFile, err := ioutil.TempFile(dir, ".*.tmp") + if err != nil { + return err + } + tmpFileName := tmpFile.Name() + + // Always remove that temp file... + defer os.Remove(tmpFileName) + + // Write data to the temporary file + if _, err := tmpFile.Write(data); err != nil { + tmpFile.Close() + return err + } + if err := tmpFile.Close(); err != nil { + return err + } + + // Atomically rename the temporary file to the final filename + return os.Rename(tmpFileName, filename) +} + // overrideCNIVersion overrides cniVersion in cniConfigFile, it should be used only in kind case func overrideCNIVersion(cniConfigFile string, multusCNIVersion string) error { path, err := filepath.Abs(cniConfigFile) @@ -87,7 +114,7 @@ func overrideCNIVersion(cniConfigFile string, multusCNIVersion string) error { return fmt.Errorf("couldn't update cluster network config: %v", err) } - err = os.WriteFile(path, configBytes, 0644) + err = atomicFileWrite(path, configBytes, 0644) if err != nil { return fmt.Errorf("couldn't update cluster network config: %v", err) } @@ -268,7 +295,7 @@ func (m *Manager) PersistMultusConfig(config string) (string, error) { } else { logging.Debugf("Writing Multus CNI configuration @ %s", m.multusConfigFilePath) } - return m.multusConfigFilePath, os.WriteFile(m.multusConfigFilePath, []byte(config), UserRWPermission) + return m.multusConfigFilePath, atomicFileWrite(m.multusConfigFilePath, []byte(config), UserRWPermission) } func (m *Manager) shouldRegenerateConfig(event fsnotify.Event) bool {