Skip to content

[codex] harden command execution and tests #22

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
lobis merged 1 commit into from
Apr 23, 2026
Merged

[codex] harden command execution and tests #22

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
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ jobs:

- name: Run integration tests
env:
EOS_TEST_INTEGRATION: '1'
EOS_TEST_SSH_TARGET: eos-mgm
run: |
go test -v -timeout 10m -run TestIntegration ./eos/...
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ sudo rpm -i eos-tui.rpm
go install github.com/lobis/eos-tui@latest
```

Requires Go 1.21+. Make sure `$GOPATH/bin` (or `$HOME/go/bin`) is in your `PATH`.
Requires the Go version declared in `go.mod`. Make sure `$GOPATH/bin` (or `$HOME/go/bin`) is in your `PATH`.

---

Expand Down
4 changes: 2 additions & 2 deletions eos/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func New(_ context.Context, cfg Config) (*Client, error) {
sshTarget: cfg.SSHTarget,
timeout: timeout,
acceptNewHostKeys: cfg.AcceptNewHostKeys,
runner: execCommandRunner{},
}
c.sessionLogPath = initSessionLog()
return c, nil
Expand Down Expand Up @@ -98,8 +99,7 @@ func ensureRootPrefix(target string) string {
// commands are routed directly to the leader host.
// Returns the resolved hostname (e.g. "eospilot-ns-02.cern.ch").
func (c *Client) DiscoverMGMMaster(ctx context.Context) (string, error) {
_ = ctx
output, err := c.runCommand("redis-cli", "-p", "7777", "raft-info")
output, err := c.runCommandContext(ctx, "redis-cli", "-p", "7777", "raft-info")
if err != nil {
return "", fmt.Errorf("raft-info for master discovery: %w", err)
}
Expand Down
96 changes: 96 additions & 0 deletions eos/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,38 @@
package eos

import (
"context"
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"time"
)

type runnerCall struct {
name string
args []string
}

type recordingRunner struct {
out []byte
err error
calls []runnerCall
}

func (r *recordingRunner) CombinedOutput(ctx context.Context, name string, args ...string) ([]byte, error) {
if err := ctx.Err(); err != nil {
return nil, err
}
r.calls = append(r.calls, runnerCall{name: name, args: append([]string(nil), args...)})
if r.err != nil {
return r.out, r.err
}
return r.out, nil
}

func TestParseLabeledValues(t *testing.T) {
input := `
ALL Files 78 [booted] (0s)
Expand Down Expand Up @@ -669,6 +694,77 @@ func TestSessionCommandsKeepsLastNEntries(t *testing.T) {
}
}

func TestSessionCommandsReadsAppendsIncrementally(t *testing.T) {
dir := t.TempDir()
logPath := filepath.Join(dir, "session.log")
if err := os.WriteFile(logPath, []byte("[2026-04-09 10:00:00] cmd-1\n"), 0644); err != nil {
t.Fatalf("write log: %v", err)
}

client := &Client{sessionLogPath: logPath}
lines, err := client.SessionCommands(10)
if err != nil {
t.Fatalf("SessionCommands initial error: %v", err)
}
if got := strings.Join(lines, ","); got != "[2026-04-09 10:00:00] cmd-1" {
t.Fatalf("unexpected initial lines: %s", got)
}

f, err := os.OpenFile(logPath, os.O_APPEND|os.O_WRONLY, 0644)
if err != nil {
t.Fatalf("open append: %v", err)
}
if _, err := f.WriteString("[2026-04-09 10:00:01] ERROR (cmd): boom\n[2026-04-09 10:00:02] cmd-2\n"); err != nil {
_ = f.Close()
t.Fatalf("append log: %v", err)
}
if err := f.Close(); err != nil {
t.Fatalf("close append: %v", err)
}

lines, err = client.SessionCommands(10)
if err != nil {
t.Fatalf("SessionCommands append error: %v", err)
}
if got := strings.Join(lines, ","); got != "[2026-04-09 10:00:00] cmd-1,[2026-04-09 10:00:02] cmd-2" {
t.Fatalf("unexpected appended lines: %s", got)
}
}

func TestRunCommandContextUsesCallerCancellation(t *testing.T) {
runner := &recordingRunner{out: []byte("EOS_SERVER_VERSION=5.4.0\n")}
client := &Client{timeout: time.Minute, runner: runner}
ctx, cancel := context.WithCancel(context.Background())
cancel()

_, err := client.EOSVersion(ctx)
if !errors.Is(err, context.Canceled) {
t.Fatalf("expected context canceled error, got %v", err)
}
if len(runner.calls) != 0 {
t.Fatalf("runner should not record completed command after cancellation, got %#v", runner.calls)
}
}

