Merge pull request #1816 from twz123/atomic-manifest-writes

Write manifests atomically

go.mod

@@ -45,6 +45,7 @@ require (
 	go.etcd.io/etcd/client/pkg/v3 v3.5.4
 	go.etcd.io/etcd/client/v3 v3.5.4
 	go.etcd.io/etcd/etcdutl/v3 v3.5.4
+	go.uber.org/multierr v1.8.0
 	go.uber.org/zap v1.23.0
 	golang.org/x/crypto v0.0.0-20220824171710-5757bc0c5503
 	golang.org/x/exp v0.0.0-20220827204233-334a2380cb91
@@ -258,7 +259,6 @@ require (
 	go.opentelemetry.io/proto/otlp v0.11.0 // indirect
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
-	go.uber.org/multierr v1.8.0 // indirect
 	golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
 	golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect
 	golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b // indirect

internal/pkg/file/file.go

@@ -18,9 +18,13 @@ package file
 
 import (
 	"fmt"
+	"io"
 	"os"
+	"path/filepath"
 
 	"github.com/k0sproject/k0s/internal/pkg/users"
+
+	"go.uber.org/multierr"
 )
 
 // Exists checks if a file exists and is not a directory before we
@@ -71,15 +75,89 @@ func Copy(src, dst string) error {
 	return nil
 }
 
+// WriteAtomically will atomically create or replace a file. The contents of the
+// file will be those that the write callback writes to the Writer that gets
+// passed in. The Writer will be unbuffered. WriteAtomically will buffer the
+// contents in a hidden (i.e. its name will start with a dot), temporary file
+// (it will have a .tmp extension). When write returns without an error, the
+// temporary file will be renamed to fileName, otherwise it will be deleted
+// without touching the target file.
+//
+// Note that this function is only best-effort on Windows:
+// https://github.com/golang/go/issues/22397#issuecomment-498856679
+func WriteAtomically(fileName string, perm os.FileMode, write func(file io.Writer) error) error {
+	var fd *os.File
+	fd, err := os.CreateTemp(filepath.Dir(fileName), fmt.Sprintf(".%s.*.tmp", filepath.Base(fileName)))
+	if err != nil {
+		return err
+	}
+
+	tmpFileName := fd.Name()
+	close := true
+	defer func() {
+		remove := err != nil
+		if close {
+			err = multierr.Append(err, fd.Close())
+		}
+		if remove {
+			err = multierr.Append(err, os.Remove(tmpFileName))
+		}
+	}()
+
+	err = write(fd)
+	if err != nil {
+		return err
+	}
+
+	// https://github.com/google/renameio/blob/v2.0.0/tempfile.go#L150-L157
+	err = fd.Sync()
+	if err != nil {
+		return err
+	}
+
+	err = fd.Close()
+	close = false
+	if err != nil {
+		return err
+	}
+
+	err = os.Chmod(tmpFileName, perm)
+	if err != nil {
+		return err
+	}
+
+	err = os.Rename(tmpFileName, fileName)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// WriteContentAtomically will atomically create or replace a file with the
+// given content. WriteContentAtomically will create a hidden (i.e. its name
+// will start with a dot), temporary file (it will have a .tmp extension) with
+// the given content. Afterwards, the temporary file will be renamed to
+// fileName, otherwise it will be deleted without touching the target file.
+//
+// Note that this function is only best-effort on Windows:
+// https://github.com/golang/go/issues/22397#issuecomment-498856679
+func WriteContentAtomically(fileName string, content []byte, perm os.FileMode) error {
+	return WriteAtomically(fileName, perm, func(file io.Writer) error {
+		_, err := file.Write(content)
+		return err
+	})
+}
+
 func WriteTmpFile(data string, prefix string) (path string, err error) {
 	tmpFile, err := os.CreateTemp("", prefix)
 	if err != nil {
-		return "", fmt.Errorf("cannot create temporary file: %v", err)
+		return "", fmt.Errorf("cannot create temporary file: %w", err)
 	}
 
 	text := []byte(data)
 	if _, err = tmpFile.Write(text); err != nil {
-		return "", fmt.Errorf("failed to write to temporary file: %v", err)
+		return "", fmt.Errorf("failed to write to temporary file: %w", err)
 	}
 
 	return tmpFile.Name(), nil

internal/pkg/file/file_test.go

@@ -18,7 +18,6 @@ package file
 
 import (
 	"os"
-	"path"
 	"path/filepath"
 	"testing"
 
@@ -36,7 +35,7 @@ func TestExists(t *testing.T) {
 		t.Errorf("test non-existing: got %t, wanted %t", got, want)
 	}
 
-	existingFileName := path.Join(dir, "existing")
+	existingFileName := filepath.Join(dir, "existing")
 	require.NoError(t, os.WriteFile(existingFileName, []byte{}, 0644))
 
 	// test existing

internal/pkg/templatewriter/templatewriter.go

@@ -19,11 +19,11 @@ package templatewriter
 import (
 	"fmt"
 	"io"
-	"os"
 	"text/template"
 
 	"github.com/Masterminds/sprig"
 
+	"github.com/k0sproject/k0s/internal/pkg/file"
 	"github.com/k0sproject/k0s/pkg/constant"
 )
 
@@ -35,14 +35,9 @@ type TemplateWriter struct {
 	Path     string
 }
 
-// Write writes executes the template and writes the results on disk
+// Write executes the template and writes the results on disk
 func (p *TemplateWriter) Write() error {
-	podFile, err := os.OpenFile(p.Path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, constant.CertMode)
-	if err != nil {
-		return fmt.Errorf("failed to open pod file for %s: %w", p.Name, err)
-	}
-	defer podFile.Close()
-	return p.WriteToBuffer(podFile)
+	return file.WriteAtomically(p.Path, constant.CertMode, p.WriteToBuffer)
 }
 
 // WriteToBuffer writes executed template tot he given writer

pkg/applier/applier.go

@@ -24,7 +24,8 @@ import (
 	"path"
 	"path/filepath"
 
-	"github.com/sirupsen/logrus"
+	"github.com/k0sproject/k0s/pkg/kubernetes"
+
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/cli-runtime/pkg/resource"
@@ -33,7 +34,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
 
-	"github.com/k0sproject/k0s/pkg/kubernetes"
+	"github.com/sirupsen/logrus"
 )
 
 // manifestFilePattern is the glob pattern that all applicable manifest files need to match.
@@ -105,10 +106,12 @@ func (a *Applier) Apply(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
+
 	files, err := filepath.Glob(path.Join(a.Dir, manifestFilePattern))
 	if err != nil {
 		return err
 	}
+
 	resources, err := a.parseFiles(files)
 	if err != nil {
 		return err

pkg/applier/stackapplier.go

@@ -108,19 +108,17 @@ func (s *StackApplier) Run(ctx context.Context) error {
 }
 
 func (*StackApplier) triggersApply(event fsnotify.Event) bool {
-	// always let the initial apply happen
+	// Always let the initial apply happen
 	if event == (fsnotify.Event{}) {
 		return true
 	}
 
-	// ignore chmods (3845479a0)
-	if event.Op == fsnotify.Chmod {
+	// Only consider events on manifest files
+	if match, _ := filepath.Match(manifestFilePattern, filepath.Base(event.Name)); !match {
 		return false
 	}
 
-	// Only consider events on manifest files
-	match, _ := filepath.Match(manifestFilePattern, filepath.Base(event.Name))
-	return match
+	return true
 }
 
 func (s *StackApplier) apply(ctx context.Context) {

pkg/component/controller/manifests.go

@@ -19,11 +19,10 @@ package controller
 import (
 	"crypto/md5"
 	"fmt"
-	"os"
-	"path"
 	"path/filepath"
 
 	"github.com/k0sproject/k0s/internal/pkg/dir"
+	"github.com/k0sproject/k0s/internal/pkg/file"
 	"github.com/k0sproject/k0s/pkg/constant"
 	"github.com/sirupsen/logrus"
 )
@@ -36,10 +35,12 @@ type FsManifestsSaver struct {
 // Save saves given manifest under the given path
 func (f FsManifestsSaver) Save(dst string, content []byte) error {
 	target := filepath.Join(f.dir, dst)
-	if err := os.WriteFile(target, content, constant.ManifestsDirMode); err != nil {
-		return fmt.Errorf("can't write manifest %s: %v", target, err)
+
+	if err := file.WriteContentAtomically(target, content, constant.CertMode); err != nil {
+		return err
 	}
-	logrus.WithField("component", "manifest-saver").Debugf("succesfully wrote %s:%s", target, hash(content))
+
+	logrus.WithField("component", "manifest-saver").Debugf("Successfully wrote %s:%s", target, hash(content))
 	return nil
 }
 
@@ -49,7 +50,7 @@ func hash(data []byte) string {
 
 // NewManifestsSaver builds new filesystem manifests saver
 func NewManifestsSaver(manifest string, dataDir string) (*FsManifestsSaver, error) {
-	manifestDir := path.Join(dataDir, "manifests", manifest)
+	manifestDir := filepath.Join(dataDir, "manifests", manifest)
 	err := dir.Init(manifestDir, constant.ManifestsDirMode)
 	if err != nil {
 		return nil, err