Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write manifests atomically #1816

Merged
merged 1 commit into from
Oct 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
82 changes: 80 additions & 2 deletions internal/pkg/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions internal/pkg/file/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package file

import (
"os"
"path"
"path/filepath"
"testing"

Expand All @@ -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
Expand Down
11 changes: 3 additions & 8 deletions internal/pkg/templatewriter/templatewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions pkg/applier/applier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions pkg/applier/stackapplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
13 changes: 7 additions & 6 deletions pkg/component/controller/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}

Expand All @@ -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
Expand Down