feat(storage): provider interface + Exec abstraction (RealExec / FakeExec)

kvaps · claude · kvaps · commit 053e6c1cc641 · 2026-05-08T00:57:22.000+02:00
Phase 3 fourth slice. Lays the foundation for the LVM/ZFS/file backends
without committing to any one of them yet.

- pkg/storage/storage.go: Provider interface (CreateVolume, DeleteVolume,
  VolumeStatus, CreateSnapshot, DeleteSnapshot, PoolStatus, Kind) plus
  Volume/Snapshot/VolumeStatus/PoolStatus value types and ErrNotFound /
  ErrAlreadyExists sentinels. The contract is reconcile-friendly:
  CreateVolume is idempotent, DeleteVolume swallows ErrNotFound.
- pkg/storage/exec.go: Exec interface + RealExec (production, wraps
  os/exec.CommandContext) + FakeExec (test, records calls, canned
  responses). Stderr folds into the wrapped error so the original chain
  survives; tests can ContainsAll-assert against CommandLines().
- pkg/storage/exec_test.go: 6 cases — records, canned-stdout, canned-err,
  empty-default, joined CommandLines, Reset clears Calls but keeps
  Responses.

Next slice plugs the first real provider (LVM-thin) into this seam.

Co-Authored-By: Claude &lt;noreply@anthropic.com&gt;
Signed-off-by: Andrei Kvapil &lt;kvapss@gmail.com&gt;
diff --git a/pkg/storage/exec.go b/pkg/storage/exec.go
@@ -0,0 +1,149 @@
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package storage
+
+import (
+	"bytes"
+	"context"
+	"os/exec"
+	"strings"
+	"sync"
+
+	"github.com/cockroachdb/errors"
+)
+
+// Exec is a context-aware shell-out abstraction. Production uses
+// RealExec which wraps os/exec; tests substitute FakeExec to assert what
+// was called and inject canned output without root.
+type Exec interface {
+	// Run invokes name with the given args under the supplied context
+	// and returns combined stdout+stderr. Non-zero exit codes turn into
+	// non-nil errors that wrap the original exec error.
+	Run(ctx context.Context, name string, args ...string) ([]byte, error)
+}
+
+// RealExec is the production implementation backed by os/exec.
+type RealExec struct{}
+
+// Run executes name with args, capturing stdout. Stderr is folded into
+// the returned error so callers can include it in surfaced diagnostics
+// without losing the original error chain.
+func (RealExec) Run(ctx context.Context, name string, args ...string) ([]byte, error) {
+	cmd := exec.CommandContext(ctx, name, args...)
+
+	var (
+		stdout bytes.Buffer
+		stderr bytes.Buffer
+	)
+
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err := cmd.Run()
+	if err != nil {
+		return stdout.Bytes(), errors.Wrapf(err, "%s %s: %s",
+			name, strings.Join(args, " "), strings.TrimSpace(stderr.String()))
+	}
+
+	return stdout.Bytes(), nil
+}
+
+// FakeExec is the test implementation. Programmes can pre-register
+// canned responses via Expect and inspect Calls afterwards.
+type FakeExec struct {
+	mu sync.Mutex
+
+	// Calls records every Run invocation in order.
+	Calls []FakeCall
+
+	// Responses maps "name arg1 arg2 …" → canned output.
+	// Falling back to empty stdout + nil error if absent.
+	Responses map[string]FakeResponse
+}
+
+// FakeCall records one invocation of FakeExec.Run.
+type FakeCall struct {
+	Name string
+	Args []string
+}
+
+// FakeResponse is the canned output for a Responses entry.
+type FakeResponse struct {
+	Stdout []byte
+	Err    error
+}
+
+// NewFakeExec returns a FakeExec ready for use.
+func NewFakeExec() *FakeExec {
+	return &FakeExec{Responses: map[string]FakeResponse{}}
+}
+
+// Run looks up a pre-registered response (full command line) and falls
+// back to an empty success if none was registered.
+func (f *FakeExec) Run(_ context.Context, name string, args ...string) ([]byte, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.Calls = append(f.Calls, FakeCall{Name: name, Args: append([]string(nil), args...)})
+
+	key := name
+	if len(args) > 0 {
+		key += " " + strings.Join(args, " ")
+	}
+
+	if resp, ok := f.Responses[key]; ok {
+		return resp.Stdout, resp.Err
+	}
+
+	return nil, nil
+}
+
+// Expect registers a canned response. Match is exact on the command line.
+func (f *FakeExec) Expect(cmdline string, resp FakeResponse) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.Responses[cmdline] = resp
+}
+
+// CommandLines returns the recorded calls as space-joined command lines —
+// convenient for ContainsAll-style assertions in tests.
+func (f *FakeExec) CommandLines() []string {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	out := make([]string, 0, len(f.Calls))
+	for i := range f.Calls {
+		line := f.Calls[i].Name
+		if len(f.Calls[i].Args) > 0 {
+			line += " " + strings.Join(f.Calls[i].Args, " ")
+		}
+
+		out = append(out, line)
+	}
+
+	return out
+}
+
+// Reset clears recorded calls but keeps registered responses. Useful in
+// table-driven tests where each row reuses the same FakeExec.
+func (f *FakeExec) Reset() {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.Calls = nil
+}
diff --git a/pkg/storage/exec_test.go b/pkg/storage/exec_test.go
@@ -0,0 +1,134 @@
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package storage_test
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/cozystack/blockstor/pkg/storage"
+)
+
+// TestFakeExecRecordsCalls: every Run invocation appears in Calls in order.
+func TestFakeExecRecordsCalls(t *testing.T) {
+	fx := storage.NewFakeExec()
+
+	_, _ = fx.Run(t.Context(), "lvs")
+	_, _ = fx.Run(t.Context(), "lvcreate", "-T", "vg/pool", "-V", "1G", "-n", "vol")
+
+	if len(fx.Calls) != 2 {
+		t.Fatalf("Calls len: got %d, want 2", len(fx.Calls))
+	}
+
+	if fx.Calls[1].Name != "lvcreate" {
+		t.Errorf("Calls[1].Name: got %q, want lvcreate", fx.Calls[1].Name)
+	}
+
+	want := []string{"-T", "vg/pool", "-V", "1G", "-n", "vol"}
+	if strings.Join(fx.Calls[1].Args, ",") != strings.Join(want, ",") {
+		t.Errorf("Calls[1].Args: got %v, want %v", fx.Calls[1].Args, want)
+	}
+}
+
+// TestFakeExecCannedResponse: Expect returns the registered stdout/error.
+func TestFakeExecCannedResponse(t *testing.T) {
+	fx := storage.NewFakeExec()
+	fx.Expect("lvs --noheadings -o lv_name vg", storage.FakeResponse{
+		Stdout: []byte("vol1\nvol2\n"),
+	})
+
+	out, err := fx.Run(t.Context(), "lvs", "--noheadings", "-o", "lv_name", "vg")
+	if err != nil {
+		t.Fatalf("Run: %v", err)
+	}
+
+	if !strings.Contains(string(out), "vol1") {
+		t.Errorf("stdout: got %q, want to contain vol1", out)
+	}
+}
+
+// errFakeVGMissing is a static sentinel; err113 forbids ad-hoc errors.New
+// in tests so we declare it once here.
+var errFakeVGMissing = errors.New("volume group \"vg\" not found")
+
+// TestFakeExecCannedError: Expect can return an error, callers must
+// surface it.
+func TestFakeExecCannedError(t *testing.T) {
+	fx := storage.NewFakeExec()
+	fx.Expect("vgs vg", storage.FakeResponse{Err: errFakeVGMissing})
+
+	_, err := fx.Run(t.Context(), "vgs", "vg")
+	if !errors.Is(err, errFakeVGMissing) {
+		t.Errorf("Run err: got %v, want %v", err, errFakeVGMissing)
+	}
+}
+
+// TestFakeExecEmptyDefault: no Expect → empty stdout, nil error.
+func TestFakeExecEmptyDefault(t *testing.T) {
+	fx := storage.NewFakeExec()
+
+	out, err := fx.Run(t.Context(), "anything")
+	if err != nil {
+		t.Errorf("default err: got %v, want nil", err)
+	}
+
+	if len(out) != 0 {
+		t.Errorf("default stdout: got %q, want empty", out)
+	}
+}
+
+// TestFakeExecCommandLinesJoinsArgs: convenience accessor returns joined
+// command lines for ContainsAll-style assertions.
+func TestFakeExecCommandLinesJoinsArgs(t *testing.T) {
+	fx := storage.NewFakeExec()
+
+	_, _ = fx.Run(t.Context(), "lvremove", "-f", "vg/vol")
+	_, _ = fx.Run(t.Context(), "vgs")
+
+	got := fx.CommandLines()
+	if len(got) != 2 {
+		t.Fatalf("len: got %d", len(got))
+	}
+
+	if got[0] != "lvremove -f vg/vol" {
+		t.Errorf("[0]: got %q", got[0])
+	}
+
+	if got[1] != "vgs" {
+		t.Errorf("[1]: got %q", got[1])
+	}
+}
+
+// TestFakeExecReset clears recorded calls but keeps Responses.
+func TestFakeExecReset(t *testing.T) {
+	fx := storage.NewFakeExec()
+	fx.Expect("lvs", storage.FakeResponse{Stdout: []byte("ok")})
+
+	_, _ = fx.Run(t.Context(), "lvs")
+
+	fx.Reset()
+
+	if len(fx.Calls) != 0 {
+		t.Errorf("Calls: got %d, want 0 after Reset", len(fx.Calls))
+	}
+
+	out, _ := fx.Run(t.Context(), "lvs")
+	if string(out) != "ok" {
+		t.Errorf("Responses lost across Reset: got %q", out)
+	}
+}
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
@@ -0,0 +1,107 @@
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package storage defines the satellite-side storage provider contract.
+// Implementations live under pkg/storage/{lvm,zfs,file} and translate
+// Volume / Snapshot intent into shell-out calls (lvcreate, zfs create,
+// dd, ...) wrapped behind the Exec interface so unit tests can drive
+// them without root or real block devices.
+package storage
+
+import (
+	"context"
+
+	"github.com/cockroachdb/errors"
+)
+
+// Sentinel errors returned by Provider implementations. REST handlers and
+// reconcilers map these to HTTP statuses / Status conditions.
+var (
+	// ErrNotFound — the named volume/snapshot/pool does not exist on
+	// this node.
+	ErrNotFound = errors.New("storage object not found")
+	// ErrAlreadyExists — Create called on an object that already exists.
+	ErrAlreadyExists = errors.New("storage object already exists")
+)
+
+// Volume identifies a block-level volume on a satellite. The triple
+// (PoolName, ResourceName, VolumeNumber) uniquely names it; the
+// implementation maps that to a backend-specific path (LVM LV, ZFS
+// dataset, file).
+type Volume struct {
+	PoolName       string
+	ResourceName   string
+	VolumeNumber   int32
+	SizeKib        int64
+	StoragePoolDir string // for FILE providers
+}
+
+// Snapshot is a captured-at-a-point-in-time copy of a Volume on the same
+// node. Snapshot shipping (intra-cluster clone / replica expansion) lives
+// on the Provider too because zfs/thin tooling differ per provider.
+type Snapshot struct {
+	PoolName     string
+	ResourceName string
+	SnapshotName string
+}
+
+// VolumeStatus is the observed state the satellite reports back to the
+// controller. Empty DevicePath means the volume is not yet provisioned.
+type VolumeStatus struct {
+	DevicePath   string
+	AllocatedKib int64
+	UsableKib    int64
+	State        string // PROVISIONED / ERROR / NOT_PROVISIONED
+}
+
+// PoolStatus mirrors `linstor sp l` output for one pool on this node.
+type PoolStatus struct {
+	FreeCapacityKib   int64
+	TotalCapacityKib  int64
+	SupportsSnapshots bool
+}
+
+// Provider is the per-storage-kind interface every backend implements.
+// Implementations are constructed once per pool by the satellite agent
+// and re-used across reconcile cycles.
+type Provider interface {
+	// Kind returns the LINSTOR provider kind string (e.g. "LVM_THIN").
+	Kind() string
+
+	// PoolStatus reports free/total capacity and snapshot capability.
+	// Used by the satellite's per-pool reporter.
+	PoolStatus(ctx context.Context) (PoolStatus, error)
+
+	// CreateVolume materialises the volume on disk. Idempotent: if the
+	// volume already exists with the same size, returns nil.
+	CreateVolume(ctx context.Context, vol Volume) error
+
+	// DeleteVolume removes the volume. Idempotent: ErrNotFound is
+	// silently swallowed so repeated reconciles converge.
+	DeleteVolume(ctx context.Context, vol Volume) error
+
+	// VolumeStatus reports observed state. DevicePath empty + State
+	// NOT_PROVISIONED means the volume hasn't been created yet.
+	VolumeStatus(ctx context.Context, vol Volume) (VolumeStatus, error)
+
+	// CreateSnapshot captures the volume at a point in time. The
+	// implementation chooses the on-disk representation (LV snapshot,
+	// zfs snapshot, COW copy).
+	CreateSnapshot(ctx context.Context, snap Snapshot) error
+
+	// DeleteSnapshot is the inverse of CreateSnapshot.
+	DeleteSnapshot(ctx context.Context, snap Snapshot) error
+}