func TestRunCommandContextUsesInjectedRunner(t *testing.T) {
runner := &recordingRunner{out: []byte("EOS_SERVER_VERSION=5.4.0\n")}
client := &Client{timeout: time.Minute, runner: runner}

version, err := client.EOSVersion(context.Background())
if err != nil {
t.Fatalf("EOSVersion error: %v", err)
}
if version != "5.4.0" {
t.Fatalf("version = %q, want 5.4.0", version)
}
if len(runner.calls) != 1 {
t.Fatalf("expected 1 runner call, got %d", len(runner.calls))
}
if runner.calls[0].name != "eos" || strings.Join(runner.calls[0].args, " ") != "version" {
t.Fatalf("unexpected runner call: %#v", runner.calls[0])
}
}

func TestNamespaceStatsParseWithPreamble(t *testing.T) {
raw := "* msg: ns booted\n" + `{
"errormsg": "",
Expand Down
12 changes: 3 additions & 9 deletions eos/fetch_access.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import (
)

func (c *Client) AccessList(ctx context.Context) ([]AccessRecord, error) {
_ = ctx

output, err := c.runCommand("eos", "access", "ls", "-m")
output, err := c.runCommandContext(ctx, "eos", "access", "ls", "-m")
if err != nil {
return nil, fmt.Errorf("eos access ls -m: %w", err)
}
Expand All @@ -18,26 +16,22 @@ func (c *Client) AccessList(ctx context.Context) ([]AccessRecord, error) {
}

func (c *Client) SetAccessRule(ctx context.Context, op, category, value string) error {
_ = ctx

args, err := accessRuleArgs(op, category, value)
if err != nil {
return err
}
if _, err := c.runCommand(args...); err != nil {
if _, err := c.runCommandContext(ctx, args...); err != nil {
return fmt.Errorf("%s %s %s: %w", strings.Join(args[:3], " "), category, value, err)
}
return nil
}

func (c *Client) SetAccessStall(ctx context.Context, seconds int) error {
_ = ctx

args, err := accessStallArgs(seconds)
if err != nil {
return err
}
if _, err := c.runCommand(args...); err != nil {
if _, err := c.runCommandContext(ctx, args...); err != nil {
return fmt.Errorf("%s: %w", strings.Join(args, " "), err)
}
return nil
Expand Down
7 changes: 2 additions & 5 deletions eos/fetch_filesystems.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import (
)

func (c *Client) FileSystems(ctx context.Context) ([]FileSystemRecord, error) {
_ = ctx

output, err := c.runCommand("eos", "-j", "-b", "fs", "ls")
output, err := c.runCommandContext(ctx, "eos", "-j", "-b", "fs", "ls")
if err != nil {
return nil, fmt.Errorf("eos fs ls: %w", err)
}
Expand Down Expand Up @@ -91,8 +89,7 @@ func (c *Client) FileSystems(ctx context.Context) ([]FileSystemRecord, error) {
// FsConfigStatus sets the configstatus of a filesystem by its ID.
// Valid values are "rw", "ro", and "" (empty to clear).
func (c *Client) FsConfigStatus(ctx context.Context, fsID uint64, value string) error {
_ = ctx
_, err := c.runCommand("eos", "-b", "fs", "config", fmt.Sprintf("%d", fsID), fmt.Sprintf("configstatus=%s", value))
_, err := c.runCommandContext(ctx, "eos", "-b", "fs", "config", fmt.Sprintf("%d", fsID), fmt.Sprintf("configstatus=%s", value))
if err != nil {
return fmt.Errorf("eos fs config %d configstatus=%s: %w", fsID, value, err)
}
Expand Down
8 changes: 2 additions & 6 deletions eos/fetch_groups.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import (
)

func (c *Client) Groups(ctx context.Context) ([]GroupRecord, error) {
_ = ctx

output, err := c.runCommand("eos", "-j", "-b", "group", "ls")
output, err := c.runCommandContext(ctx, "eos", "-j", "-b", "group", "ls")
if err != nil {
return nil, fmt.Errorf("eos group ls: %w", err)
}
Expand Down Expand Up @@ -61,14 +59,12 @@ func (c *Client) Groups(ctx context.Context) ([]GroupRecord, error) {
}

func (c *Client) SetGroupStatus(ctx context.Context, group, status string) error {
_ = ctx

args, err := groupSetArgs(group, status)
if err != nil {
return err
}

_, err = c.runCommand(args...)
_, err = c.runCommandContext(ctx, args...)
if err != nil {
return fmt.Errorf("eos group set %s %s: %w", group, status, err)
}
Expand Down
4 changes: 1 addition & 3 deletions eos/fetch_inspector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import (
)

func (c *Client) Inspector(ctx context.Context) (InspectorStats, error) {
_ = ctx

output, err := c.runCommand("eos", "inspector", "-l", "-m")
output, err := c.runCommandContext(ctx, "eos", "inspector", "-l", "-m")
if err != nil {
msg := strings.TrimSpace(string(output))
lower := strings.ToLower(msg)
Expand Down
13 changes: 4 additions & 9 deletions eos/fetch_ioshaping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (c *Client) IOShaping(ctx context.Context, mode IOShapingMode) ([]IOShaping
case IOShapingGroups:
flag = "--groups"
}
output, err := c.runCommand("eos", "io", "shaping", "ls", flag, "--json", "--window", "5")
output, err := c.runCommandContext(ctx, "eos", "io", "shaping", "ls", flag, "--json", "--window", "5")
if err != nil {
return nil, fmt.Errorf("io shaping ls: %w: %s", err, strings.TrimSpace(string(output)))
}
Expand Down Expand Up @@ -50,8 +50,7 @@ func (c *Client) IOShaping(ctx context.Context, mode IOShapingMode) ([]IOShaping
}

func (c *Client) IOShapingPolicies(ctx context.Context) ([]IOShapingPolicyRecord, error) {
_ = ctx
output, err := c.runCommand("eos", "io", "shaping", "policy", "ls", "--json")
output, err := c.runCommandContext(ctx, "eos", "io", "shaping", "policy", "ls", "--json")
if err != nil {
return nil, fmt.Errorf("io shaping policy ls: %w: %s", err, strings.TrimSpace(string(output)))
}
Expand Down Expand Up @@ -85,27 +84,23 @@ func (c *Client) IOShapingPolicies(ctx context.Context) ([]IOShapingPolicyRecord
}

func (c *Client) SetIOShapingPolicy(ctx context.Context, update IOShapingPolicyUpdate) error {
_ = ctx

args, err := ioShapingPolicySetArgs(update)
if err != nil {
return err
}

if _, err := c.runCommand(args...); err != nil {
if _, err := c.runCommandContext(ctx, args...); err != nil {
return fmt.Errorf("eos io shaping policy set %s: %w", update.ID, err)
}
return nil
}

func (c *Client) RemoveIOShapingPolicy(ctx context.Context, mode IOShapingMode, id string) error {
_ = ctx

args, err := ioShapingPolicyRemoveArgs(mode, id)
if err != nil {
return err
}
if _, err := c.runCommand(args...); err != nil {
if _, err := c.runCommandContext(ctx, args...); err != nil {
return fmt.Errorf("eos io shaping policy rm %s: %w", id, err)
}
return nil
Expand Down
12 changes: 5 additions & 7 deletions eos/fetch_mgm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,23 @@ import (
)

func (c *Client) MGMs(ctx context.Context) ([]MgmRecord, error) {
_ = ctx

if output, err := c.runCommand("eos", "-b", "ns", "stat", "-m"); err == nil {
if output, err := c.runCommandContext(ctx, "eos", "-b", "ns", "stat", "-m"); err == nil {
values := parseMonitoringKeyValues(output)
if mgms, ok := parseMGMsFromMonitoringValues(values); ok {
return mgms, nil
}
return c.mgmsFromRaftInfo(mgmPortFromMonitoringValues(values))
return c.mgmsFromRaftInfo(ctx, mgmPortFromMonitoringValues(values))
}

return c.mgmsFromRaftInfo("")
return c.mgmsFromRaftInfo(ctx, "")
}

func (c *Client) mgmsFromRaftInfo(mgmPort string) ([]MgmRecord, error) {
func (c *Client) mgmsFromRaftInfo(ctx context.Context, mgmPort string) ([]MgmRecord, error) {

// Run redis-cli raft-info directly via runCommand.
// The SSH target (if set) is always the MGM or an MGM leader node,
// so we do not need a separate SSH hop.
output, err := c.runCommand("redis-cli", "-p", "7777", "raft-info")
output, err := c.runCommandContext(ctx, "redis-cli", "-p", "7777", "raft-info")
if err != nil {
return nil, fmt.Errorf("redis-cli raft-info: %w\n%s", err, strings.TrimSpace(string(output)))
}
Expand Down
Loading