internal/apply/hold.go

@@ -51,11 +51,21 @@ func KeepIn(dir string) Keep {
 // record (novox/hq ADR 0103). Looked at first, because the apply itself makes such things — a
 // file's parent directory, a unit file a module writes, a package that brings its unit — and what
 // the mesh made in this apply was not found.
-type foundBefore map[string]bool
+type foundBefore struct {
+	is map[string]bool
+	// trouble is what could not be asked about, by the same key, so the resource that would need
+	// the answer fails rather than proceeding as if the machine had nothing there.
+	trouble map[string]string
+}
+
+func (f foundBefore) has(key string) bool { return f.is[key] }
+
+// why is the reason a key could not be settled, and empty when there was none.
+func (f foundBefore) why(key string) string { return f.trouble[key] }
 
 func lookBefore(ctx context.Context, sys system.System, d *declaration.Declaration, known store.State,
 	run Runner) foundBefore {
-	seen := foundBefore{}
+	seen := foundBefore{is: map[string]bool{}, trouble: map[string]string{}}
 	if d.Adoption == nil {
 		return seen
 	}
@@ -70,25 +80,25 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati
 		switch res := r.(type) {
 		case *declaration.Directory:
 			if present(res.Path) && !recordedPath(known, res.Path) {
-				seen["path:"+res.Path] = true
+				seen.is["path:"+res.Path] = true
 			}
 		case *declaration.Archive:
 			// Unpacking over it, and re-owning it recursively, would change the predecessor's
 			// files.
 			if present(res.Path) && !recordedPath(known, res.Path) {
-				seen["path:"+res.Path] = true
+				seen.is["path:"+res.Path] = true
 			}
 		case *declaration.Process:
 			// Its unit would be written over and restarted.
 			if !known.Recorded(string(declaration.TypeProcess), res.Name) &&
 				present(filepath.Join(unitDir, res.Name+".service")) {
-				seen["unit-file:"+res.Name] = true
+				seen.is["unit-file:"+res.Name] = true
 			}
 		case *declaration.User:
 			// Its shell and groups would be changed.
 			if !known.Recorded(string(declaration.TypeUser), res.Name) {
 				if _, exists, err := system.LookUpUser(ctx, run, res.Name); err == nil && exists {
-					seen["user:"+res.Name] = true
+					seen.is["user:"+res.Name] = true
 				}
 			}
 		case *declaration.Service:
@@ -105,7 +115,7 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati
 			}
 			boot, _ := sys.ServiceBoot(ctx, run, res.Unit)
 			if state == "running" || boot == "enabled" {
-				seen["unit:"+res.Unit] = true
+				seen.is["unit:"+res.Unit] = true
 			}
 		case *declaration.Container:
 			if known.Recorded(string(declaration.TypeContainer), res.Name) {
@@ -117,7 +127,7 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati
 				case src == "":
 				case strings.HasPrefix(src, "/"):
 					if !systemPath(src) && present(src) && !recordedPath(known, src) {
-						seen["path:"+src] = true
+						seen.is["path:"+src] = true
 					}
 				default:
 					if !asked {
@@ -128,7 +138,10 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati
 						continue
 					}
 					if _, err := run(ctx, cri, "volume", "inspect", src); err == nil {
-						seen["volume:"+src] = true
+						seen.is["volume:"+src] = true
+					} else if !absent(err) {
+						seen.trouble["volume:"+src] = fmt.Sprintf(
+							"the container runtime could not say whether the volume %s is here: %v", src, err)
 					}
 				}
 			}
@@ -287,21 +300,28 @@ func holdOnAdopted(ctx context.Context, sys system.System, r declaration.Resourc
 				if strings.HasPrefix(src, "/") {
 					key = "path:" + src
 				}
-				if src != "" && before[key] {
+				if src == "" {
+					continue
+				}
+				if trouble := before.why(key); trouble != "" {
+					return false, false, begin(r), fmt.Errorf(
+						"%s, so it is not safe to create a container that would mount it", trouble)
+				}
+				if before.has(key) {
 					isFound, why = true, "would mount "+src+", found on the machine"
 					break
 				}
 			}
 		case *declaration.Directory:
-			isFound = before["path:"+res.Path]
+			isFound = before.has("path:" + res.Path)
 		case *declaration.Archive:
-			isFound = before["path:"+res.Path]
+			isFound = before.has("path:" + res.Path)
 		case *declaration.Process:
-			isFound = before["unit-file:"+res.Name]
+			isFound = before.has("unit-file:" + res.Name)
 		case *declaration.User:
-			isFound = before["user:"+res.Name]
+			isFound = before.has("user:" + res.Name)
 		case *declaration.Service:
-			isFound = before["unit:"+res.Unit]
+			isFound = before.has("unit:" + res.Unit)
 		}
 	}
 	if !isFound {
@@ -373,7 +393,16 @@ func inspectFound(ctx context.Context, name string, run Runner) (foundContainer,
 	out, err := run(ctx, cri, "inspect", "--format",
 		"{{.Id}}\t{{.State.Running}}\t{{index .Config.Labels \""+specLabel+"\"}}", name)
 	if err != nil {
-		return foundContainer{}, false, nil
+		if absent(err) {
+			return foundContainer{}, false, nil
+		}
+		// **A runtime that could not answer is not a machine with nothing there.** Read as
+		// absence, a daemon that is down or a permission denied would let the mesh create its own
+		// container over a predecessor's — the one thing an adopted node must never do
+		// (novox/hq ADR 0100).
+		return foundContainer{}, false, fmt.Errorf(
+			"the container runtime could not say whether %s is here, so it is not safe to make one: %w",
+			name, err)
 	}
 	parts := strings.Split(strings.TrimSpace(out), "\t")
 	for len(parts) < 3 {
@@ -386,6 +415,19 @@ func inspectFound(ctx context.Context, name string, run Runner) (foundContainer,
 	return foundContainer{id: strings.TrimSpace(parts[0]), running: parts[1] == "true", spec: spec}, true, nil
 }
 
+// absent is whether a runtime said the thing is not there, rather than failing to answer. Its own
+// words: docker and podman both say "No such object", "No such container" or "No such volume".
+func absent(err error) bool {
+	said := strings.ToLower(err.Error())
+	for _, missing := range []string{"no such object", "no such container", "no such volume",
+		"no such image"} {
+		if strings.Contains(said, missing) {
+			return true
+		}
+	}
+	return false
+}
+
 // hold keeps a found file or container as it is, and reports it — the first time by recording
 // what was found, every time after by comparing against that. Nothing is reverted, restarted or
 // created: a held target that disappears stays held and gone until its module is taken.

internal/apply/hold_test.go

@@ -825,3 +825,82 @@ func TestMountingTheMachinesOwnPlumbingIsNotFoundData(t *testing.T) {
 		t.Errorf("held: %+v", state.Held)
 	}
 }
+
+// Defends novox/hq ADR 0100: a runtime that cannot answer is not a machine with nothing there.
+
+// unreachable is a machine whose container runtime answers everything with a daemon that is down.
+type unreachable struct{ asked []string }
+
+func (u *unreachable) run(_ context.Context, name string, args ...string) (string, error) {
+	u.asked = append(u.asked, name+" "+strings.Join(args, " "))
+	if name == "docker" && args[0] == "info" {
+		return "27.0\n", nil
+	}
+	if name == "docker" {
+		return "", errors.New("docker exited 1: Cannot connect to the Docker daemon at unix:///var/run/docker.sock")
+	}
+	return "", nil
+}
+
+func TestARuntimeThatCannotAnswerNeverLetsTheMeshCreateOverAFoundContainer(t *testing.T) {
+	dir := t.TempDir()
+	u := &unreachable{}
+	d := adopted(t, untaken("hello-web.server"),
+		`{"id":"hello-web.server","type":"container","name":"hello-web","image":"`+pinned+`"}`)
+	_, state, err := ApplyKeeping(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared,
+		u.run, nil, nil, KeepIn(dir))
+	if err == nil || !strings.Contains(err.Error(), "not safe to make one") {
+		t.Fatalf("a runtime that could not answer was read as nothing there: %v", err)
+	}
+	for _, a := range u.asked {
+		if strings.HasPrefix(a, "docker run") || strings.HasPrefix(a, "docker rm") {
+			t.Errorf("the mesh acted on a container it could not ask about: %s", a)
+		}
+	}
+	if len(state.Held) != 0 {
+		t.Errorf("something was held on an answer the machine never gave: %+v", state.Held)
+	}
+}
+
+func TestAHeldContainerStaysHeldWhenTheRuntimeCannotAnswer(t *testing.T) {
+	dir, page, m := predecessor(t)
+	d := adopted(t, untaken("hello-web.page", "hello-web.server"), webResources(page))
+	_, state := applyAdopted(t, d, store.State{}, m, dir)
+	if _, held := state.HeldAt("hello-web.server"); !held {
+		t.Fatal("the found container was not held to begin with")
+	}
+	u := &unreachable{}
+	_, state, err := ApplyKeeping(context.Background(), archHost(t), d, state, store.OriginDeclared,
+		u.run, nil, nil, KeepIn(dir))
+	if err == nil {
+		t.Fatal("a runtime that could not answer reported success")
+	}
+	if h, held := state.HeldAt("hello-web.server"); !held || h.Changed != "" {
+		t.Errorf("a hold was let go or called changed on an answer the machine never gave: %+v", h)
+	}
+}
+
+func TestAVolumeTheRuntimeCannotBeAskedAboutStopsTheContainer(t *testing.T) {
+	dir := t.TempDir()
+	run := func(_ context.Context, name string, args ...string) (string, error) {
+		switch {
+		case name == "docker" && args[0] == "info":
+			return "27.0\n", nil
+		case name == "docker" && args[0] == "volume":
+			return "", errors.New("docker exited 1: Cannot connect to the Docker daemon")
+		case name == "docker" && args[0] == "inspect":
+			return "", errors.New("Error: No such object: hello-web")
+		case name == "docker":
+			return "", errors.New("docker run must not happen")
+		}
+		return "", nil
+	}
+	d := adopted(t, untaken("hello-web.server"),
+		`{"id":"hello-web.server","type":"container","name":"hello-web","image":"`+pinned+`",
+		  "volumes":["predecessor-data:/data"]}`)
+	_, _, err := ApplyKeeping(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared,
+		run, nil, nil, KeepIn(dir))
+	if err == nil || !strings.Contains(err.Error(), "could not say whether the volume") {
+		t.Fatalf("a volume the runtime could not be asked about did not stop the container: %v", err)
+	}
+}