Skip to content

Fix W-033 CLI correctness regressions #13

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.

Sign up for GitHub

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

Merged
stenh0use merged 1 commit into from
May 2, 2026
Merged

Fix W-033 CLI correctness regressions #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 11 additions & 11 deletions pkg/cluster/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,16 @@ func (m *Manager) StopWithOptions(ctx context.Context, opts StopOptions) (StopRe

func (m *Manager) Delete(ctx context.Context) error {
existed := m.ConfigFileExists()
if !existed {
return fmt.Errorf("cluster '%s' not found", m.config.Name)
}

if existed {
// Load existing config, else just use the config created by New()
cfg, err := m.loadConfig()
if err != nil {
return fmt.Errorf("failed to load cluster config: %w", err)
}
m.config = cfg
// Load existing config, else just use the config created by New()
cfg, err := m.loadConfig()
if err != nil {
return fmt.Errorf("failed to load cluster config: %w", err)
}
m.config = cfg

// TODO: make delete idempotent

Expand Down Expand Up @@ -321,13 +322,12 @@ func (m *Manager) ConfigFileExists() bool {
}

// LoadPersistedConfig loads cluster configuration from disk when available.
// If no persisted config exists, the current in-memory config is left unchanged.
func (m *Manager) LoadPersistedConfig() error {
if !m.ConfigFileExists() {
if m.config == nil || m.config.Name == "" {
return fmt.Errorf("cluster config not found")
if m.config != nil && m.config.Name != "" {
return fmt.Errorf("cluster '%s' not found", m.config.Name)
}
return nil
return fmt.Errorf("cluster config not found")
}

cfg, err := m.loadConfig()
Expand Down
40 changes: 36 additions & 4 deletions pkg/cluster/manager_behavior_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,25 @@ func newManagerForBehaviorTests(t *testing.T, clusterName string, cfg *config.Cl
t.Fatalf("file.New() error = %v", err)
}

return &Manager{
m := &Manager{
logger: &log.Logger{Handler: discard.New(), Level: log.ErrorLevel},
provider: stub,
config: cfg,
fm: fm,
configFile: file.JoinPath(ClusterConfigDir, clusterName, ClusterConfigFile),
}

if cfg != nil && cfg.Name != "" {
data, err := json.Marshal(cfg)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
if err := m.fm.WriteFile(m.configFile, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}
}

return m
}

func TestManagerStart_ReturnsErrorWhenPersistedConfigInvalid(t *testing.T) {
Expand Down Expand Up @@ -112,14 +124,17 @@ func TestManagerStart_UsesPersistedConfigForReconcile(t *testing.T) {
func TestManagerGet_ReturnsErrorWhenNoPersistedConfigAndNoDefaults(t *testing.T) {
t.Parallel()

m := newManagerForBehaviorTests(t, "demo", &config.Cluster{}, &mock.ClientStub{})
m := newManagerForBehaviorTests(t, "demo", &config.Cluster{Name: "demo"}, &mock.ClientStub{})
if err := m.fm.RemoveFile(m.configFile); err != nil {
t.Fatalf("RemoveFile() error = %v", err)
}

_, err := m.Get(context.Background())
if err == nil {
t.Fatal("Get() expected error, got nil")
}
if !strings.Contains(err.Error(), "cluster config not found") {
t.Fatalf("Get() error = %q, want missing-config error", err)
if !strings.Contains(err.Error(), "cluster 'demo' not found") {
t.Fatalf("Get() error = %q, want missing-cluster error", err)
}
}

Expand Down Expand Up @@ -156,6 +171,23 @@ func TestManagerStop_AggregatesStopContainerError(t *testing.T) {
}
}

func TestManagerDelete_ReturnsNotFoundWhenConfigMissing(t *testing.T) {
t.Parallel()

m := newManagerForBehaviorTests(t, "ghost", &config.Cluster{Name: "ghost"}, &mock.ClientStub{})
if err := m.fm.RemoveFile(m.configFile); err != nil {
t.Fatalf("RemoveFile() error = %v", err)
}

err := m.Delete(context.Background())
if err == nil {
t.Fatal("Delete() expected error, got nil")
}
if !strings.Contains(err.Error(), "cluster 'ghost' not found") {
t.Fatalf("Delete() error = %q, want missing-cluster error", err)
}
}

func TestManagerDelete_ReturnsWrappedStopContainerError(t *testing.T) {
t.Parallel()

Expand Down
113 changes: 90 additions & 23 deletions pkg/cluster/manager_get_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,36 @@ func TestManagerGet_ReturnsInspectNetworkError(t *testing.T) {
t.Parallel()

wantErr := errors.New("docker daemon unavailable")
root := t.TempDir()
fm, err := file.New(root)
if err != nil {
t.Fatalf("file.New() error = %v", err)
}
cfg := &config.Cluster{
Name: "demo",
Network: config.Network{Name: "hind.demo"},
}
data, err := json.Marshal(cfg)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
configPath := file.JoinPath(ClusterConfigDir, "demo", ClusterConfigFile)
if err := fm.WriteFile(configPath, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

m := &Manager{
provider: &mock.ClientStub{
InspectNetworkFn: func(ctx context.Context, name string) (*provider.NetworkInfo, error) {
return nil, wantErr
},
},
config: &config.Cluster{
Name: "demo",
Network: config.Network{Name: "hind.demo"},
},
config: cfg,
fm: fm,
configFile: configPath,
}

_, err := m.Get(context.Background())
_, err = m.Get(context.Background())
if err == nil {
t.Fatal("Get() expected error, got nil")
}
Expand All @@ -108,6 +125,25 @@ func TestManagerGet_ReturnsInspectContainerError(t *testing.T) {
t.Parallel()

wantErr := errors.New("inspect container failed")
root := t.TempDir()
fm, err := file.New(root)
if err != nil {
t.Fatalf("file.New() error = %v", err)
}
cfg := &config.Cluster{
Name: "demo",
Network: config.Network{Name: "hind.demo"},
Nodes: []config.Node{{Name: "hind.demo.consul.01"}},
}
data, err := json.Marshal(cfg)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
configPath := file.JoinPath(ClusterConfigDir, "demo", ClusterConfigFile)
if err := fm.WriteFile(configPath, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

m := &Manager{
provider: &mock.ClientStub{
InspectNetworkFn: func(ctx context.Context, name string) (*provider.NetworkInfo, error) {
Expand All @@ -117,14 +153,12 @@ func TestManagerGet_ReturnsInspectContainerError(t *testing.T) {
return nil, wantErr
},
},
config: &config.Cluster{
Name: "demo",
Network: config.Network{Name: "hind.demo"},
Nodes: []config.Node{{Name: "hind.demo.consul.01"}},
},
config: cfg,
fm: fm,
configFile: configPath,
}

_, err := m.Get(context.Background())
_, err = m.Get(context.Background())
if err == nil {
t.Fatal("Get() expected error, got nil")
}
Expand Down Expand Up @@ -277,7 +311,7 @@ func TestManagerStop_UsesPersistedTopology(t *testing.T) {
}
}

func TestManagerLoadPersistedConfig_MissingFileKeepsDefaults(t *testing.T) {
func TestManagerLoadPersistedConfig_MissingFileReturnsNotFound(t *testing.T) {
t.Parallel()

m := &Manager{
Expand All @@ -288,12 +322,12 @@ func TestManagerLoadPersistedConfig_MissingFileKeepsDefaults(t *testing.T) {
},
}

if err := m.LoadPersistedConfig(); err != nil {
t.Fatalf("LoadPersistedConfig() unexpected error: %v", err)
err := m.LoadPersistedConfig()
if err == nil {
t.Fatal("LoadPersistedConfig() expected error when persisted config file is missing")
}

if m.config.Network.Name != "hind.demo-default" {
t.Fatalf("LoadPersistedConfig() changed defaults unexpectedly; got network %q", m.config.Network.Name)
if !strings.Contains(err.Error(), "cluster 'demo' not found") {
t.Fatalf("LoadPersistedConfig() error = %q, want missing-cluster error", err)
}
}

Expand All @@ -310,6 +344,25 @@ func TestManagerStop_PropagatesInspectContainerError(t *testing.T) {
t.Parallel()

wantErr := errors.New("container inspect failed")
root := t.TempDir()
fm, err := file.New(root)
if err != nil {
t.Fatalf("file.New() error = %v", err)
}
cfg := &config.Cluster{
Name: "demo",
Network: config.Network{Name: "hind.demo"},
Nodes: []config.Node{{Name: "hind.demo.consul.01"}},
}
data, err := json.Marshal(cfg)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
configPath := file.JoinPath(ClusterConfigDir, "demo", ClusterConfigFile)
if err := fm.WriteFile(configPath, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

m := &Manager{
logger: &log.Logger{Handler: discard.New(), Level: log.ErrorLevel},
provider: &mock.ClientStub{
Expand All @@ -318,14 +371,12 @@ func TestManagerStop_PropagatesInspectContainerError(t *testing.T) {
return nil, wantErr
},
},
config: &config.Cluster{
Name: "demo",
Network: config.Network{Name: "hind.demo"},
Nodes: []config.Node{{Name: "hind.demo.consul.01"}},
},
config: cfg,
fm: fm,
configFile: configPath,
}

err := m.Stop(context.Background())
err = m.Stop(context.Background())
if err == nil {
t.Fatal("Stop() expected error when InspectContainer returns error, got nil")
}
Expand Down Expand Up @@ -361,6 +412,14 @@ func TestManagerDelete_PropagatesInspectContainerError(t *testing.T) {
configFile: file.JoinPath(ClusterConfigDir, "demo", ClusterConfigFile),
}

data, err := json.Marshal(m.config)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
if err := fm.WriteFile(m.configFile, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

err = m.Delete(context.Background())
if err == nil {
t.Fatal("Delete() expected error when InspectContainer returns error, got nil")
Expand Down Expand Up @@ -401,6 +460,14 @@ func TestManagerDelete_PropagatesInspectNetworkError(t *testing.T) {
configFile: file.JoinPath(ClusterConfigDir, "demo", ClusterConfigFile),
}

data, err := json.Marshal(m.config)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
if err := fm.WriteFile(m.configFile, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

err = m.Delete(context.Background())
if err == nil {
t.Fatal("Delete() expected error when InspectNetwork returns error, got nil")
Expand Down
40 changes: 30 additions & 10 deletions pkg/cluster/manager_wait_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,36 @@ package cluster

import (
"context"
"encoding/json"
"errors"
"testing"
"time"

"github.com/apex/log"
"github.com/apex/log/handlers/discard"
"github.com/stenh0use/hind/pkg/config"
"github.com/stenh0use/hind/pkg/file"
"github.com/stenh0use/hind/pkg/provider"
"github.com/stenh0use/hind/pkg/provider/mock"
)

func TestWaitForContainersRunning_ReturnsContextErrorPromptly(t *testing.T) {
root := t.TempDir()
fm, err := file.New(root)
if err != nil {
t.Fatalf("file.New() error = %v", err)
}

clusterCfg := &config.Cluster{
Name: "test",
Network: config.Network{
Name: "hind.test",
},
Nodes: []config.Node{
{Name: "hind.test.consul.01"},
},
}

m := &Manager{
logger: &log.Logger{Handler: discard.New()},
provider: &mock.ClientStub{
Expand All @@ -24,22 +42,24 @@ func TestWaitForContainersRunning_ReturnsContextErrorPromptly(t *testing.T) {
return &provider.NetworkInfo{Name: name}, nil
},
},
config: &config.Cluster{
Name: "test",
Network: config.Network{
Name: "hind.test",
},
Nodes: []config.Node{
{Name: "hind.test.consul.01"},
},
},
config: clusterCfg,
fm: fm,
configFile: file.JoinPath(ClusterConfigDir, "test", ClusterConfigFile),
}

data, err := json.Marshal(clusterCfg)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
if err := fm.WriteFile(m.configFile, data); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

ctx, cancel := context.WithCancel(context.Background())
cancel()

start := time.Now()
err := m.waitForContainersRunning(ctx, 5*time.Second)
err = m.waitForContainersRunning(ctx, 5*time.Second)
elapsed := time.Since(start)

if !errors.Is(err, context.Canceled) {
Expand Down
Loading
Loading