Skip to content

Commit

Permalink
gfd: fixed logic of atomic writing of the feature file
Browse files Browse the repository at this point in the history
fix #791 Corrected the incorrect order of operations in atomic writing of the feature file, which could lead to a "permission denied" error observed in the nfd-worker logs

note: now the temporary file is created in $TMPDIR instead of /etc/kubernetes/node-feature-discovery/features.d/gfd-tmp/

Signed-off-by: belo4ya <[email protected]>
  • Loading branch information
belo4ya authored and elezar committed Jul 16, 2024
1 parent c6b3907 commit 442edd4
Show file tree
Hide file tree
Showing 11 changed files with 554 additions and 51 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/NVIDIA/go-nvml v0.12.4-0
github.com/NVIDIA/nvidia-container-toolkit v1.16.0
github.com/fsnotify/fsnotify v1.7.0
github.com/google/renameio v1.0.1
github.com/google/uuid v1.6.0
github.com/mittwald/go-helm-client v0.12.9
github.com/onsi/ginkgo/v2 v2.19.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 h1:k7nVchz72niMH6YLQNvHSdIE7iqsQxK1P41mySCvssg=
github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw=
github.com/google/renameio v1.0.1 h1:Lh/jXZmvZxb0BBeSY5VKEfidcbcbenKjZFzM/q0fSeU=
github.com/google/renameio v1.0.1/go.mod h1:t/HQoYBZSsWSNK35C6CO/TpPLDVWvxOHboWUAweKUpk=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
Expand Down
55 changes: 4 additions & 51 deletions internal/lm/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"strings"

apiequality "k8s.io/apimachinery/pkg/api/equality"
Expand All @@ -32,6 +31,8 @@ import (
nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
nfdclientset "sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned"

"github.com/google/renameio"

spec "github.com/NVIDIA/k8s-device-plugin/api/config/v1"
"github.com/NVIDIA/k8s-device-plugin/internal/flags"
)
Expand Down Expand Up @@ -85,8 +86,8 @@ func (path *toFile) Output(labels Labels) error {
if err := output.Output(labels); err != nil {
return fmt.Errorf("error writing labels to buffer: %v", err)
}
err := writeFileAtomically(string(*path), buffer.Bytes(), 0644)
if err != nil {
// write file atomically
if err := renameio.WriteFile(string(*path), buffer.Bytes(), 0644); err != nil {
return fmt.Errorf("error atomically writing file '%s': %w", *path, err)
}
return nil
Expand All @@ -102,54 +103,6 @@ func (output *toWriter) Output(labels Labels) error {
return nil
}

func writeFileAtomically(path string, contents []byte, perm os.FileMode) error {
absPath, err := filepath.Abs(path)
if err != nil {
return fmt.Errorf("failed to retrieve absolute path of output file: %v", err)
}

absDir := filepath.Dir(absPath)
tmpDir := filepath.Join(absDir, "gfd-tmp")

err = os.MkdirAll(tmpDir, os.ModePerm)
if err != nil && !os.IsExist(err) {
return fmt.Errorf("failed to create temporary directory: %v", err)
}
defer func() {
if err != nil {
os.RemoveAll(tmpDir)
}
}()

tmpFile, err := os.CreateTemp(tmpDir, "gfd-")
if err != nil {
return fmt.Errorf("fail to create temporary output file: %v", err)
}
defer func() {
if err != nil {
tmpFile.Close()
os.Remove(tmpFile.Name())
}
}()

err = os.WriteFile(tmpFile.Name(), contents, perm)
if err != nil {
return fmt.Errorf("error writing temporary file '%v': %v", tmpFile.Name(), err)
}

err = os.Rename(tmpFile.Name(), path)
if err != nil {
return fmt.Errorf("error moving temporary file to '%v': %v", path, err)
}

err = os.Chmod(path, perm)
if err != nil {
return fmt.Errorf("error setting permissions on '%v': %v", path, err)
}

return nil
}

const nodeFeatureVendorPrefix = "nvidia-features-for"

type nodeFeatureObject struct {
Expand Down
5 changes: 5 additions & 0 deletions vendor/github.com/google/renameio/.golangci.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions vendor/github.com/google/renameio/CONTRIBUTING.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

202 changes: 202 additions & 0 deletions vendor/github.com/google/renameio/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 61 additions & 0 deletions vendor/github.com/google/renameio/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 442edd4

Please sign in to comment.