Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 14 additions & 1 deletion internal/cli/delete.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ var (
var (
osExecutable = os.Executable
osRemoveAll = os.RemoveAll
osStat = os.Stat
)

// deleteOpts bundles the `tracebloc delete` flags.
Expand DownExpand Up@@ -382,7 +383,19 @@ func removeHostDataDir() error {
if err != nil {
return err
}
return osRemoveAll(dir)
if err := osRemoveAll(dir); err != nil {
return err
}
// Verify the directory is actually gone before the caller prints "✔ Removed".
// A nil RemoveAll is not proof the tree is absent — a racing writer, a mount,
// or a masked partial failure can leave it present — and claiming a clean wipe
// we didn't achieve is exactly the offboard-hygiene gap RFC-0003 flags.
if _, statErr := osStat(dir); statErr == nil {
return fmt.Errorf("%s still present after removal", dir)
} else if !errors.Is(statErr, os.ErrNotExist) {
return fmt.Errorf("verifying removal of %s: %w", dir, statErr)
}
return nil
}

// hostDataDirDisplay is the data dir for a user-facing hint; falls back to the
Expand Down
45 changes: 43 additions & 2 deletions internal/cli/delete_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,6 +39,7 @@ type fakeNodeboot struct {
pruneErr error
removedPaths []string
removeErr map[string]error // path → error to return from osRemoveAll
statPresent bool // when true, osStat reports the data dir still exists after rm
executable string
executableErr error
// Kubeconfig/context the uninstall seam was handed — so a test can prove the
Expand All@@ -50,7 +51,7 @@ type fakeNodeboot struct {
func (f *fakeNodeboot) install(t *testing.T) {
t.Helper()
origU, origT, origP := uninstallChart, teardownCluster, pruneImages
origExe, origRm:= osExecutable, osRemoveAll
origExe, origRm, origStat := osExecutable, osRemoveAll, osStat
uninstallChart = func(_ context.Context, ns, kubeconfig, kubeContext string) error {
f.calls = append(f.calls, "uninstall:"+ns)
f.uninstallKubeconfig, f.uninstallContext = kubeconfig, kubeContext
Expand DownExpand Up@@ -78,9 +79,19 @@ func (f *fakeNodeboot) install(t *testing.T) {
}
return nil
}
// osStat backs removeHostDataDir's verify-before-success. Default: the data dir
// is gone (ErrNotExist) so a clean wipe reports success. statPresent flips it to
// "still there" so a test can prove the CLI does NOT claim ✔ on an unverified wipe.
osStat = func(path string) (os.FileInfo, error) {
f.calls = append(f.calls, "stat:"+path)
if f.statPresent {
return nil, nil
}
return nil, os.ErrNotExist
}
t.Cleanup(func() {
uninstallChart, teardownCluster, pruneImages = origU, origT, origP
osExecutable, osRemoveAll= origExe, origRm
osExecutable, osRemoveAll, osStat = origExe, origRm, origStat
})
}

Expand DownExpand Up@@ -402,6 +413,36 @@ func TestDelete_WipeFails_StillClearsPointer(t *testing.T) {
}
}

// A nil RemoveAll is not proof the wipe happened: if the data dir is still present
// afterwards, delete must NOT print "✔ Removed" — it must warn and flag degraded,
// so offboard never claims a clean slate it didn't achieve (RFC-0003).
func TestDelete_WipeUnverified_DoesNotClaimSuccess(t *testing.T) {
withClientBackend(t, func(w http.ResponseWriter, r *http.Request) {
switch {
case r.Method == http.MethodGet && r.URL.Path == "/edge-device/":
_, _ = w.Write([]byte(`[{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":0}]`))
case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"):
w.WriteHeader(http.StatusOK)
}
})
setActiveForDelete(t, "5", "gpu-box-01", "gpu-box-01")
exe := filepath.Join(t.TempDir(), "tracebloc")
// osRemoveAll returns nil (no removeErr), but osStat reports the dir still present.
fn := &fakeNodeboot{executable: exe, statPresent: true}
fn.install(t)

var out bytes.Buffer
if err := runDelete(context.Background(), ui.New(&out), nil, deleteOpts{yes: true}); err != nil {
t.Fatalf("offboard: %v", err)
}
if strings.Contains(out.String(), "Removed local tracebloc data and config") {
t.Errorf("must NOT claim a clean wipe when the dir is still present:\n%s", out.String())
}
if !strings.Contains(out.String(), "Couldn't remove local data") {
t.Errorf("expected the unverified-wipe warning, got:\n%s", out.String())
}
}

// When a teardown step leaves real state behind, the closing line must NOT claim a
// clean offboard — it should flag that some cleanup didn't complete.
func TestDelete_TeardownFailure_HonestClosing(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions internal/cli/testdata/golden/zz-all-strings.golden
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,6 +48,7 @@ screen. %s/%d are runtime placeholders.
"%s of %s GiB"
"%s of %s cores"
"%s requires CLIENT_WRITE permission"
"%s still present after removal"
"%s unreachable: %v"
"%s · %d"
"%s · Online%s"
Expand DownExpand Up@@ -579,6 +580,7 @@ screen. %s/%d are runtime placeholders.
"unavailable"
"unknown command %q for %q"
"values:"
"verifying removal of %s: %w"
"waiting for ingestor Pod: %w"
"waiting for staging-cleanup pod: %w"
"waiting for teardown pod: %w"
Expand Down
Loading