Merge branch 'develop' into bolt/opencost-core

core/pkg/opencost/window_test.go

@@ -672,13 +672,17 @@ func TestWindow_DurationOffsetForPrometheus(t *testing.T) {
 	}
 
 	dur, off, err := w.DurationOffsetForPrometheus()
-	expDur := int(now.Sub(startOfToday).Seconds())
-	expDurStr := fmt.Sprintf("%ds", expDur)
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
-	if dur != expDurStr {
-		t.Fatalf(`expect: window to be "%s"; actual: "%s"`, expDurStr, dur)
+	// We can get a response in seconds OR minutes. Check seconds first as it
+	// is higher resolution.
+	expDurSec := int(now.Sub(startOfToday).Seconds())
+	expDurSecStr := fmt.Sprintf("%ds", expDurSec)
+	expDurMin := int(now.Sub(startOfToday).Minutes())
+	expDurMinStr := fmt.Sprintf("%dm", expDurMin)
+	if dur != expDurSecStr && dur != expDurMinStr {
+		t.Fatalf(`expect: window to be "%s" (or "%s"); actual: "%s"`, expDurSecStr, expDurMinStr, dur)
 	}
 	if off != "" {
 		t.Fatalf(`expect: offset to be ""; actual: "%s"`, off)

pkg/prom/ratelimitedclient_test.go

@@ -156,6 +156,9 @@ func TestRateLimitedOnceAndSuccess(t *testing.T) {
 
 	// we just need to execute this  once to see retries in effect
 	res, body, err := client.Do(context.Background(), req)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if res.StatusCode != 200 {
 		t.Fatalf("200 StatusCode expected. Got: %d", res.StatusCode)
@@ -198,6 +201,9 @@ func TestRateLimitedOnceAndFail(t *testing.T) {
 
 	// we just need to execute this  once to see retries in effect
 	res, body, err := client.Do(context.Background(), req)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if res.StatusCode != 400 {
 		t.Fatalf("400 StatusCode expected. Got: %d", res.StatusCode)

pkg/util/fileutil/locks_test.go

@@ -0,0 +1,108 @@
+package fileutil
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// Make sure read works on file created without locking logic
+func TestReadLocked(t *testing.T) {
+	toWrite := "hello world"
+	dir := t.TempDir()
+	filename := filepath.Join(dir, "test.txt")
+	if err := os.WriteFile(filename, []byte(toWrite), 0600); err != nil {
+		t.Fatalf("failed to write test data: %s", err)
+	}
+
+	read, err := ReadLocked(filename)
+	if err != nil {
+		t.Fatalf("Failed to read: %s", err)
+	}
+	sread := string(read)
+
+	if toWrite != sread {
+		t.Errorf("Expected read data to be '%s' but was '%s'", toWrite, sread)
+	}
+}
+
+// Does not test concurrency, just makes sure the basic read write functionality
+// works
+func TestRWLocked(t *testing.T) {
+	toWrite := "hello world"
+	dir := t.TempDir()
+	filename := filepath.Join(dir, "test.txt")
+
+	if _, err := WriteLocked(filename, []byte(toWrite)); err != nil {
+		t.Fatalf("Failed to write: %s", err)
+	}
+
+	read, err := ReadLocked(filename)
+	if err != nil {
+		t.Fatalf("Failed to read: %s", err)
+	}
+	sread := string(read)
+
+	if toWrite != sread {
+		t.Errorf("Expected read data to be '%s' but was '%s'", toWrite, sread)
+	}
+}
+
+func TestReadLockedFDMiddlePosition(t *testing.T) {
+	toWrite := "hello world"
+	dir := t.TempDir()
+	filename := filepath.Join(dir, "test.txt")
+	if err := os.WriteFile(filename, []byte(toWrite), 0600); err != nil {
+		t.Fatalf("failed to write test data: %s", err)
+	}
+
+	f, err := os.Open(filename)
+	if err != nil {
+		t.Fatalf("opening after write: %s", err)
+	}
+	if _, err := f.Seek(3, 0); err != nil {
+		t.Fatalf("seeking: %s", err)
+	}
+
+	read, err := ReadLockedFD(f)
+	if err != nil {
+		t.Fatalf("Failed to read: %s", err)
+	}
+	sread := string(read)
+
+	if toWrite != sread {
+		t.Errorf("Expected read data to be '%s' but was '%s'", toWrite, sread)
+	}
+}
+
+func TestWriteLockedFDMiddlePosition(t *testing.T) {
+	toWrite := "hello world"
+	toWriteOver := "goodbye land"
+	dir := t.TempDir()
+	filename := filepath.Join(dir, "test.txt")
+	if err := os.WriteFile(filename, []byte(toWrite), 0600); err != nil {
+		t.Fatalf("failed to write test data: %s", err)
+	}
+
+	f, err := os.OpenFile(filename, os.O_RDWR, 0600)
+	if err != nil {
+		t.Fatalf("opening after write: %s", err)
+	}
+	if _, err := f.Seek(3, 0); err != nil {
+		t.Fatalf("seeking: %s", err)
+	}
+
+	if _, err := WriteLockedFD(f, []byte(toWriteOver)); err != nil {
+		t.Fatalf("writing over: %s", err)
+	}
+
+	read, err := ReadLockedFD(f)
+	if err != nil {
+		t.Fatalf("Failed to read: %s", err)
+	}
+	sread := string(read)
+
+	if toWriteOver != sread {
+		t.Errorf("Expected read data to be '%s' but was '%s'", toWriteOver, sread)
+	}
+}

pkg/util/fileutil/locks_unix.go

@@ -0,0 +1,124 @@
+//go:build darwin || dragonfly || freebsd || illumos || linux || netbsd || openbsd
+
+// The above platforms support flock()
+
+package fileutil
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+	"syscall"
+
+	"github.com/opencost/opencost/pkg/log"
+)
+
+// WriteLockedFD uses the flock() syscall to safely write to an open file as
+// long as other users of the file are also using flock()-based access.
+//
+// WriteLocked will block until it gets lock access.
+//
+// The file will be truncated before writing and at the end of writing the
+// FD will be reset to position 0.
+//
+// For the reasons outlined best in https://lwn.net/Articles/586904/ this uses
+// flock() instead of fcntl(). The ability to lock byte ranges is not necessary
+// and flock() has better behavior.
+func WriteLockedFD(f *os.File, data []byte) (int, error) {
+	// For the reasons outlined best in https://lwn.net/Articles/586904/ we're
+	// going to use flock() instead of fcntl() because we want a whole-file lock.
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
+		return 0, fmt.Errorf("unexpected error flock()-ing with EX: %w", err)
+	}
+	defer func() {
+		if err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN); err != nil {
+			log.Errorf("unexpected error flock()-ing FD %d with UN after writing: %s", f.Fd(), err)
+		}
+	}()
+
+	if err := f.Truncate(0); err != nil {
+		return 0, fmt.Errorf("truncating: %w", err)
+	}
+
+	if _, err := f.Seek(0, 0); err != nil {
+		return 0, fmt.Errorf("seeking to 0 before write: %w", err)
+	}
+	defer func() {
+		if _, err := f.Seek(0, 0); err != nil {
+			log.Errorf("unexpected error seeking to 0 after write on FD %d: %s", f.Fd(), err)
+		}
+	}()
+
+	n, err := f.Write(data)
+	if err != nil {
+		return n, fmt.Errorf("writing data: %w", err)
+	}
+
+	return n, nil
+}
+
+// WriteLocked opens the file and then calls WriteLockedFD.
+func WriteLocked(filename string, data []byte) (int, error) {
+	file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0600)
+	if err != nil {
+		return 0, fmt.Errorf("opening %s: %w", filename, err)
+	}
+	defer file.Close()
+
+	return WriteLockedFD(file, data)
+}
+
+// ReadLockedFD uses the flock() syscall to safely read from an open file as
+// long as other users of the file are also using flock()-based access.
+//
+// ReadLockedFD will block until it gets lock access.
+//
+// This will read the file in full, from 0, regardless of the current
+// position and then reset the position to 0.
+//
+// For the reasons outlined best in https://lwn.net/Articles/586904/ this uses
+// flock() instead of fcntl(). The ability to lock byte ranges is not necessary
+// and flock() has better behavior.
+func ReadLockedFD(f *os.File) ([]byte, error) {
+	// For the reasons outlined best in https://lwn.net/Articles/586904/ we're
+	// going to use flock() instead of fcntl() because we want a whole-file lock.
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_SH); err != nil {
+		return nil, fmt.Errorf("unexpected error flock()-ing with SH: %w", err)
+	}
+	defer func() {
+		if err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN); err != nil {
+			log.Errorf("unexpected error flock()-ing FD %d with UN reading: %s", f.Fd(), err)
+		}
+	}()
+
+	if _, err := f.Seek(0, 0); err != nil {
+		return nil, fmt.Errorf("seeking to 0 before read: %w", err)
+	}
+	defer func() {
+		if _, err := f.Seek(0, 0); err != nil {
+			log.Errorf("unexpected error seeking to 0 after read on FD %d: %s", f.Fd(), err)
+		}
+	}()
+
+	buf := bytes.NewBuffer(nil)
+	if _, err := io.Copy(buf, f); err != nil {
+		if err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN); err != nil {
+			log.Errorf("unexpected error flock()-ing with UN after error reading: %s", err)
+		}
+		return nil, fmt.Errorf("copying data out of file: %w", err)
+	}
+
+	return buf.Bytes(), nil
+}
+
+// ReadLocked opens the given file and then calls ReadLockedFD.
+func ReadLocked(filename string) ([]byte, error) {
+	file, err := os.OpenFile(filename, os.O_RDONLY, 0600)
+	if err != nil {
+		return nil, fmt.Errorf("opening %s: %w", filename, err)
+	}
+	defer file.Close()
+
+	return ReadLockedFD(file)
+}

pkg/util/fileutil/locks_windows.go

@@ -0,0 +1,24 @@
+//go:build windows
+
+package fileutil
+
+import (
+	"fmt"
+	"os"
+)
+
+func WriteLockedFD(f *os.File, data []byte) (int, error) {
+	return 0, fmt.Errorf("WriteLockedFD is not implemented on Windows. Please open an issue.")
+}
+
+func WriteLocked(filename string, data []byte) (int, error) {
+	return 0, fmt.Errorf("WriteLocked is not implemented on Windows. Please open an issue.")
+}
+
+func ReadLockedFD(f *os.File) ([]byte, error) {
+	return nil, fmt.Errorf("ReadLockedFD is not implemented on Windows. Please open an issue.")
+}
+
+func ReadLocked(filename string) ([]byte, error) {
+	return nil, fmt.Errorf("ReadLocked is not implemented on Windows. Please open an issue.")
+}