From da6bae348986d71d3b17dd984560878de66d4f2d Mon Sep 17 00:00:00 2001 From: Gary Date: Thu, 8 Oct 2026 19:43:19 +0200 Subject: [PATCH 1/4] feat(packages): Docker apart from the System packages update The update installed everything apt could upgrade, Docker's packages included, and upgrading docker-ce or containerd.io restarts the Docker daemon: every container stops until it is back, and the ones without a restart policy stay stopped. The check lists Docker on a line of its own (version, where it came from, what apt offers, the command to type to update it oneself), and the update installs the explicit list the check shows, minus Docker's packages: --only-upgrade install of names apt printed and that pass as package names, after a second simulation of exactly that list has been found to change nothing of Docker's and to remove nothing. DPkg::Lock::Timeout=120. One check of 'the box is being changed' (a ReCasaOS update, an update of Docker, a package manager holding dpkg's lock) now guards the package update, the ReCasaOS update and the automatic updater. --- main.go | 1 + pkg/autoupdate/autoupdate.go | 6 +- pkg/autoupdate/autoupdate_test.go | 23 ++ pkg/dockerpkg/dockerpkg.go | 133 ++++++++++ pkg/dockerpkg/dockerpkg_test.go | 118 +++++++++ route/v1/system.go | 8 +- route/v1/system_package_test.go | 48 ++++ service/system.go | 7 + service/system_package.go | 51 +++- service/system_package_docker.go | 185 ++++++++++++++ service/system_package_docker_test.go | 339 ++++++++++++++++++++++++++ service/system_package_test.go | 44 ++-- 12 files changed, 937 insertions(+), 26 deletions(-) create mode 100644 pkg/dockerpkg/dockerpkg.go create mode 100644 pkg/dockerpkg/dockerpkg_test.go create mode 100644 service/system_package_docker.go create mode 100644 service/system_package_docker_test.go diff --git a/main.go b/main.go index 18af480..dc1011e 100644 --- a/main.go +++ b/main.go @@ -130,6 +130,7 @@ func main() { // version.json, and settle an update that restarted the core before checking. autoupdate.Default.Releases = service.MyService.Casa() autoupdate.Default.Start = service.MyService.System().UpdateSystemVersion + autoupdate.Default.Busy = service.MyService.System().MaintenanceBusy go autoupdate.Default.Run(context.Background()) v1Router := route.InitV1Router() diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index dce243a..c589ce4 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -71,6 +71,10 @@ type AutoUpdate struct { // AppsBusy reports whether AppManagement lists an operation in progress, // or did not answer. AppsBusy func(ctx context.Context) bool + // Busy, when set, reports whether something else is changing the box: a + // package update, an update of Docker, a package manager holding dpkg's lock. + // Nothing starts while it does. + Busy func() bool // Notify, when set, hears how an attempt ended: succeeded, failed, or // paused on its second failure. main sets it to the push alerts' hook. Notify func(result, version string) @@ -148,7 +152,7 @@ func (a *AutoUpdate) Check(ctx context.Context) { if !decide(f).start { return } - f.unitActive, f.appsBusy = a.unitActive(), a.AppsBusy(ctx) + f.unitActive, f.appsBusy = a.unitActive(), a.AppsBusy(ctx) || (a.Busy != nil && a.Busy()) if v := decide(f); v.start { a.start(v.next.Version, now) } diff --git a/pkg/autoupdate/autoupdate_test.go b/pkg/autoupdate/autoupdate_test.go index 95f9364..dc213b6 100644 --- a/pkg/autoupdate/autoupdate_test.go +++ b/pkg/autoupdate/autoupdate_test.go @@ -551,3 +551,26 @@ func TestAppManagementIsAskedAsAnInternalRequest(t *testing.T) { } }) } + +func TestAPackageUpdateOrDpkgsLockHoldsTheStart(t *testing.T) { + a, b := newBox(t) + saveState(t, a, on()) + busy := true + a.Busy = func() bool { return busy } + before := stateFile(t, a) + + a.Check(context.Background()) + + if len(b.started) != 0 { + t.Fatalf("started %v while the box was being changed", b.started) + } + if after := stateFile(t, a); after != before { + t.Fatalf("autoupdate.json = %s, want it untouched: the next check tries again", after) + } + + busy = false + a.Check(context.Background()) + if len(b.started) != 1 { + t.Fatalf("started %v once the box was free, want one start", b.started) + } +} diff --git a/pkg/dockerpkg/dockerpkg.go b/pkg/dockerpkg/dockerpkg.go new file mode 100644 index 0000000..78646d1 --- /dev/null +++ b/pkg/dockerpkg/dockerpkg.go @@ -0,0 +1,133 @@ +// Package dockerpkg knows which of a Debian-family box's packages are Docker's, where +// the installed Docker came from, and what an apt simulation says it would do to them. +// +// ReCasaOS updates system packages with one apt upgrade, and an upgrade of docker-ce or +// containerd.io restarts the Docker daemon, which stops every container until it is back. +// The System packages update therefore leaves this family out and shows it on a line of +// its own; this package is what tells the two apart. It runs nothing: callers give it +// text apt printed, and it gives back what is in it. Nothing here is ever built into a +// shell command from a version string: the manual commands are constants. +package dockerpkg + +import ( + "bufio" + "regexp" + "strings" +) + +// family is every package whose upgrade replaces, or restarts, the Docker engine or the +// parts ReCasaOS uses with it. +var family = map[string]struct{}{ + "docker-ce": {}, + "docker-ce-cli": {}, + "docker-ce-rootless-extras": {}, + "containerd.io": {}, + "docker-buildx-plugin": {}, + "docker-compose-plugin": {}, + "docker-model-plugin": {}, + "docker.io": {}, + "docker-compose-v2": {}, + "docker-buildx": {}, + "containerd": {}, +} + +// IsFamily reports whether name is one of the Docker engine's packages. +func IsFamily(name string) bool { + _, ok := family[name] + return ok +} + +var namePattern = regexp.MustCompile(`^[a-z0-9][a-z0-9+.\-]*(:[a-z0-9]+)?$`) + +// ValidName reports whether name looks like a Debian package name (with an optional +// architecture), the only kind of text that may be put into an apt command line. +func ValidName(name string) bool { + return len(name) <= 128 && namePattern.MatchString(name) +} + +// Origin is where the installed Docker came from. +type Origin string + +const ( + // OriginDockerRepo is docker-ce from Docker's own apt repository, the one the + // installer sets up: https://download.docker.com/linux/. + OriginDockerRepo Origin = "docker-repository" + // OriginDistribution is the docker.io package of Debian, Ubuntu or Raspberry Pi OS. + OriginDistribution Origin = "distribution" + // OriginSnap is the snap. + OriginSnap Origin = "snap" + // OriginUnknown is a Docker whose source this does not recognise: a mirror, a plain + // http repository, a package installed by hand. + OriginUnknown Origin = "unknown" +) + +const dockerRepositoryPrefix = "https://download.docker.com/linux/" + +var policySourcePattern = regexp.MustCompile(`^\s+(?:\*\*\*\s+)?\d+\s+(https?://\S+)\s`) + +// OriginFromPolicy tells where docker-ce comes from, from the output of +// `apt-cache policy docker-ce`: Docker's repository when some version of the package is +// offered by https://download.docker.com/linux/, anything else is unknown. +func OriginFromPolicy(policy string) Origin { + scanner := bufio.NewScanner(strings.NewReader(policy)) + for scanner.Scan() { + if m := policySourcePattern.FindStringSubmatch(scanner.Text()); m != nil && strings.HasPrefix(m[1], dockerRepositoryPrefix) { + return OriginDockerRepo + } + } + return OriginUnknown +} + +// ManualCommand is what an owner types to update Docker themselves, for an origin, or "" +// when there is no command this can stand behind. They are constants on purpose. +func ManualCommand(origin Origin) string { + switch origin { + case OriginDockerRepo: + return "sudo apt-get update && sudo apt-get install --only-upgrade docker-ce docker-ce-cli containerd.io docker-buildx-plugin docker-compose-plugin" + case OriginDistribution: + return "sudo apt-get update && sudo apt-get install --only-upgrade docker.io" + case OriginSnap: + return "sudo snap refresh docker" + default: + return "" + } +} + +var engineVersionPattern = regexp.MustCompile(`^(?:\d+:)?(\d+(?:\.\d+)*)`) + +// EngineVersion is the upstream version in a dpkg version: "5:29.8.1-1~ubuntu.22.04~jammy" +// is "29.8.1". It is "" for text that does not start with one. +func EngineVersion(dpkgVersion string) string { + if m := engineVersionPattern.FindStringSubmatch(strings.TrimSpace(dpkgVersion)); m != nil { + return m[1] + } + return "" +} + +var ( + simulationInstPattern = regexp.MustCompile(`^Inst\s+(\S+)`) + simulationRemvPattern = regexp.MustCompile(`^Remv\s+(\S+)`) +) + +// Touched is what an apt simulation (`apt-get -s`) says it would do. +type Touched struct { + // Docker is the packages of the Docker family it would install or upgrade. + Docker []string + // Removed is every package it would remove. + Removed []string +} + +// Touches reads the Inst and Remv lines of an apt simulation. +func Touches(simulation string) Touched { + var touched Touched + scanner := bufio.NewScanner(strings.NewReader(simulation)) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if m := simulationInstPattern.FindStringSubmatch(line); m != nil && IsFamily(m[1]) { + touched.Docker = append(touched.Docker, m[1]) + } else if m := simulationRemvPattern.FindStringSubmatch(line); m != nil { + touched.Removed = append(touched.Removed, m[1]) + } + } + return touched +} diff --git a/pkg/dockerpkg/dockerpkg_test.go b/pkg/dockerpkg/dockerpkg_test.go new file mode 100644 index 0000000..525e7f9 --- /dev/null +++ b/pkg/dockerpkg/dockerpkg_test.go @@ -0,0 +1,118 @@ +package dockerpkg + +import ( + "reflect" + "strings" + "testing" +) + +func TestIsFamily(t *testing.T) { + for _, name := range []string{"docker-ce", "docker-ce-cli", "containerd.io", "docker-buildx-plugin", "docker-compose-plugin", "docker-ce-rootless-extras", "docker.io", "containerd"} { + if !IsFamily(name) { + t.Errorf("IsFamily(%q) = false", name) + } + } + for _, name := range []string{"", "docker", "docker-ce-extra", "libc6", "runc", "dockerd", "containerd.io2", "podman", "snapd", "Docker-CE"} { + if IsFamily(name) { + t.Errorf("IsFamily(%q) = true", name) + } + } +} + +func TestValidName(t *testing.T) { + for _, name := range []string{"zlib1g", "libc6", "g++-12", "libstdc++6", "python3.11", "apache2-bin", "libc6:amd64", "a"} { + if !ValidName(name) { + t.Errorf("ValidName(%q) = false", name) + } + } + for _, name := range []string{"", "-rf", "x;rm -rf /", "a b", "$(id)", "`id`", "a\nb", "UPPER", "a&&b", ".hidden", "pkg=1.0", strings.Repeat("a", 129), "pkg/../x", "a|b"} { + if ValidName(name) { + t.Errorf("ValidName(%q) = true", name) + } + } +} + +const dockerRepoPolicy = `docker-ce: + Installed: 5:29.8.1-1~ubuntu.22.04~jammy + Candidate: 5:29.8.2-1~ubuntu.22.04~jammy + Version table: + 5:29.8.2-1~ubuntu.22.04~jammy 500 + 500 https://download.docker.com/linux/ubuntu jammy/stable amd64 Packages + *** 5:29.8.1-1~ubuntu.22.04~jammy 500 + 500 https://download.docker.com/linux/ubuntu jammy/stable amd64 Packages + 100 /var/lib/dpkg/status +` + +func TestOriginFromPolicy(t *testing.T) { + cases := map[string]Origin{ + dockerRepoPolicy: OriginDockerRepo, + // the installed version is old and the repository has dropped it: another version is still offered by it + "docker-ce:\n Installed: 5:24.0.5-1\n Candidate: 5:24.0.9-1\n Version table:\n 5:24.0.9-1 500\n 500 https://download.docker.com/linux/debian bookworm/stable amd64 Packages\n *** 5:24.0.5-1 100\n 100 /var/lib/dpkg/status\n": OriginDockerRepo, + // only dpkg's own record: installed by hand, or the repository is gone + "docker-ce:\n Installed: 5:24.0.5-1\n Candidate: 5:24.0.5-1\n Version table:\n *** 5:24.0.5-1 100\n 100 /var/lib/dpkg/status\n": OriginUnknown, + // a mirror, and plain http: not Docker's repository as this knows it + "docker-ce:\n Version table:\n 5:29.0.0-1 500\n 500 https://mirrors.example.org/docker-ce/linux/ubuntu jammy/stable amd64 Packages\n": OriginUnknown, + "docker-ce:\n Version table:\n 5:29.0.0-1 500\n 500 http://download.docker.com/linux/ubuntu jammy/stable amd64 Packages\n": OriginUnknown, + // a lookalike host + "docker-ce:\n Version table:\n 5:29.0.0-1 500\n 500 https://download.docker.com.evil.example/linux/ubuntu jammy stable Packages\n": OriginUnknown, + "": OriginUnknown, + } + for policy, want := range cases { + if got := OriginFromPolicy(policy); got != want { + t.Errorf("OriginFromPolicy(%q) = %q, want %q", policy, got, want) + } + } +} + +func TestManualCommandsAreConstants(t *testing.T) { + if got := ManualCommand(OriginDockerRepo); !strings.Contains(got, "--only-upgrade docker-ce docker-ce-cli containerd.io") || strings.Contains(got, " -y") { + t.Errorf("docker repository command = %q", got) + } + if got := ManualCommand(OriginDistribution); !strings.Contains(got, "docker.io") { + t.Errorf("distribution command = %q", got) + } + if got := ManualCommand(OriginSnap); got != "sudo snap refresh docker" { + t.Errorf("snap command = %q", got) + } + if got := ManualCommand(OriginUnknown); got != "" { + t.Errorf("unknown origin command = %q, want none", got) + } + if got := ManualCommand(Origin("x; rm -rf /")); got != "" { + t.Errorf("a made-up origin gave a command: %q", got) + } +} + +func TestEngineVersion(t *testing.T) { + cases := map[string]string{ + "5:29.8.1-1~ubuntu.22.04~jammy": "29.8.1", + "5:24.0.5-1~debian.12~bookworm": "24.0.5", + "26.1.5+dfsg1-9+deb13u1": "26.1.5", + "20.10.24-0ubuntu1": "20.10.24", + "": "", + "junk": "", + } + for in, want := range cases { + if got := EngineVersion(in); got != want { + t.Errorf("EngineVersion(%q) = %q, want %q", in, got, want) + } + } +} + +func TestTouches(t *testing.T) { + simulation := `Reading package lists... +Inst libc6 [2.35-0ubuntu3.10] (2.35-0ubuntu3.11 Ubuntu:22.04/jammy-updates [amd64]) +Inst docker-ce [5:29.8.1-1~ubuntu.22.04~jammy] (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64]) +Inst containerd.io [2.3.5-1~ubuntu.22.04~jammy] (2.3.6-1~ubuntu.22.04~jammy Docker CE:jammy [amd64]) +Remv oldthing [1.0] +Conf docker-ce (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64]) +Inst docker-ce-extra-thing [1] (2 x [amd64]) +` + got := Touches(simulation) + want := Touched{Docker: []string{"docker-ce", "containerd.io"}, Removed: []string{"oldthing"}} + if !reflect.DeepEqual(got, want) { + t.Errorf("Touches() = %#v, want %#v", got, want) + } + if clean := Touches("Inst libc6 [1] (2 x [amd64])\n"); len(clean.Docker) != 0 || len(clean.Removed) != 0 { + t.Errorf("Touches(clean) = %#v", clean) + } +} diff --git a/route/v1/system.go b/route/v1/system.go index a89bcc8..7d4f8e8 100644 --- a/route/v1/system.go +++ b/route/v1/system.go @@ -65,6 +65,9 @@ func SystemUpdate(ctx echo.Context) error { need, latestVersion := version.IsNeedUpdate(service.MyService.Casa().GetCasaosVersion()) if need { if err := service.MyService.System().UpdateSystemVersion(latestVersion.Version); err != nil { + if errors.Is(err, service.ErrSystemMaintenanceBusy) { + return ctx.JSON(http.StatusConflict, model.Result{Success: http.StatusConflict, Message: err.Error()}) + } return ctx.JSON(common_err.SERVICE_ERROR, model.Result{Success: common_err.SERVICE_ERROR, Message: err.Error()}) } } @@ -108,7 +111,10 @@ func GetSystemPackageUpdates(ctx echo.Context) error { func StartSystemPackageUpdate(ctx echo.Context) error { status, err := service.MyService.System().StartSystemPackageUpdate() if err != nil { - if errors.Is(err, service.ErrSystemPackageUpdateRunning) { + if errors.Is(err, service.ErrSystemPackageUpdateRunning) || + errors.Is(err, service.ErrSystemMaintenanceBusy) || + errors.Is(err, service.ErrSystemPackageNothingToUpdate) || + errors.Is(err, service.ErrSystemPackageTouchesDocker) { return ctx.JSON(http.StatusConflict, model.Result{ Success: http.StatusConflict, Message: err.Error(), diff --git a/route/v1/system_package_test.go b/route/v1/system_package_test.go index b9b4de4..c3e6f24 100644 --- a/route/v1/system_package_test.go +++ b/route/v1/system_package_test.go @@ -3,6 +3,7 @@ package v1 import ( "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "testing" @@ -168,3 +169,50 @@ func TestGetSystemPackageUpdateStatusResponse(t *testing.T) { t.Fatalf("response data = %#v", response.Data) } } + +func TestStartSystemPackageUpdateRefusalsAreConflicts(t *testing.T) { + original := service.MyService + t.Cleanup(func() { service.MyService = original }) + + for name, refusal := range map[string]error{ + "another maintenance": service.ErrSystemMaintenanceBusy, + "nothing but Docker": service.ErrSystemPackageNothingToUpdate, + "a list that touches Docker": service.ErrSystemPackageTouchesDocker, + } { + fakeSystem := &fakeSystemPackageService{startErr: fmt.Errorf("%w: detail", refusal)} + service.MyService = fakeSystemPackageRepository{system: fakeSystem} + + recorder := performSystemPackageRequest(t, http.MethodPost, "/v1/sys/packages/update") + if recorder.Code != http.StatusConflict { + t.Errorf("%s: status = %d, want %d", name, recorder.Code, http.StatusConflict) + } + } +} + +func TestGetSystemPackageUpdatesCarriesDockerOnItsOwnLine(t *testing.T) { + original := service.MyService + t.Cleanup(func() { service.MyService = original }) + + fakeSystem := &fakeSystemPackageService{updates: service.SystemPackageUpdates{ + Supported: true, + Manager: "apt", + Updates: []service.SystemPackageUpdate{}, + Docker: &service.SystemPackageDocker{ + Installed: true, Origin: "docker-repository", Version: "29.8.1", + Updates: []service.SystemPackageUpdate{{Name: "docker-ce", CurrentVersion: "5:29.8.1", CandidateVersion: "5:29.8.2"}}, + ManualCommand: "sudo apt-get update && sudo apt-get install --only-upgrade docker-ce", + }, + }} + service.MyService = fakeSystemPackageRepository{system: fakeSystem} + + recorder := performSystemPackageRequest(t, http.MethodGet, "/v1/sys/packages") + var response model.Result + if err := json.Unmarshal(recorder.Body.Bytes(), &response); err != nil { + t.Fatalf("decode response: %v", err) + } + data, _ := response.Data.(map[string]interface{}) + docker, _ := data["docker"].(map[string]interface{}) + if docker["version"] != "29.8.1" || docker["origin"] != "docker-repository" || docker["manual_command"] == "" { + t.Fatalf("response data = %#v", response.Data) + } +} diff --git a/service/system.go b/service/system.go index 541c13f..474bc4f 100644 --- a/service/system.go +++ b/service/system.go @@ -1,6 +1,7 @@ package service import ( + "context" "encoding/json" "errors" "fmt" @@ -37,6 +38,9 @@ import ( type SystemService interface { UpdateSystemVersion(version string) error + // MaintenanceBusy says whether a package update, an update of Docker or a package + // manager is changing the box now. + MaintenanceBusy() bool GetSystemPackageUpdates() (SystemPackageUpdates, error) StartSystemPackageUpdate() (SystemPackageUpdateStatus, error) GetSystemPackageUpdateStatus() SystemPackageUpdateStatus @@ -377,6 +381,9 @@ func (c *systemService) GetNet(physics bool) []string { } func (s *systemService) UpdateSystemVersion(version string) error { + if reason, busy := s.systemPackageUpdater().maintenanceBusy(context.Background(), common.UPDATE_UNIT+".service"); busy { + return fmt.Errorf("%w: %s", ErrSystemMaintenanceBusy, reason) + } keyName := "casa_version:" + resolveUpdateVersionURL() Cache.Delete(keyName) diff --git a/service/system_package.go b/service/system_package.go index af93b91..5255b34 100644 --- a/service/system_package.go +++ b/service/system_package.go @@ -18,6 +18,7 @@ import ( commonfile "github.com/ReCasaOS/CasaOS-Common/utils/file" "github.com/ReCasaOS/CasaOS/pkg/config" + "github.com/ReCasaOS/CasaOS/pkg/dockerpkg" ) const ( @@ -56,6 +57,8 @@ type SystemPackageUpdates struct { Count int `json:"count"` CheckedAt *time.Time `json:"checked_at,omitempty"` RebootRequired bool `json:"reboot_required"` + // Docker is shown on a line of its own: the update never installs it. + Docker *SystemPackageDocker `json:"docker,omitempty"` } type SystemPackageUpdateStatus struct { @@ -205,8 +208,10 @@ func (u *systemPackageUpdater) check() (SystemPackageUpdates, error) { return result, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) } - result.Updates = parseAPTUpgradeSimulation(string(output)) + var dockerUpdates []SystemPackageUpdate + result.Updates, dockerUpdates = splitDockerUpdates(parseAPTUpgradeSimulation(string(output))) result.Count = len(result.Updates) + result.Docker = u.dockerInfo(ctx, dockerUpdates) now := u.now().UTC() result.CheckedAt = &now result.RebootRequired = u.rebootRequired() @@ -233,6 +238,18 @@ func (u *systemPackageUpdater) startUpdate() (SystemPackageUpdateStatus, error) return u.statusLocked(support), ErrSystemPackageUpdateRunning } + if reason, busy := u.maintenanceBusy(context.Background(), systemPackageUpdateUnit); busy { + status.Error = reason + return status, fmt.Errorf("%w: %s", ErrSystemMaintenanceBusy, reason) + } + listCtx, cancelList := context.WithTimeout(context.Background(), systemPackageCheckTimeout) + names, err := u.upgradeNames(listCtx, support) + cancelList() + if err != nil { + status.Error = err.Error() + return status, err + } + logPath := u.logPath() if err := u.mkdirAll(filepath.Dir(logPath), 0o755); err != nil { markSystemPackageUpdateFailed(&status, fmt.Sprintf("prepare package update log: %v", err), u.now) @@ -245,7 +262,11 @@ func (u *systemPackageUpdater) startUpdate() (SystemPackageUpdateStatus, error) return status, err } - args := systemPackageUpdateArgs(support.aptPath, logPath) + args, err := systemPackageUpdateArgs(support.aptPath, logPath, names) + if err != nil { + markSystemPackageUpdateFailed(&status, err.Error(), u.now) + return status, err + } if output, err := u.start(support.systemdPath, systemPackageUpdateUnit, args...); err != nil { failure := fmt.Sprintf("CASAOS_PACKAGE_UPDATE_FAILED %s 1\n%s\n", u.now().UTC().Format(time.RFC3339), trimSystemPackageOutput(output)) _ = u.writeFile(logPath, []byte(failure), 0o644) @@ -379,7 +400,11 @@ func parseAPTUpgradeSimulation(output string) []SystemPackageUpdate { return result } -func systemPackageUpdateArgs(aptPath, logPath string) []string { +func systemPackageUpdateArgs(aptPath, logPath string, names []string) ([]string, error) { + command, err := systemPackageUpdateCommand(aptPath, logPath, names) + if err != nil { + return nil, err + } return []string{ "--quiet", "--no-block", @@ -392,14 +417,26 @@ func systemPackageUpdateArgs(aptPath, logPath string) []string { "-o", "pipefail", "-c", - systemPackageUpdateCommand(aptPath, logPath), - } + command, + }, nil } -func systemPackageUpdateCommand(aptPath, logPath string) string { +// systemPackageUpdateCommand installs exactly the packages in names, as upgrades: a name +// that is not a package name is refused, never put into the shell line. +func systemPackageUpdateCommand(aptPath, logPath string, names []string) (string, error) { + if len(names) == 0 { + return "", ErrSystemPackageNothingToUpdate + } + quotedNames := make([]string, 0, len(names)) + for _, name := range names { + if !dockerpkg.ValidName(name) { + return "", fmt.Errorf("%q is not a package name", name) + } + quotedNames = append(quotedNames, shellQuote(name)) + } quotedLogPath := shellQuote(logPath) quotedAPTPath := shellQuote(aptPath) - return "set -o pipefail; exec >> " + quotedLogPath + " 2>&1; printf 'CASAOS_PACKAGE_UPDATE_STARTED %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; " + quotedAPTPath + " -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold upgrade; status=$?; if [ -f /var/run/reboot-required ]; then printf 'CASAOS_PACKAGE_UPDATE_REBOOT_REQUIRED\\n'; fi; if [ \"$status\" -eq 0 ]; then printf 'CASAOS_PACKAGE_UPDATE_SUCCESS %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; else printf 'CASAOS_PACKAGE_UPDATE_FAILED %s %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\" \"$status\"; fi; exit \"$status\"" + return "set -o pipefail; exec >> " + quotedLogPath + " 2>&1; printf 'CASAOS_PACKAGE_UPDATE_STARTED %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; " + quotedAPTPath + " -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold -o DPkg::Lock::Timeout=120 install --only-upgrade --no-install-recommends " + strings.Join(quotedNames, " ") + "; status=$?; if [ -f /var/run/reboot-required ]; then printf 'CASAOS_PACKAGE_UPDATE_REBOOT_REQUIRED\\n'; fi; if [ \"$status\" -eq 0 ]; then printf 'CASAOS_PACKAGE_UPDATE_SUCCESS %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; else printf 'CASAOS_PACKAGE_UPDATE_FAILED %s %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\" \"$status\"; fi; exit \"$status\"", nil } type parsedSystemPackageUpdateLog struct { diff --git a/service/system_package_docker.go b/service/system_package_docker.go new file mode 100644 index 0000000..919db20 --- /dev/null +++ b/service/system_package_docker.go @@ -0,0 +1,185 @@ +package service + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" + + "github.com/ReCasaOS/CasaOS/common" + "github.com/ReCasaOS/CasaOS/pkg/dockerpkg" +) + +// Docker is not part of the System packages update. Upgrading docker-ce or containerd.io +// restarts the Docker daemon and stops every container until it is back (the apps that +// have a restart policy start again by themselves, the others stay stopped), and an apt +// upgrade of everything used to do that without a word. The update now installs the +// packages the check lists, minus Docker's, and Docker is shown on a line of its own. + +var ( + ErrSystemPackageNothingToUpdate = errors.New("there is nothing to update here: what is left is Docker's, which is updated on its own") + ErrSystemPackageTouchesDocker = errors.New("this update would also change Docker's packages") + ErrSystemMaintenanceBusy = errors.New("another update or package operation is running on this box") +) + +// SystemPackageDocker is the Docker line of the System packages check. +type SystemPackageDocker struct { + Installed bool `json:"installed"` + // Origin is where Docker came from: see dockerpkg.Origin. + Origin string `json:"origin,omitempty"` + // Version is the engine's, from its package: 29.8.1. + Version string `json:"version,omitempty"` + // Updates is what apt offers for Docker's packages; none of it is installed by the + // System packages update. + Updates []SystemPackageUpdate `json:"updates"` + // ManualCommand is what to type to update Docker oneself; empty when this does not + // know a command to stand behind. + ManualCommand string `json:"manual_command,omitempty"` +} + +const ( + systemDockerUpdateUnit = "casaos-docker-update.service" // reserved for an update of Docker on its own + dpkgLockFile = "/var/lib/dpkg/lock-frontend" +) + +// systemMaintenanceUnits are the transient units that change the box's packages or the +// ReCasaOS release: at most one of them runs at a time, and none while dpkg's lock is held. +var systemMaintenanceUnits = []string{common.UPDATE_UNIT + ".service", systemPackageUpdateUnit, systemDockerUpdateUnit} + +// maintenanceBusy says whether the box is being changed by something else than the unit +// named in except (the caller's own, which it checks in its own way): another of the +// units above is active, or a package manager holds dpkg's lock. Systemd or flock not +// answering is not busy: a refusal that cannot be explained helps nobody. +func (u *systemPackageUpdater) maintenanceBusy(ctx context.Context, except string) (string, bool) { + for _, unit := range systemMaintenanceUnits { + if unit == except { + continue + } + output, err := u.command(ctx, "systemctl", "show", unit, "--property=ActiveState", "--value") + if err != nil { + continue + } + switch strings.TrimSpace(string(output)) { + case "active", "activating", "deactivating": + return fmt.Sprintf("%s is running", strings.TrimSuffix(unit, ".service")), true + } + } + if _, err := u.command(ctx, "flock", "--nonblock", "--exclusive", dpkgLockFile, "true"); err != nil { + var exit *exec.ExitError + if errors.As(err, &exit) { + return "another package manager holds dpkg's lock", true + } + } + return "", false +} + +// MaintenanceBusy is maintenanceBusy for a caller that is not one of the units: the +// automatic updater, which starts nothing while the box is being changed. +func (s *systemService) MaintenanceBusy() bool { + _, busy := s.systemPackageUpdater().maintenanceBusy(context.Background(), common.UPDATE_UNIT+".service") + return busy +} + +// splitDockerUpdates separates what apt would upgrade in Docker's family from the rest. +func splitDockerUpdates(all []SystemPackageUpdate) (rest, docker []SystemPackageUpdate) { + rest, docker = []SystemPackageUpdate{}, []SystemPackageUpdate{} + for _, update := range all { + if dockerpkg.IsFamily(update.Name) { + docker = append(docker, update) + } else { + rest = append(rest, update) + } + } + return rest, docker +} + +// dockerInfo is the Docker line: nil when there is no Docker here and nothing of its +// family to upgrade, so that a box without Docker shows none. +func (u *systemPackageUpdater) dockerInfo(ctx context.Context, pending []SystemPackageUpdate) *SystemPackageDocker { + info := &SystemPackageDocker{Updates: pending} + if info.Updates == nil { + info.Updates = []SystemPackageUpdate{} + } + switch { + case u.installedVersion(ctx, "docker-ce") != "": + version := u.installedVersion(ctx, "docker-ce") + info.Installed = true + info.Version = dockerpkg.EngineVersion(version) + policy, _ := u.command(ctx, "apt-cache", "policy", "docker-ce") + info.Origin = string(dockerpkg.OriginFromPolicy(string(policy))) + case u.installedVersion(ctx, "docker.io") != "": + info.Installed = true + info.Version = dockerpkg.EngineVersion(u.installedVersion(ctx, "docker.io")) + info.Origin = string(dockerpkg.OriginDistribution) + default: + if _, err := u.command(ctx, "snap", "list", "docker"); err == nil { + info.Installed = true + info.Origin = string(dockerpkg.OriginSnap) + } + } + if !info.Installed && len(info.Updates) == 0 { + return nil + } + info.ManualCommand = dockerpkg.ManualCommand(dockerpkg.Origin(info.Origin)) + return info +} + +// installedVersion is the dpkg version of an installed package, or "". +func (u *systemPackageUpdater) installedVersion(ctx context.Context, name string) string { + output, err := u.command(ctx, "dpkg-query", "-W", "-f=${db:Status-Abbrev}\t${Version}\n", name) + if err != nil { + return "" + } + for _, line := range strings.Split(string(output), "\n") { + status, version, ok := strings.Cut(strings.TrimSpace(line), "\t") + if ok && strings.HasPrefix(strings.TrimSpace(status), "ii") { + return strings.TrimSpace(version) + } + } + return "" +} + +// systemPackageSimulateArgs are apt's arguments for a simulation that takes no lock. +func systemPackageSimulateArgs(rest ...string) []string { + return append([]string{"-s", "--no-remove", "-o", "Debug::NoLocking=true", "-o", "Dpkg::Use-Pty=0"}, rest...) +} + +// upgradeNames is the explicit list the update installs: what a plain upgrade would +// upgrade, minus Docker's packages. It is the contract of the exclusion: the list is +// made of names apt printed and checked as package names, and a second simulation of +// installing exactly that list must change nothing of Docker's and remove nothing. +func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemPackageSupport) ([]string, error) { + output, err := u.command(ctx, support.aptPath, systemPackageSimulateArgs("upgrade")...) + if err != nil { + return nil, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) + } + var names []string + for _, update := range parseAPTUpgradeSimulation(string(output)) { + if dockerpkg.IsFamily(update.Name) || update.CurrentVersion == "" { + // Docker's own, or a package apt would install new rather than upgrade + continue + } + if !dockerpkg.ValidName(update.Name) { + return nil, fmt.Errorf("apt named a package that is not a package name: %q", update.Name) + } + names = append(names, update.Name) + } + if len(names) == 0 { + return nil, ErrSystemPackageNothingToUpdate + } + + args := systemPackageSimulateArgs(append([]string{"install", "--only-upgrade", "--no-install-recommends"}, names...)...) + output, err = u.command(ctx, support.aptPath, args...) + if err != nil { + return nil, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) + } + touched := dockerpkg.Touches(string(output)) + if len(touched.Docker) > 0 { + return nil, fmt.Errorf("%w: %s", ErrSystemPackageTouchesDocker, strings.Join(touched.Docker, ", ")) + } + if len(touched.Removed) > 0 { + return nil, fmt.Errorf("this update would remove packages (%s), which the System packages update never does", strings.Join(touched.Removed, ", ")) + } + return names, nil +} diff --git a/service/system_package_docker_test.go b/service/system_package_docker_test.go new file mode 100644 index 0000000..6a11a15 --- /dev/null +++ b/service/system_package_docker_test.go @@ -0,0 +1,339 @@ +package service + +import ( + "context" + "errors" + "os" + "os/exec" + "strings" + "testing" + + "github.com/ReCasaOS/CasaOS/common" +) + +// aptBox answers the commands the System packages updater runs, from its fields. +type aptBox struct { + upgradeSimulation string // apt-get -s upgrade + installSimulation string // apt-get -s install --only-upgrade + activeUnits map[string]bool + dpkgLocked bool + flockMissing bool + dockerCE string // the dpkg version of docker-ce, "" when it is not installed + dockerIO string + policy string + snap bool + calls []string +} + +func (b *aptBox) command(_ context.Context, name string, args ...string) ([]byte, error) { + b.calls = append(b.calls, name+" "+strings.Join(args, " ")) + switch name { + case "systemctl": + if len(args) > 1 && b.activeUnits[args[1]] { + return []byte("active\n"), nil + } + return []byte("inactive\n"), nil + case "flock": + if b.flockMissing { + return nil, &exec.Error{Name: "flock", Err: exec.ErrNotFound} + } + if b.dpkgLocked { + return nil, &exec.ExitError{} + } + return nil, nil + case "dpkg-query": + pkg := args[len(args)-1] + if pkg == "docker-ce" && b.dockerCE != "" { + return []byte("ii \t" + b.dockerCE + "\n"), nil + } + if pkg == "docker.io" && b.dockerIO != "" { + return []byte("ii \t" + b.dockerIO + "\n"), nil + } + return nil, errors.New("dpkg-query: no packages found matching " + pkg) + case "apt-cache": + return []byte(b.policy), nil + case "snap": + if b.snap { + return nil, nil + } + return nil, errors.New("snap: not installed") + } + if strings.HasSuffix(name, "apt-get") { + joined := " " + strings.Join(args, " ") + " " + switch { + case len(args) > 0 && args[0] == "update": + return nil, nil + case strings.Contains(joined, " -s ") && strings.Contains(joined, " install "): + return []byte(b.installSimulation), nil + case strings.Contains(joined, " -s "): + return []byte(b.upgradeSimulation), nil + } + } + return nil, nil +} + +const ( + simLibc = "Inst libc6 [2.35-0ubuntu3.10] (2.35-0ubuntu3.11 Ubuntu:22.04/jammy-updates [amd64])\n" + simZlib = "Inst zlib1g [1:1.2.11] (1:1.2.12 Ubuntu:22.04/jammy-updates [amd64])\n" + simDockerC = "Inst docker-ce [5:29.8.1-1~ubuntu.22.04~jammy] (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64])\n" + simContain = "Inst containerd.io [2.3.5-1~ubuntu.22.04~jammy] (2.3.6-1~ubuntu.22.04~jammy Docker CE:jammy [amd64])\n" + dockerRepo = "docker-ce:\n Installed: 5:29.8.1-1~ubuntu.22.04~jammy\n Candidate: 5:29.8.2-1~ubuntu.22.04~jammy\n Version table:\n 5:29.8.2-1~ubuntu.22.04~jammy 500\n 500 https://download.docker.com/linux/ubuntu jammy/stable amd64 Packages\n *** 5:29.8.1-1~ubuntu.22.04~jammy 100\n 100 /var/lib/dpkg/status\n" +) + +func TestSystemPackageCheckListsDockerOnALineOfItsOwn(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + box := &aptBox{ + upgradeSimulation: simLibc + simDockerC + simContain + simZlib, + dockerCE: "5:29.8.1-1~ubuntu.22.04~jammy", + policy: dockerRepo, + } + updater.command = box.command + + got, err := updater.check() + if err != nil { + t.Fatalf("check() error = %v", err) + } + if got.Count != 2 || len(got.Updates) != 2 || got.Updates[0].Name != "libc6" || got.Updates[1].Name != "zlib1g" { + t.Fatalf("the update list holds %#v, want libc6 and zlib1g only", got.Updates) + } + if got.Docker == nil || !got.Docker.Installed || got.Docker.Version != "29.8.1" || got.Docker.Origin != "docker-repository" { + t.Fatalf("Docker = %#v", got.Docker) + } + if len(got.Docker.Updates) != 2 || got.Docker.Updates[0].Name != "containerd.io" || got.Docker.Updates[1].Name != "docker-ce" { + t.Fatalf("Docker's updates = %#v", got.Docker.Updates) + } + if !strings.Contains(got.Docker.ManualCommand, "--only-upgrade docker-ce docker-ce-cli containerd.io") { + t.Fatalf("manual command = %q", got.Docker.ManualCommand) + } +} + +func TestSystemPackageCheckHasNoDockerLineWithoutDocker(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + box := &aptBox{upgradeSimulation: simLibc} + updater.command = box.command + + got, err := updater.check() + if err != nil || got.Count != 1 || got.Docker != nil { + t.Fatalf("check() = %#v, %v", got, err) + } +} + +func TestSystemPackageCheckKnowsWhereDockerCameFrom(t *testing.T) { + cases := map[string]struct { + box aptBox + origin string + }{ + "distribution": {aptBox{dockerIO: "26.1.5+dfsg1-9+deb13u1"}, "distribution"}, + "snap": {aptBox{snap: true}, "snap"}, + "by hand": {aptBox{dockerCE: "5:24.0.5-1", policy: "docker-ce:\n Version table:\n *** 5:24.0.5-1 100\n 100 /var/lib/dpkg/status\n"}, "unknown"}, + } + for name, c := range cases { + box := c.box + updater := newTestSystemPackageUpdater(t) + updater.command = box.command + got, err := updater.check() + if err != nil || got.Docker == nil || got.Docker.Origin != c.origin { + t.Errorf("%s: check() = %#v, %v; want origin %q", name, got.Docker, err, c.origin) + } + } +} + +func TestSystemPackageCheckShowsDockerPendingEvenWhenItIsNotInstalledByDpkg(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + box := &aptBox{upgradeSimulation: simContain} + updater.command = box.command + + got, err := updater.check() + if err != nil || got.Count != 0 || got.Docker == nil || len(got.Docker.Updates) != 1 { + t.Fatalf("check() = %#v, %v", got, err) + } +} + +func startedCommand(t *testing.T, updater *systemPackageUpdater) string { + t.Helper() + var started string + updater.start = func(_ string, _ string, args ...string) ([]byte, error) { + started = strings.Join(args, " ") + return nil, nil + } + if _, err := updater.startUpdate(); err != nil { + t.Fatalf("startUpdate() error = %v", err) + } + return started +} + +func TestSystemPackageUpdateInstallsTheListAndLeavesDockerOut(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + box := &aptBox{ + upgradeSimulation: simLibc + simDockerC + simContain + simZlib + "Inst brand-new-dependency (1 Ubuntu:22.04 [amd64])\n", + installSimulation: simLibc + simZlib, + } + updater.command = box.command + + started := startedCommand(t, updater) + for _, want := range []string{ + "install --only-upgrade --no-install-recommends 'libc6' 'zlib1g';", + "-o DPkg::Lock::Timeout=120", + "--no-remove", + } { + if !strings.Contains(started, want) { + t.Errorf("the unit's command lacks %q: %s", want, started) + } + } + for _, unwanted := range []string{"docker", "containerd", "brand-new-dependency", " upgrade;"} { + if strings.Contains(started, unwanted) { + t.Errorf("the unit's command has %q: %s", unwanted, started) + } + } + // the list was simulated as it will be installed, before it was started + var simulated bool + for _, call := range box.calls { + if strings.Contains(call, "-s") && strings.Contains(call, "install --only-upgrade --no-install-recommends libc6 zlib1g") { + simulated = true + } + } + if !simulated { + t.Errorf("the explicit list was not simulated: %#v", box.calls) + } +} + +func TestSystemPackageUpdateWithOnlyDockerLeftIsRefusedAndWritesNothing(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: simDockerC + simContain}).command + var started bool + updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + + _, err := updater.startUpdate() + if !errors.Is(err, ErrSystemPackageNothingToUpdate) { + t.Fatalf("startUpdate() error = %v, want ErrSystemPackageNothingToUpdate", err) + } + if started { + t.Fatal("a unit was started with nothing to install") + } + if _, statErr := os.Stat(updater.logPath()); !os.IsNotExist(statErr) { + t.Fatalf("a log was written: %v", statErr) + } +} + +func TestSystemPackageUpdateRefusesAListThatWouldChangeDocker(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{ + upgradeSimulation: simLibc, + // something in the list depends on a newer docker-ce + installSimulation: simLibc + simDockerC, + }).command + var started bool + updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + + _, err := updater.startUpdate() + if !errors.Is(err, ErrSystemPackageTouchesDocker) || !strings.Contains(err.Error(), "docker-ce") { + t.Fatalf("startUpdate() error = %v, want ErrSystemPackageTouchesDocker naming docker-ce", err) + } + if started { + t.Fatal("a unit was started for a list that changes Docker") + } +} + +func TestSystemPackageUpdateRefusesAListThatWouldRemoveSomething(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: simLibc, installSimulation: simLibc + "Remv oldthing [1.0]\n"}).command + _, err := updater.startUpdate() + if err == nil || !strings.Contains(err.Error(), "oldthing") { + t.Fatalf("startUpdate() error = %v, want a refusal naming oldthing", err) + } +} + +func TestSystemPackageUpdateNeverPutsAFunnyNameInAShellLine(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: "Inst foo;rm [1] (2 Ubuntu [amd64])\n"}).command + var started bool + updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + + _, err := updater.startUpdate() + if err == nil || started { + t.Fatalf("startUpdate() error = %v, started = %v: a name with a semicolon got through", err, started) + } + if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", []string{"zlib1g", "x; rm -rf /"}); err == nil { + t.Fatal("systemPackageUpdateCommand() accepted a name with a semicolon") + } + if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", nil); !errors.Is(err, ErrSystemPackageNothingToUpdate) { + t.Fatalf("systemPackageUpdateCommand(nil) error = %v", err) + } +} + +func TestSystemPackageUpdateWaitsForOtherMaintenance(t *testing.T) { + cases := map[string]aptBox{ + "a ReCasaOS update": {activeUnits: map[string]bool{common.UPDATE_UNIT + ".service": true}}, + "an update of Docker": {activeUnits: map[string]bool{systemDockerUpdateUnit: true}}, + "a package manager's lock": {dpkgLocked: true}, + } + for name, c := range cases { + box := c + box.upgradeSimulation = simLibc + box.installSimulation = simLibc + updater := newTestSystemPackageUpdater(t) + updater.command = box.command + var started bool + updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + + _, err := updater.startUpdate() + if !errors.Is(err, ErrSystemMaintenanceBusy) || started { + t.Errorf("%s: startUpdate() error = %v, started = %v", name, err, started) + } + } +} + +func TestMaintenanceBusy(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + for _, unit := range systemMaintenanceUnits { + box := &aptBox{activeUnits: map[string]bool{unit: true}} + updater.command = box.command + if reason, busy := updater.maintenanceBusy(context.Background(), ""); !busy || reason == "" { + t.Errorf("%s active: busy = %v, reason = %q", unit, busy, reason) + } + // the caller's own unit is its own to judge + if _, busy := updater.maintenanceBusy(context.Background(), unit); busy { + t.Errorf("%s active but excepted: busy", unit) + } + } + for name, box := range map[string]*aptBox{ + "nothing running": {}, + "flock not installed": {flockMissing: true}, + "lock held": {dpkgLocked: true}, + "another unit stopped": {activeUnits: map[string]bool{}}, + } { + updater.command = box.command + _, busy := updater.maintenanceBusy(context.Background(), "") + if want := box.dpkgLocked; busy != want { + t.Errorf("%s: busy = %v, want %v", name, busy, want) + } + } + // systemd that does not answer is not a reason to refuse + updater.command = func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "systemctl" { + return nil, errors.New("systemctl: not found") + } + return nil, nil + } + if _, busy := updater.maintenanceBusy(context.Background(), ""); busy { + t.Error("an unanswering systemctl counted as busy") + } +} + +func TestAReCasaOSUpdateWaitsForAPackageUpdate(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{activeUnits: map[string]bool{systemPackageUpdateUnit: true}}).command + system := &systemService{packageUpdates: updater} + + err := system.UpdateSystemVersion("v0.0.0-test") + if !errors.Is(err, ErrSystemMaintenanceBusy) { + t.Fatalf("UpdateSystemVersion() error = %v, want ErrSystemMaintenanceBusy", err) + } + if !system.MaintenanceBusy() { + t.Fatal("MaintenanceBusy() = false with a package update running") + } + updater.command = (&aptBox{}).command + if system.MaintenanceBusy() { + t.Fatal("MaintenanceBusy() = true on an idle box") + } +} diff --git a/service/system_package_test.go b/service/system_package_test.go index baa4160..4443ed3 100644 --- a/service/system_package_test.go +++ b/service/system_package_test.go @@ -60,9 +60,12 @@ func TestParseAPTUpgradeSimulationNoUpdates(t *testing.T) { } func TestSystemPackageUpdateCommand(t *testing.T) { - command := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/casaos/package update.log") + command, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/casaos/package update.log", []string{"libc6", "zlib1g"}) + if err != nil { + t.Fatalf("systemPackageUpdateCommand() error = %v", err) + } for _, expected := range []string{ - "'/usr/bin/apt-get' -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold upgrade", + "'/usr/bin/apt-get' -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold -o DPkg::Lock::Timeout=120 install --only-upgrade --no-install-recommends 'libc6' 'zlib1g'; status=$?", "CASAOS_PACKAGE_UPDATE_STARTED", "CASAOS_PACKAGE_UPDATE_SUCCESS", "CASAOS_PACKAGE_UPDATE_FAILED", @@ -78,7 +81,10 @@ func TestSystemPackageUpdateCommand(t *testing.T) { } func TestSystemPackageUpdateArgsAreDetached(t *testing.T) { - args := systemPackageUpdateArgs("/usr/bin/apt-get", "/var/log/casaos/package-update.log") + args, err := systemPackageUpdateArgs("/usr/bin/apt-get", "/var/log/casaos/package-update.log", []string{"libc6"}) + if err != nil { + t.Fatalf("systemPackageUpdateArgs() error = %v", err) + } joined := strings.Join(args, " ") if !strings.Contains(joined, "--no-block") || !strings.Contains(joined, "--collect") { t.Fatalf("systemPackageUpdateArgs() = %#v, want detached systemd job", args) @@ -124,18 +130,8 @@ func TestReadBoundedSystemPackageLog(t *testing.T) { func TestSystemPackageUpdaterCheck(t *testing.T) { updater := newTestSystemPackageUpdater(t) - simulation := "Inst zlib1g [1.2.13.dfsg-1] (1.2.13.dfsg-1+deb12u1 Debian:12/stable [amd64])\n" - var calls []string - updater.command = func(_ context.Context, name string, args ...string) ([]byte, error) { - calls = append(calls, name+" "+strings.Join(args, " ")) - if name == "systemctl" { - return []byte("inactive\n"), nil - } - if len(args) > 0 && args[0] == "update" { - return nil, nil - } - return []byte(simulation), nil - } + box := &aptBox{upgradeSimulation: "Inst zlib1g [1.2.13.dfsg-1] (1.2.13.dfsg-1+deb12u1 Debian:12/stable [amd64])\n"} + updater.command = box.command got, err := updater.check() if err != nil { @@ -144,8 +140,21 @@ func TestSystemPackageUpdaterCheck(t *testing.T) { if !got.Supported || got.Manager != systemPackageManagerAPT || got.Count != 1 { t.Fatalf("check() = %#v", got) } - if len(calls) != 3 || !strings.Contains(calls[1], "apt-get update") || !strings.Contains(calls[2], "upgrade") { - t.Fatalf("unexpected command sequence: %#v", calls) + // the index is refreshed, then the upgrade is simulated, and nothing is installed + var update, simulation int + for i, call := range box.calls { + if strings.Contains(call, "apt-get update") { + update = i + } + if strings.Contains(call, "apt-get -s") && strings.HasSuffix(call, " upgrade") { + simulation = i + } + if strings.Contains(call, " install ") && !strings.Contains(call, " -s ") { + t.Fatalf("check() installed something: %q", call) + } + } + if update == 0 && simulation == 0 || update >= simulation { + t.Fatalf("unexpected command sequence: %#v", box.calls) } } @@ -209,6 +218,7 @@ func TestSystemPackageUpdaterRejectsDuplicateUpdate(t *testing.T) { func TestSystemPackageUpdaterStartWritesQueuedState(t *testing.T) { updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: simLibc, installSimulation: simLibc}).command var startedPath, startedUnit string var startedArgs []string updater.start = func(path, unit string, args ...string) ([]byte, error) { From b105102ad7905244e4cb348c6fa70db5022a88ce Mon Sep 17 00:00:00 2001 From: Gary Date: Thu, 8 Oct 2026 20:14:06 +0200 Subject: [PATCH 2/4] fix(packages): a real look at dpkg's lock, and the review's other findings The check on dpkg's lock used flock(1), whose lock apt and dpkg never see (they take POSIX record locks), and counted any non-zero exit as held: on a host without dpkg it refused the ReCasaOS update for ever. The lock is now probed with fcntl F_GETLK on the lock files, only on a host that has dpkg, and tested against a lock held by another process. Also: the manual command names every package the line lists; the family is excluded only on a box that has a Docker engine; held and half-configured packages count as installed; the simulations no longer hold the update's lock; the box is looked at again right before the unit starts, and the unit itself simulates the list again right before it installs and installs nothing if it now touches Docker or removes a package; a list that is no longer the checked upgrades is a conflict; a refusal to start is not a failed attempt of the automatic updater; 'nothing to update' no longer blames Docker on a box without it. --- main.go | 9 +- pkg/autoupdate/autoupdate.go | 22 +- pkg/autoupdate/autoupdate_test.go | 31 +++ pkg/dockerpkg/dockerpkg.go | 101 +++++++-- pkg/dockerpkg/dockerpkg_test.go | 87 ++++++-- route/v1/system.go | 3 +- route/v1/system_package_test.go | 1 + service/dpkg_lock_linux.go | 39 ++++ service/dpkg_lock_linux_test.go | 86 ++++++++ service/dpkg_lock_other.go | 6 + service/system_package.go | 40 +++- service/system_package_docker.go | 123 +++++++---- service/system_package_docker_test.go | 286 ++++++++++++++++++++------ 13 files changed, 691 insertions(+), 143 deletions(-) create mode 100644 service/dpkg_lock_linux.go create mode 100644 service/dpkg_lock_linux_test.go create mode 100644 service/dpkg_lock_other.go diff --git a/main.go b/main.go index dc1011e..e1095c3 100644 --- a/main.go +++ b/main.go @@ -5,6 +5,7 @@ package main import ( "context" _ "embed" + "errors" "flag" "fmt" "net" @@ -129,7 +130,13 @@ func main() { // Automatic updates start the button's own update, read the button's own // version.json, and settle an update that restarted the core before checking. autoupdate.Default.Releases = service.MyService.Casa() - autoupdate.Default.Start = service.MyService.System().UpdateSystemVersion + autoupdate.Default.Start = func(release string) error { + err := service.MyService.System().UpdateSystemVersion(release) + if errors.Is(err, service.ErrSystemMaintenanceBusy) { + return fmt.Errorf("%w: %v", autoupdate.ErrBusy, err) + } + return err + } autoupdate.Default.Busy = service.MyService.System().MaintenanceBusy go autoupdate.Default.Run(context.Background()) v1Router := route.InitV1Router() diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index c589ce4..aa8a6b8 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -17,6 +17,7 @@ package autoupdate import ( "context" "encoding/json" + "errors" "math/rand/v2" "net/http" "os" @@ -53,6 +54,10 @@ type Releases interface { FetchCasaosVersion() model.Version } +// ErrBusy is what Start returns, wrapped, when something else is changing the box and the +// update was not started: that is not a failed attempt, and the next check tries again. +var ErrBusy = errors.New("the box is being changed by something else") + // AutoUpdate is the automatic updates of one box. type AutoUpdate struct { // Root is where autoupdate.json is kept: "/" on a box, a temporary @@ -163,12 +168,27 @@ func (a *AutoUpdate) Check(ctx context.Context) { // that fails is failed at once. func (a *AutoUpdate) start(release string, now time.Time) { last := Last{Version: release, StartedAt: now.UTC().Truncate(time.Second), Result: resultRunning} - if err := a.modify(func(s *State) bool { s.Last = &last; return true }); err != nil { + var before *Last + if err := a.modify(func(s *State) bool { before = s.Last; s.Last = &last; return true }); err != nil { logger.Info("automatic update: cannot save autoupdate.json, not starting", zap.String("version", release), zap.Error(err)) return } logger.Info("automatic update: starting", zap.String("version", release)) if err := a.Start(release); err != nil { + if errors.Is(err, ErrBusy) { + // nothing was started: the attempt is taken back, not counted + logger.Info("automatic update: the box is busy, not starting", zap.String("version", release), zap.Error(err)) + if err := a.modify(func(s *State) bool { + if s.Last == nil || s.Last.Result != resultRunning || s.Last.Version != release { + return false + } + s.Last = before + return true + }); err != nil { + logger.Info("automatic update: cannot save autoupdate.json", zap.Error(err)) + } + return + } logger.Info("automatic update: cannot start", zap.String("version", release), zap.Error(err)) if err := a.modify(func(s *State) bool { if s.Last == nil || s.Last.Result != resultRunning { diff --git a/pkg/autoupdate/autoupdate_test.go b/pkg/autoupdate/autoupdate_test.go index dc213b6..100558d 100644 --- a/pkg/autoupdate/autoupdate_test.go +++ b/pkg/autoupdate/autoupdate_test.go @@ -3,6 +3,7 @@ package autoupdate import ( "context" "errors" + "fmt" "net/http" "net/http/httptest" "os" @@ -574,3 +575,33 @@ func TestAPackageUpdateOrDpkgsLockHoldsTheStart(t *testing.T) { t.Fatalf("started %v once the box was free, want one start", b.started) } } + +func TestABusyRefusalFromStartIsNotAFailedAttempt(t *testing.T) { + a, b := newBox(t) + saveState(t, a, on()) + refuse := true + a.Start = func(version string) error { + b.started = append(b.started, version) + if refuse { + return fmt.Errorf("%w: a package update is running", ErrBusy) + } + return nil + } + before := stateFile(t, a) + + a.Check(context.Background()) + a.Check(context.Background()) + + if len(b.started) != 2 { + t.Fatalf("started %v: each check should have tried again", b.started) + } + if after := stateFile(t, a); after != before { + t.Fatalf("autoupdate.json = %s, want it as it was: a refusal is not an attempt, and two of them must not pause the release", after) + } + + refuse = false + a.Check(context.Background()) + if len(b.started) != 3 { + t.Fatalf("started %v once the box was free", b.started) + } +} diff --git a/pkg/dockerpkg/dockerpkg.go b/pkg/dockerpkg/dockerpkg.go index 78646d1..ad90093 100644 --- a/pkg/dockerpkg/dockerpkg.go +++ b/pkg/dockerpkg/dockerpkg.go @@ -1,17 +1,19 @@ // Package dockerpkg knows which of a Debian-family box's packages are Docker's, where // the installed Docker came from, and what an apt simulation says it would do to them. // -// ReCasaOS updates system packages with one apt upgrade, and an upgrade of docker-ce or -// containerd.io restarts the Docker daemon, which stops every container until it is back. -// The System packages update therefore leaves this family out and shows it on a line of -// its own; this package is what tells the two apart. It runs nothing: callers give it -// text apt printed, and it gives back what is in it. Nothing here is ever built into a -// shell command from a version string: the manual commands are constants. +// ReCasaOS updates system packages with an apt install of a list, and an upgrade of +// docker-ce or containerd.io restarts the Docker daemon, which stops every container until +// it is back. The System packages update therefore leaves this family out and shows it on a +// line of its own; this package is what tells the two apart. It runs nothing: callers give +// it text apt printed, and it gives back what is in it. Nothing here is ever built into a +// shell command from a version string, and a package name reaches a command only if it is +// one of the fixed names below. package dockerpkg import ( "bufio" "regexp" + "sort" "strings" ) @@ -31,12 +33,37 @@ var family = map[string]struct{}{ "containerd": {}, } +// restarts are the packages whose upgrade restarts the daemon (or containerd under it). +var restarts = map[string]struct{}{ + "docker-ce": {}, + "containerd.io": {}, + "docker.io": {}, + "containerd": {}, +} + +// base is a package name without the architecture apt adds to a foreign one: +// "containerd.io:armhf" is "containerd.io". +func base(name string) string { + b, _, _ := strings.Cut(name, ":") + return b +} + // IsFamily reports whether name is one of the Docker engine's packages. func IsFamily(name string) bool { - _, ok := family[name] + _, ok := family[base(name)] return ok } +// RestartsEngine reports whether upgrading any of names restarts Docker. +func RestartsEngine(names []string) bool { + for _, name := range names { + if _, ok := restarts[base(name)]; ok { + return true + } + } + return false +} + var namePattern = regexp.MustCompile(`^[a-z0-9][a-z0-9+.\-]*(:[a-z0-9]+)?$`) // ValidName reports whether name looks like a Debian package name (with an optional @@ -61,6 +88,12 @@ const ( OriginUnknown Origin = "unknown" ) +// CanSeeUpdates reports whether apt can tell of an update for an installation from this +// origin: of a snap, or of a Docker whose source is not known, it cannot. +func (o Origin) CanSeeUpdates() bool { + return o == OriginDockerRepo || o == OriginDistribution +} + const dockerRepositoryPrefix = "https://download.docker.com/linux/" var policySourcePattern = regexp.MustCompile(`^\s+(?:\*\*\*\s+)?\d+\s+(https?://\S+)\s`) @@ -78,19 +111,42 @@ func OriginFromPolicy(policy string) Origin { return OriginUnknown } +// defaultPackages is what to update by hand when apt has not said which packages are +// behind. +var defaultPackages = map[Origin][]string{ + OriginDockerRepo: {"docker-ce", "docker-ce-cli", "containerd.io", "docker-buildx-plugin", "docker-compose-plugin"}, + OriginDistribution: {"docker.io"}, +} + // ManualCommand is what an owner types to update Docker themselves, for an origin, or "" -// when there is no command this can stand behind. They are constants on purpose. -func ManualCommand(origin Origin) string { - switch origin { - case OriginDockerRepo: - return "sudo apt-get update && sudo apt-get install --only-upgrade docker-ce docker-ce-cli containerd.io docker-buildx-plugin docker-compose-plugin" - case OriginDistribution: - return "sudo apt-get update && sudo apt-get install --only-upgrade docker.io" - case OriginSnap: +// when there is no command this can stand behind. It names the packages apt says are +// pending when it said some, so that running it leaves nothing behind, and only names from +// the fixed family: nothing apt printed reaches it unless it is one of them. +func ManualCommand(origin Origin, pending []string) string { + if origin == OriginSnap { return "sudo snap refresh docker" - default: + } + defaults, ok := defaultPackages[origin] + if !ok { return "" } + chosen := map[string]struct{}{} + for _, name := range pending { + if b := base(name); IsFamily(name) { + chosen[b] = struct{}{} + } + } + if len(chosen) == 0 { + for _, name := range defaults { + chosen[name] = struct{}{} + } + } + names := make([]string, 0, len(chosen)) + for name := range chosen { + names = append(names, name) + } + sort.Strings(names) + return "sudo apt-get update && sudo apt-get install --only-upgrade " + strings.Join(names, " ") } var engineVersionPattern = regexp.MustCompile(`^(?:\d+:)?(\d+(?:\.\d+)*)`) @@ -131,3 +187,16 @@ func Touches(simulation string) Touched { } return touched } + +// ContractPattern is an extended regular expression (for grep -E) that matches a line of +// an apt simulation that breaks the contract of the update: an install or upgrade of a +// package of the Docker family, or any removal. The unit's shell line runs the simulation +// again just before it installs, and installs nothing if a line matches. +func ContractPattern() string { + names := make([]string, 0, len(family)) + for name := range family { + names = append(names, strings.ReplaceAll(name, ".", `\.`)) + } + sort.Strings(names) + return `^(Inst (` + strings.Join(names, "|") + `)[: ]|Remv )` +} diff --git a/pkg/dockerpkg/dockerpkg_test.go b/pkg/dockerpkg/dockerpkg_test.go index 525e7f9..9587bd2 100644 --- a/pkg/dockerpkg/dockerpkg_test.go +++ b/pkg/dockerpkg/dockerpkg_test.go @@ -2,30 +2,40 @@ package dockerpkg import ( "reflect" + "regexp" "strings" "testing" ) func TestIsFamily(t *testing.T) { - for _, name := range []string{"docker-ce", "docker-ce-cli", "containerd.io", "docker-buildx-plugin", "docker-compose-plugin", "docker-ce-rootless-extras", "docker.io", "containerd"} { + for _, name := range []string{"docker-ce", "docker-ce-cli", "containerd.io", "docker-buildx-plugin", "docker-compose-plugin", "docker-ce-rootless-extras", "docker.io", "containerd", "containerd.io:armhf", "docker-ce:i386"} { if !IsFamily(name) { t.Errorf("IsFamily(%q) = false", name) } } - for _, name := range []string{"", "docker", "docker-ce-extra", "libc6", "runc", "dockerd", "containerd.io2", "podman", "snapd", "Docker-CE"} { + for _, name := range []string{"", "docker", "docker-ce-extra", "libc6", "runc", "dockerd", "containerd.io2", "podman", "snapd", "Docker-CE", ":armhf"} { if IsFamily(name) { t.Errorf("IsFamily(%q) = true", name) } } } +func TestRestartsEngine(t *testing.T) { + if !RestartsEngine([]string{"docker-buildx-plugin", "containerd.io"}) || !RestartsEngine([]string{"docker.io"}) || !RestartsEngine([]string{"docker-ce:amd64"}) { + t.Error("an engine package did not restart Docker") + } + if RestartsEngine([]string{"docker-buildx-plugin", "docker-compose-plugin", "docker-ce-cli", "docker-ce-rootless-extras"}) || RestartsEngine(nil) { + t.Error("a client or plugin restarted Docker") + } +} + func TestValidName(t *testing.T) { for _, name := range []string{"zlib1g", "libc6", "g++-12", "libstdc++6", "python3.11", "apache2-bin", "libc6:amd64", "a"} { if !ValidName(name) { t.Errorf("ValidName(%q) = false", name) } } - for _, name := range []string{"", "-rf", "x;rm -rf /", "a b", "$(id)", "`id`", "a\nb", "UPPER", "a&&b", ".hidden", "pkg=1.0", strings.Repeat("a", 129), "pkg/../x", "a|b"} { + for _, name := range []string{"", "-rf", "-oAPT::Foo", "x;rm -rf /", "a b", "$(id)", "`id`", "a\nb", "UPPER", "a&&b", ".hidden", "pkg=1.0", strings.Repeat("a", 129), "pkg/../x", "a|b"} { if ValidName(name) { t.Errorf("ValidName(%q) = true", name) } @@ -64,22 +74,44 @@ func TestOriginFromPolicy(t *testing.T) { } } -func TestManualCommandsAreConstants(t *testing.T) { - if got := ManualCommand(OriginDockerRepo); !strings.Contains(got, "--only-upgrade docker-ce docker-ce-cli containerd.io") || strings.Contains(got, " -y") { - t.Errorf("docker repository command = %q", got) +func TestCanSeeUpdates(t *testing.T) { + for origin, want := range map[Origin]bool{OriginDockerRepo: true, OriginDistribution: true, OriginSnap: false, OriginUnknown: false, "": false} { + if got := origin.CanSeeUpdates(); got != want { + t.Errorf("%q.CanSeeUpdates() = %v, want %v", origin, got, want) + } + } +} + +func TestManualCommandNamesWhatIsPending(t *testing.T) { + // nothing said: the usual set + if got := ManualCommand(OriginDockerRepo, nil); got != "sudo apt-get update && sudo apt-get install --only-upgrade containerd.io docker-buildx-plugin docker-ce docker-ce-cli docker-compose-plugin" { + t.Errorf("default docker repository command = %q", got) + } + // what apt said is behind, every name of it, sorted, without the architecture + got := ManualCommand(OriginDockerRepo, []string{"docker-ce", "docker-ce-rootless-extras", "docker-model-plugin", "containerd.io:amd64"}) + if got != "sudo apt-get update && sudo apt-get install --only-upgrade containerd.io docker-ce docker-ce-rootless-extras docker-model-plugin" { + t.Errorf("pending command = %q", got) } - if got := ManualCommand(OriginDistribution); !strings.Contains(got, "docker.io") { + if got := ManualCommand(OriginDistribution, []string{"docker.io", "containerd", "docker-compose-v2"}); got != "sudo apt-get update && sudo apt-get install --only-upgrade containerd docker-compose-v2 docker.io" { t.Errorf("distribution command = %q", got) } - if got := ManualCommand(OriginSnap); got != "sudo snap refresh docker" { + if got := ManualCommand(OriginDistribution, nil); !strings.HasSuffix(got, "--only-upgrade docker.io") { + t.Errorf("default distribution command = %q", got) + } + if got := ManualCommand(OriginSnap, []string{"docker-ce"}); got != "sudo snap refresh docker" { t.Errorf("snap command = %q", got) } - if got := ManualCommand(OriginUnknown); got != "" { + if got := ManualCommand(OriginUnknown, []string{"docker-ce"}); got != "" { t.Errorf("unknown origin command = %q, want none", got) } - if got := ManualCommand(Origin("x; rm -rf /")); got != "" { + if got := ManualCommand(Origin("x; rm -rf /"), nil); got != "" { t.Errorf("a made-up origin gave a command: %q", got) } + // nothing apt printed that is not a family name gets into the line + got = ManualCommand(OriginDockerRepo, []string{"docker-ce", "x; rm -rf /", "$(id)", "-oAPT::Foo", "libc6"}) + if strings.ContainsAny(got, ";$()") || strings.Contains(got, "libc6") || strings.Contains(got, "-oAPT") || !strings.Contains(got, "docker-ce") { + t.Errorf("a name outside the family reached the command: %q", got) + } } func TestEngineVersion(t *testing.T) { @@ -102,13 +134,13 @@ func TestTouches(t *testing.T) { simulation := `Reading package lists... Inst libc6 [2.35-0ubuntu3.10] (2.35-0ubuntu3.11 Ubuntu:22.04/jammy-updates [amd64]) Inst docker-ce [5:29.8.1-1~ubuntu.22.04~jammy] (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64]) -Inst containerd.io [2.3.5-1~ubuntu.22.04~jammy] (2.3.6-1~ubuntu.22.04~jammy Docker CE:jammy [amd64]) +Inst containerd.io:armhf [2.3.5-1~ubuntu.22.04~jammy] (2.3.6-1~ubuntu.22.04~jammy Docker CE:jammy [armhf]) Remv oldthing [1.0] Conf docker-ce (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64]) Inst docker-ce-extra-thing [1] (2 x [amd64]) ` got := Touches(simulation) - want := Touched{Docker: []string{"docker-ce", "containerd.io"}, Removed: []string{"oldthing"}} + want := Touched{Docker: []string{"docker-ce", "containerd.io:armhf"}, Removed: []string{"oldthing"}} if !reflect.DeepEqual(got, want) { t.Errorf("Touches() = %#v, want %#v", got, want) } @@ -116,3 +148,34 @@ Inst docker-ce-extra-thing [1] (2 x [amd64]) t.Errorf("Touches(clean) = %#v", clean) } } + +func TestContractPatternMatchesWhatTouchesFinds(t *testing.T) { + pattern := regexp.MustCompile(ContractPattern()) + for _, line := range []string{ + "Inst docker-ce [5:29.8.1] (5:29.8.2 Docker CE:jammy [amd64])", + "Inst containerd.io [2.3.5] (2.3.6 Docker CE [amd64])", + "Inst containerd.io:armhf [2.3.5] (2.3.6 Docker CE [armhf])", + "Inst docker.io [26.1.5] (26.1.6 Debian [amd64])", + "Inst docker-ce-rootless-extras (5:29.8.2 Docker CE [amd64])", + "Remv anything [1.0]", + } { + if !pattern.MatchString(line) { + t.Errorf("the pattern does not match %q", line) + } + } + for _, line := range []string{ + "Inst libc6 [2.35-0ubuntu3.10] (2.35-0ubuntu3.11 Ubuntu [amd64])", + "Inst docker-ce-extra-thing [1] (2 x [amd64])", + "Inst containerdxio [1] (2 x [amd64])", // the dot is a dot + "Conf docker-ce (5:29.8.2 Docker CE [amd64])", + "Reading package lists...", + } { + if pattern.MatchString(line) { + t.Errorf("the pattern matches %q", line) + } + } + // no character of it needs more than single quotes in a shell + if strings.Contains(ContractPattern(), "'") { + t.Errorf("the pattern holds a single quote: %q", ContractPattern()) + } +} diff --git a/route/v1/system.go b/route/v1/system.go index 7d4f8e8..15626bc 100644 --- a/route/v1/system.go +++ b/route/v1/system.go @@ -114,7 +114,8 @@ func StartSystemPackageUpdate(ctx echo.Context) error { if errors.Is(err, service.ErrSystemPackageUpdateRunning) || errors.Is(err, service.ErrSystemMaintenanceBusy) || errors.Is(err, service.ErrSystemPackageNothingToUpdate) || - errors.Is(err, service.ErrSystemPackageTouchesDocker) { + errors.Is(err, service.ErrSystemPackageTouchesDocker) || + errors.Is(err, service.ErrSystemPackageListChanged) { return ctx.JSON(http.StatusConflict, model.Result{ Success: http.StatusConflict, Message: err.Error(), diff --git a/route/v1/system_package_test.go b/route/v1/system_package_test.go index c3e6f24..8e6d3be 100644 --- a/route/v1/system_package_test.go +++ b/route/v1/system_package_test.go @@ -178,6 +178,7 @@ func TestStartSystemPackageUpdateRefusalsAreConflicts(t *testing.T) { "another maintenance": service.ErrSystemMaintenanceBusy, "nothing but Docker": service.ErrSystemPackageNothingToUpdate, "a list that touches Docker": service.ErrSystemPackageTouchesDocker, + "a list that changed": service.ErrSystemPackageListChanged, } { fakeSystem := &fakeSystemPackageService{startErr: fmt.Errorf("%w: detail", refusal)} service.MyService = fakeSystemPackageRepository{system: fakeSystem} diff --git a/service/dpkg_lock_linux.go b/service/dpkg_lock_linux.go new file mode 100644 index 0000000..e15bcfc --- /dev/null +++ b/service/dpkg_lock_linux.go @@ -0,0 +1,39 @@ +//go:build linux + +package service + +import ( + "os" + "syscall" +) + +// dpkgLockFiles are the files apt and dpkg lock while they work: the frontend lock held for +// a whole apt run, dpkg's own, and the lists' lock held while the index is refreshed. +var dpkgLockFiles = []string{"/var/lib/dpkg/lock-frontend", "/var/lib/dpkg/lock", "/var/lib/apt/lists/lock"} + +// dpkgLockHeld says whether a package manager is working: one of its lock files is locked +// by another process. +func dpkgLockHeld() bool { + return anyLockHeld(dpkgLockFiles) +} + +// anyLockHeld asks the kernel, with F_GETLK, whether another process holds a lock on any +// of the files. apt and dpkg take POSIX record locks (fcntl), which flock(1) and flock(2) +// do not see: they are a different kind of lock. The probe opens the file read-only, takes +// nothing and creates nothing; a file that is not there, or cannot be opened, is not held. +// A read lock is asked for, which conflicts with the write lock they hold. +func anyLockHeld(paths []string) bool { + for _, path := range paths { + file, err := os.Open(path) + if err != nil { + continue + } + lock := syscall.Flock_t{Type: syscall.F_RDLCK, Whence: 0, Start: 0, Len: 0} + err = syscall.FcntlFlock(file.Fd(), syscall.F_GETLK, &lock) + _ = file.Close() + if err == nil && lock.Type != syscall.F_UNLCK { + return true + } + } + return false +} diff --git a/service/dpkg_lock_linux_test.go b/service/dpkg_lock_linux_test.go new file mode 100644 index 0000000..f6401a4 --- /dev/null +++ b/service/dpkg_lock_linux_test.go @@ -0,0 +1,86 @@ +//go:build linux + +package service + +import ( + "bufio" + "os" + "os/exec" + "path/filepath" + "syscall" + "testing" + "time" +) + +// TestHoldDpkgLockHelper is not a test: the test below runs it in another process, which +// takes the write lock apt and dpkg take (fcntl, a POSIX record lock) and holds it until it +// is killed. A lock held by the probing process itself would not show: locks belong to a +// process, and F_GETLK never reports one's own. +func TestHoldDpkgLockHelper(t *testing.T) { + path := os.Getenv("CASAOS_TEST_HOLD_LOCK") + if path == "" { + t.Skip("run by TestAnyLockHeldSeesAnotherProcessesLock") + } + file, err := os.OpenFile(path, os.O_RDWR, 0) + if err != nil { + t.Fatal(err) + } + lock := syscall.Flock_t{Type: syscall.F_WRLCK} + if err := syscall.FcntlFlock(file.Fd(), syscall.F_SETLK, &lock); err != nil { + t.Fatal(err) + } + os.Stdout.WriteString("locked\n") + time.Sleep(time.Minute) +} + +func TestAnyLockHeldSeesAnotherProcessesLock(t *testing.T) { + path := filepath.Join(t.TempDir(), "lock-frontend") + if err := os.WriteFile(path, nil, 0o600); err != nil { + t.Fatal(err) + } + missing := filepath.Join(t.TempDir(), "not-there") + if anyLockHeld([]string{missing, path}) { + t.Fatal("a lock nobody holds, and a file that is not there, were reported held") + } + + holder := exec.Command(os.Args[0], "-test.run=^TestHoldDpkgLockHelper$") + holder.Env = append(os.Environ(), "CASAOS_TEST_HOLD_LOCK="+path) + stdout, err := holder.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := holder.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = holder.Process.Kill(); _ = holder.Wait() }) + reader := bufio.NewReader(stdout) + for { + line, err := reader.ReadString('\n') + if err != nil { + t.Fatalf("the lock holder died before it locked: %v", err) + } + if line == "locked\n" { + break + } + } + + if !anyLockHeld([]string{missing, path}) { + t.Fatal("a write lock held by another process was not seen") + } + // the probe took nothing and created nothing: it can be asked again, and the file is as it was + if !anyLockHeld([]string{path}) { + t.Fatal("the second look at a held lock did not see it") + } + if _, err := os.Stat(missing); !os.IsNotExist(err) { + t.Fatalf("the probe created %s: %v", missing, err) + } + + _ = holder.Process.Kill() + _ = holder.Wait() + for i := 0; i < 50 && anyLockHeld([]string{path}); i++ { + time.Sleep(20 * time.Millisecond) + } + if anyLockHeld([]string{path}) { + t.Fatal("the lock is still reported after its holder is gone") + } +} diff --git a/service/dpkg_lock_other.go b/service/dpkg_lock_other.go new file mode 100644 index 0000000..f6b76ba --- /dev/null +++ b/service/dpkg_lock_other.go @@ -0,0 +1,6 @@ +//go:build !linux + +package service + +// dpkgLockHeld: only a Linux host has dpkg's lock to look at. +func dpkgLockHeld() bool { return false } diff --git a/service/system_package.go b/service/system_package.go index 5255b34..d280b06 100644 --- a/service/system_package.go +++ b/service/system_package.go @@ -94,6 +94,8 @@ type systemPackageUpdater struct { mkdirAll func(string, os.FileMode) error stat func(string) (os.FileInfo, error) now func() time.Time + // dpkgLocked says whether a package manager holds dpkg's lock; nil means never. + dpkgLocked func() bool } func newSystemPackageUpdater() *systemPackageUpdater { @@ -112,6 +114,7 @@ func newSystemPackageUpdater() *systemPackageUpdater { mkdirAll: os.MkdirAll, stat: os.Stat, now: time.Now, + dpkgLocked: dpkgLockHeld, } } @@ -209,7 +212,7 @@ func (u *systemPackageUpdater) check() (SystemPackageUpdates, error) { } var dockerUpdates []SystemPackageUpdate - result.Updates, dockerUpdates = splitDockerUpdates(parseAPTUpgradeSimulation(string(output))) + result.Updates, dockerUpdates = splitDockerUpdates(parseAPTUpgradeSimulation(string(output)), u.engineInstalled(ctx)) result.Count = len(result.Updates) result.Docker = u.dockerInfo(ctx, dockerUpdates) now := u.now().UTC() @@ -242,15 +245,33 @@ func (u *systemPackageUpdater) startUpdate() (SystemPackageUpdateStatus, error) status.Error = reason return status, fmt.Errorf("%w: %s", ErrSystemMaintenanceBusy, reason) } + + // The two simulations take a while on a small box: not with the lock held, so that the + // status and the check can still answer, and the box is looked at again afterwards. + u.mu.Unlock() listCtx, cancelList := context.WithTimeout(context.Background(), systemPackageCheckTimeout) names, err := u.upgradeNames(listCtx, support) cancelList() + u.mu.Lock() if err != nil { status.Error = err.Error() return status, err } + if u.isRunning() { + status.State = systemPackageUpdateStateRunning + return u.statusLocked(support), ErrSystemPackageUpdateRunning + } + if reason, busy := u.maintenanceBusy(context.Background(), systemPackageUpdateUnit); busy { + status.Error = reason + return status, fmt.Errorf("%w: %s", ErrSystemMaintenanceBusy, reason) + } logPath := u.logPath() + args, err := systemPackageUpdateArgs(support.aptPath, logPath, names) + if err != nil { + status.Error = err.Error() + return status, err + } if err := u.mkdirAll(filepath.Dir(logPath), 0o755); err != nil { markSystemPackageUpdateFailed(&status, fmt.Sprintf("prepare package update log: %v", err), u.now) return status, err @@ -261,12 +282,6 @@ func (u *systemPackageUpdater) startUpdate() (SystemPackageUpdateStatus, error) markSystemPackageUpdateFailed(&status, fmt.Sprintf("prepare package update log: %v", err), u.now) return status, err } - - args, err := systemPackageUpdateArgs(support.aptPath, logPath, names) - if err != nil { - markSystemPackageUpdateFailed(&status, err.Error(), u.now) - return status, err - } if output, err := u.start(support.systemdPath, systemPackageUpdateUnit, args...); err != nil { failure := fmt.Sprintf("CASAOS_PACKAGE_UPDATE_FAILED %s 1\n%s\n", u.now().UTC().Format(time.RFC3339), trimSystemPackageOutput(output)) _ = u.writeFile(logPath, []byte(failure), 0o644) @@ -436,7 +451,16 @@ func systemPackageUpdateCommand(aptPath, logPath string, names []string) (string } quotedLogPath := shellQuote(logPath) quotedAPTPath := shellQuote(aptPath) - return "set -o pipefail; exec >> " + quotedLogPath + " 2>&1; printf 'CASAOS_PACKAGE_UPDATE_STARTED %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; " + quotedAPTPath + " -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold -o DPkg::Lock::Timeout=120 install --only-upgrade --no-install-recommends " + strings.Join(quotedNames, " ") + "; status=$?; if [ -f /var/run/reboot-required ]; then printf 'CASAOS_PACKAGE_UPDATE_REBOOT_REQUIRED\\n'; fi; if [ \"$status\" -eq 0 ]; then printf 'CASAOS_PACKAGE_UPDATE_SUCCESS %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; else printf 'CASAOS_PACKAGE_UPDATE_FAILED %s %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\" \"$status\"; fi; exit \"$status\"", nil + installArgs := "install --only-upgrade --no-install-recommends " + strings.Join(quotedNames, " ") + // The check made when the update was asked for may be minutes old by the time the unit + // has the lock (an index refreshed since can make a listed package want a newer Docker): + // the same simulation is made again just before the install, and nothing is installed if + // it now touches Docker's packages or removes one. + guarded := "plan=\"$(" + quotedAPTPath + " -s --no-remove -o Debug::NoLocking=true -o Dpkg::Use-Pty=0 " + installArgs + " 2>&1)\"; " + + "if printf '%s\\n' \"$plan\" | grep -Eq " + shellQuote(dockerpkg.ContractPattern()) + "; then " + + "printf 'CASAOS_PACKAGE_UPDATE_GUARD the list now changes Docker or removes a package: nothing was installed\\n'; status=1; else " + + quotedAPTPath + " -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold -o DPkg::Lock::Timeout=120 " + installArgs + "; status=$?; fi; " + return "set -o pipefail; exec >> " + quotedLogPath + " 2>&1; printf 'CASAOS_PACKAGE_UPDATE_STARTED %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; " + guarded + "if [ -f /var/run/reboot-required ]; then printf 'CASAOS_PACKAGE_UPDATE_REBOOT_REQUIRED\\n'; fi; if [ \"$status\" -eq 0 ]; then printf 'CASAOS_PACKAGE_UPDATE_SUCCESS %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; else printf 'CASAOS_PACKAGE_UPDATE_FAILED %s %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\" \"$status\"; fi; exit \"$status\"", nil } type parsedSystemPackageUpdateLog struct { diff --git a/service/system_package_docker.go b/service/system_package_docker.go index 919db20..fcc9e4e 100644 --- a/service/system_package_docker.go +++ b/service/system_package_docker.go @@ -4,7 +4,7 @@ import ( "context" "errors" "fmt" - "os/exec" + "regexp" "strings" "github.com/ReCasaOS/CasaOS/common" @@ -15,11 +15,14 @@ import ( // restarts the Docker daemon and stops every container until it is back (the apps that // have a restart policy start again by themselves, the others stay stopped), and an apt // upgrade of everything used to do that without a word. The update now installs the -// packages the check lists, minus Docker's, and Docker is shown on a line of its own. +// packages the check lists, minus Docker's, and Docker is shown on a line of its own. This +// holds only on a box that has a Docker engine: containerd on a Kubernetes node, say, is +// an ordinary package there. var ( - ErrSystemPackageNothingToUpdate = errors.New("there is nothing to update here: what is left is Docker's, which is updated on its own") + ErrSystemPackageNothingToUpdate = errors.New("there is nothing to update") ErrSystemPackageTouchesDocker = errors.New("this update would also change Docker's packages") + ErrSystemPackageListChanged = errors.New("the update is no longer the list that was checked") ErrSystemMaintenanceBusy = errors.New("another update or package operation is running on this box") ) @@ -33,24 +36,25 @@ type SystemPackageDocker struct { // Updates is what apt offers for Docker's packages; none of it is installed by the // System packages update. Updates []SystemPackageUpdate `json:"updates"` + // RestartsDocker is whether installing those updates restarts the daemon: the engine's + // own packages do, a plugin or the client do not. + RestartsDocker bool `json:"restarts_docker"` // ManualCommand is what to type to update Docker oneself; empty when this does not // know a command to stand behind. ManualCommand string `json:"manual_command,omitempty"` } -const ( - systemDockerUpdateUnit = "casaos-docker-update.service" // reserved for an update of Docker on its own - dpkgLockFile = "/var/lib/dpkg/lock-frontend" -) +const systemDockerUpdateUnit = "casaos-docker-update.service" // reserved for an update of Docker on its own // systemMaintenanceUnits are the transient units that change the box's packages or the -// ReCasaOS release: at most one of them runs at a time, and none while dpkg's lock is held. +// ReCasaOS release: at most one of them runs at a time, and none while a package manager +// holds dpkg's lock. var systemMaintenanceUnits = []string{common.UPDATE_UNIT + ".service", systemPackageUpdateUnit, systemDockerUpdateUnit} // maintenanceBusy says whether the box is being changed by something else than the unit -// named in except (the caller's own, which it checks in its own way): another of the -// units above is active, or a package manager holds dpkg's lock. Systemd or flock not -// answering is not busy: a refusal that cannot be explained helps nobody. +// named in except (the caller's own, which it checks in its own way): another of the units +// above is active, or, on a host that has dpkg, a package manager holds its lock. Systemd +// not answering is not busy: a refusal that cannot be explained helps nobody. func (u *systemPackageUpdater) maintenanceBusy(ctx context.Context, except string) (string, bool) { for _, unit := range systemMaintenanceUnits { if unit == except { @@ -65,11 +69,9 @@ func (u *systemPackageUpdater) maintenanceBusy(ctx context.Context, except strin return fmt.Sprintf("%s is running", strings.TrimSuffix(unit, ".service")), true } } - if _, err := u.command(ctx, "flock", "--nonblock", "--exclusive", dpkgLockFile, "true"); err != nil { - var exit *exec.ExitError - if errors.As(err, &exit) { - return "another package manager holds dpkg's lock", true - } + // the lock is dpkg's: a host that has no dpkg has none to hold + if u.dpkgLocked != nil && u.support().supported && u.dpkgLocked() { + return "another package manager holds dpkg's lock", true } return "", false } @@ -81,13 +83,28 @@ func (s *systemService) MaintenanceBusy() bool { return busy } -// splitDockerUpdates separates what apt would upgrade in Docker's family from the rest. -func splitDockerUpdates(all []SystemPackageUpdate) (rest, docker []SystemPackageUpdate) { +// engineInstalled says whether this box has a Docker engine at all: docker-ce, the +// distribution's docker.io, or the snap. +func (u *systemPackageUpdater) engineInstalled(ctx context.Context) bool { + if u.installedVersion(ctx, "docker-ce") != "" || u.installedVersion(ctx, "docker.io") != "" { + return true + } + _, err := u.command(ctx, "snap", "list", "docker") + return err == nil +} + +// splitDockerUpdates separates what apt would upgrade in Docker's family from the rest, +// when the box has a Docker engine; without one everything is the rest. A package that apt +// would install new rather than upgrade is not an update and is left out of both. +func splitDockerUpdates(all []SystemPackageUpdate, engine bool) (rest, docker []SystemPackageUpdate) { rest, docker = []SystemPackageUpdate{}, []SystemPackageUpdate{} for _, update := range all { - if dockerpkg.IsFamily(update.Name) { + switch { + case update.CurrentVersion == "": + continue + case engine && dockerpkg.IsFamily(update.Name): docker = append(docker, update) - } else { + default: rest = append(rest, update) } } @@ -103,9 +120,8 @@ func (u *systemPackageUpdater) dockerInfo(ctx context.Context, pending []SystemP } switch { case u.installedVersion(ctx, "docker-ce") != "": - version := u.installedVersion(ctx, "docker-ce") info.Installed = true - info.Version = dockerpkg.EngineVersion(version) + info.Version = dockerpkg.EngineVersion(u.installedVersion(ctx, "docker-ce")) policy, _ := u.command(ctx, "apt-cache", "policy", "docker-ce") info.Origin = string(dockerpkg.OriginFromPolicy(string(policy))) case u.installedVersion(ctx, "docker.io") != "": @@ -121,19 +137,29 @@ func (u *systemPackageUpdater) dockerInfo(ctx context.Context, pending []SystemP if !info.Installed && len(info.Updates) == 0 { return nil } - info.ManualCommand = dockerpkg.ManualCommand(dockerpkg.Origin(info.Origin)) + names := make([]string, 0, len(info.Updates)) + for _, update := range info.Updates { + names = append(names, update.Name) + } + info.RestartsDocker = dockerpkg.RestartsEngine(names) + info.ManualCommand = dockerpkg.ManualCommand(dockerpkg.Origin(info.Origin), names) return info } -// installedVersion is the dpkg version of an installed package, or "". +// installedVersion is the dpkg version of a package that is on the box (installed, or held, +// or half-configured: anything but removed, purged or never there), or "". func (u *systemPackageUpdater) installedVersion(ctx context.Context, name string) string { output, err := u.command(ctx, "dpkg-query", "-W", "-f=${db:Status-Abbrev}\t${Version}\n", name) if err != nil { return "" } for _, line := range strings.Split(string(output), "\n") { - status, version, ok := strings.Cut(strings.TrimSpace(line), "\t") - if ok && strings.HasPrefix(strings.TrimSpace(status), "ii") { + status, version, ok := strings.Cut(strings.TrimRight(line, "\r"), "\t") + status = strings.TrimSpace(status) + // dpkg's abbreviation is the wanted state, then the state: "ii", "hi" for a held + // package, "iF" half-configured; "un" is not installed and "rc" only has its + // configuration files left + if ok && len(status) >= 2 && status[1] != 'n' && status[1] != 'c' && strings.TrimSpace(version) != "" { return strings.TrimSpace(version) } } @@ -145,33 +171,47 @@ func systemPackageSimulateArgs(rest ...string) []string { return append([]string{"-s", "--no-remove", "-o", "Debug::NoLocking=true", "-o", "Dpkg::Use-Pty=0"}, rest...) } +var aptRemovalRefused = regexp.MustCompile(`(?i)remove is disabled|packages need to be removed`) + // upgradeNames is the explicit list the update installs: what a plain upgrade would -// upgrade, minus Docker's packages. It is the contract of the exclusion: the list is -// made of names apt printed and checked as package names, and a second simulation of -// installing exactly that list must change nothing of Docker's and remove nothing. +// upgrade, minus Docker's packages when the box has Docker. It is the contract of the +// exclusion: the list is made of names apt printed and checked as package names, and a +// second simulation of installing exactly that list must change nothing of Docker's, +// remove nothing and install nothing that is not an upgrade. The unit's own shell line +// runs the same simulation again right before it installs. func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemPackageSupport) ([]string, error) { output, err := u.command(ctx, support.aptPath, systemPackageSimulateArgs("upgrade")...) if err != nil { return nil, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) } + engine := u.engineInstalled(ctx) var names []string + var keptDocker int for _, update := range parseAPTUpgradeSimulation(string(output)) { - if dockerpkg.IsFamily(update.Name) || update.CurrentVersion == "" { - // Docker's own, or a package apt would install new rather than upgrade - continue - } - if !dockerpkg.ValidName(update.Name) { - return nil, fmt.Errorf("apt named a package that is not a package name: %q", update.Name) + switch { + case update.CurrentVersion == "": + // a package apt would install new rather than upgrade + case engine && dockerpkg.IsFamily(update.Name): + keptDocker++ + case !dockerpkg.ValidName(update.Name): + // not a name that may go into a command line: left out, not a reason to stop + default: + names = append(names, update.Name) } - names = append(names, update.Name) } if len(names) == 0 { + if keptDocker > 0 { + return nil, fmt.Errorf("%w: what is left is Docker's, which is updated on its own", ErrSystemPackageNothingToUpdate) + } return nil, ErrSystemPackageNothingToUpdate } args := systemPackageSimulateArgs(append([]string{"install", "--only-upgrade", "--no-install-recommends"}, names...)...) output, err = u.command(ctx, support.aptPath, args...) if err != nil { + if aptRemovalRefused.Match(output) { + return nil, fmt.Errorf("%w: it would have to remove packages, which the System packages update never does", ErrSystemPackageListChanged) + } return nil, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) } touched := dockerpkg.Touches(string(output)) @@ -179,7 +219,16 @@ func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemP return nil, fmt.Errorf("%w: %s", ErrSystemPackageTouchesDocker, strings.Join(touched.Docker, ", ")) } if len(touched.Removed) > 0 { - return nil, fmt.Errorf("this update would remove packages (%s), which the System packages update never does", strings.Join(touched.Removed, ", ")) + return nil, fmt.Errorf("%w: it would remove %s, which the System packages update never does", ErrSystemPackageListChanged, strings.Join(touched.Removed, ", ")) + } + var added []string + for _, update := range parseAPTUpgradeSimulation(string(output)) { + if update.CurrentVersion == "" { + added = append(added, update.Name) + } + } + if len(added) > 0 { + return nil, fmt.Errorf("%w: it would install %s, which are not upgrades", ErrSystemPackageListChanged, strings.Join(added, ", ")) } return names, nil } diff --git a/service/system_package_docker_test.go b/service/system_package_docker_test.go index 6a11a15..06f6c32 100644 --- a/service/system_package_docker_test.go +++ b/service/system_package_docker_test.go @@ -5,24 +5,29 @@ import ( "errors" "os" "os/exec" + "path/filepath" + "runtime" "strings" "testing" "github.com/ReCasaOS/CasaOS/common" + "github.com/ReCasaOS/CasaOS/pkg/dockerpkg" ) // aptBox answers the commands the System packages updater runs, from its fields. type aptBox struct { upgradeSimulation string // apt-get -s upgrade installSimulation string // apt-get -s install --only-upgrade + installFailure string // when set, that simulation exits non-zero with this output activeUnits map[string]bool - dpkgLocked bool - flockMissing bool dockerCE string // the dpkg version of docker-ce, "" when it is not installed + dockerCEStatus string // dpkg's abbreviation, "ii" unless set dockerIO string policy string snap bool - calls []string + // onInstallSimulation runs when the list is simulated: something else may start meanwhile. + onInstallSimulation func() + calls []string } func (b *aptBox) command(_ context.Context, name string, args ...string) ([]byte, error) { @@ -33,18 +38,14 @@ func (b *aptBox) command(_ context.Context, name string, args ...string) ([]byte return []byte("active\n"), nil } return []byte("inactive\n"), nil - case "flock": - if b.flockMissing { - return nil, &exec.Error{Name: "flock", Err: exec.ErrNotFound} - } - if b.dpkgLocked { - return nil, &exec.ExitError{} - } - return nil, nil case "dpkg-query": pkg := args[len(args)-1] + status := b.dockerCEStatus + if status == "" { + status = "ii" + } if pkg == "docker-ce" && b.dockerCE != "" { - return []byte("ii \t" + b.dockerCE + "\n"), nil + return []byte(status + " \t" + b.dockerCE + "\n"), nil } if pkg == "docker.io" && b.dockerIO != "" { return []byte("ii \t" + b.dockerIO + "\n"), nil @@ -64,6 +65,12 @@ func (b *aptBox) command(_ context.Context, name string, args ...string) ([]byte case len(args) > 0 && args[0] == "update": return nil, nil case strings.Contains(joined, " -s ") && strings.Contains(joined, " install "): + if b.onInstallSimulation != nil { + b.onInstallSimulation() + } + if b.installFailure != "" { + return []byte(b.installFailure), errors.New("exit status 100") + } return []byte(b.installSimulation), nil case strings.Contains(joined, " -s "): return []byte(b.upgradeSimulation), nil @@ -76,15 +83,20 @@ const ( simLibc = "Inst libc6 [2.35-0ubuntu3.10] (2.35-0ubuntu3.11 Ubuntu:22.04/jammy-updates [amd64])\n" simZlib = "Inst zlib1g [1:1.2.11] (1:1.2.12 Ubuntu:22.04/jammy-updates [amd64])\n" simDockerC = "Inst docker-ce [5:29.8.1-1~ubuntu.22.04~jammy] (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64])\n" + simRootles = "Inst docker-ce-rootless-extras [5:29.8.1-1~ubuntu.22.04~jammy] (5:29.8.2-1~ubuntu.22.04~jammy Docker CE:jammy [amd64])\n" + simPlugin = "Inst docker-compose-plugin [2.40.0-1~ubuntu.22.04~jammy] (2.40.1-1~ubuntu.22.04~jammy Docker CE:jammy [amd64])\n" simContain = "Inst containerd.io [2.3.5-1~ubuntu.22.04~jammy] (2.3.6-1~ubuntu.22.04~jammy Docker CE:jammy [amd64])\n" + simNew = "Inst brand-new-dependency (1 Ubuntu:22.04 [amd64])\n" dockerRepo = "docker-ce:\n Installed: 5:29.8.1-1~ubuntu.22.04~jammy\n Candidate: 5:29.8.2-1~ubuntu.22.04~jammy\n Version table:\n 5:29.8.2-1~ubuntu.22.04~jammy 500\n 500 https://download.docker.com/linux/ubuntu jammy/stable amd64 Packages\n *** 5:29.8.1-1~ubuntu.22.04~jammy 100\n 100 /var/lib/dpkg/status\n" ) +const dockerCE = "5:29.8.1-1~ubuntu.22.04~jammy" + func TestSystemPackageCheckListsDockerOnALineOfItsOwn(t *testing.T) { updater := newTestSystemPackageUpdater(t) box := &aptBox{ - upgradeSimulation: simLibc + simDockerC + simContain + simZlib, - dockerCE: "5:29.8.1-1~ubuntu.22.04~jammy", + upgradeSimulation: simLibc + simDockerC + simContain + simRootles + simZlib + simNew, + dockerCE: dockerCE, policy: dockerRepo, } updater.command = box.command @@ -94,23 +106,35 @@ func TestSystemPackageCheckListsDockerOnALineOfItsOwn(t *testing.T) { t.Fatalf("check() error = %v", err) } if got.Count != 2 || len(got.Updates) != 2 || got.Updates[0].Name != "libc6" || got.Updates[1].Name != "zlib1g" { - t.Fatalf("the update list holds %#v, want libc6 and zlib1g only", got.Updates) + t.Fatalf("the update list holds %#v, want libc6 and zlib1g only: no Docker, and no package that is not an upgrade", got.Updates) } - if got.Docker == nil || !got.Docker.Installed || got.Docker.Version != "29.8.1" || got.Docker.Origin != "docker-repository" { + if got.Docker == nil || !got.Docker.Installed || got.Docker.Version != "29.8.1" || got.Docker.Origin != "docker-repository" || !got.Docker.RestartsDocker { t.Fatalf("Docker = %#v", got.Docker) } - if len(got.Docker.Updates) != 2 || got.Docker.Updates[0].Name != "containerd.io" || got.Docker.Updates[1].Name != "docker-ce" { + if len(got.Docker.Updates) != 3 { t.Fatalf("Docker's updates = %#v", got.Docker.Updates) } - if !strings.Contains(got.Docker.ManualCommand, "--only-upgrade docker-ce docker-ce-cli containerd.io") { - t.Fatalf("manual command = %q", got.Docker.ManualCommand) + // the command updates everything the line lists, so that running it clears the line + for _, name := range []string{"docker-ce", "containerd.io", "docker-ce-rootless-extras"} { + if !strings.Contains(got.Docker.ManualCommand, name) { + t.Errorf("manual command %q lacks %s", got.Docker.ManualCommand, name) + } + } +} + +func TestSystemPackageCheckSaysWhenDockerRestartsAndWhenItDoesNot(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: simPlugin, dockerCE: dockerCE, policy: dockerRepo}).command + + got, err := updater.check() + if err != nil || got.Docker == nil || len(got.Docker.Updates) != 1 || got.Docker.RestartsDocker { + t.Fatalf("check() = %#v, %v: a plugin does not restart Docker", got.Docker, err) } } func TestSystemPackageCheckHasNoDockerLineWithoutDocker(t *testing.T) { updater := newTestSystemPackageUpdater(t) - box := &aptBox{upgradeSimulation: simLibc} - updater.command = box.command + updater.command = (&aptBox{upgradeSimulation: simLibc}).command got, err := updater.check() if err != nil || got.Count != 1 || got.Docker != nil { @@ -118,6 +142,17 @@ func TestSystemPackageCheckHasNoDockerLineWithoutDocker(t *testing.T) { } } +func TestSystemPackageCheckLeavesContainerdAloneOnABoxWithoutDocker(t *testing.T) { + // a Kubernetes node: containerd is an ordinary package there + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: simLibc + simContain}).command + + got, err := updater.check() + if err != nil || got.Count != 2 || got.Docker != nil { + t.Fatalf("check() = %#v, %v: containerd.io belongs in the list when there is no Docker engine", got, err) + } +} + func TestSystemPackageCheckKnowsWhereDockerCameFrom(t *testing.T) { cases := map[string]struct { box aptBox @@ -126,26 +161,29 @@ func TestSystemPackageCheckKnowsWhereDockerCameFrom(t *testing.T) { "distribution": {aptBox{dockerIO: "26.1.5+dfsg1-9+deb13u1"}, "distribution"}, "snap": {aptBox{snap: true}, "snap"}, "by hand": {aptBox{dockerCE: "5:24.0.5-1", policy: "docker-ce:\n Version table:\n *** 5:24.0.5-1 100\n 100 /var/lib/dpkg/status\n"}, "unknown"}, + "held": {aptBox{dockerCE: dockerCE, dockerCEStatus: "hi", policy: dockerRepo}, "docker-repository"}, } for name, c := range cases { box := c.box updater := newTestSystemPackageUpdater(t) updater.command = box.command got, err := updater.check() - if err != nil || got.Docker == nil || got.Docker.Origin != c.origin { + if err != nil || got.Docker == nil || got.Docker.Origin != c.origin || !got.Docker.Installed { t.Errorf("%s: check() = %#v, %v; want origin %q", name, got.Docker, err, c.origin) } } } -func TestSystemPackageCheckShowsDockerPendingEvenWhenItIsNotInstalledByDpkg(t *testing.T) { - updater := newTestSystemPackageUpdater(t) - box := &aptBox{upgradeSimulation: simContain} - updater.command = box.command - - got, err := updater.check() - if err != nil || got.Count != 0 || got.Docker == nil || len(got.Docker.Updates) != 1 { - t.Fatalf("check() = %#v, %v", got, err) +func TestInstalledVersionReadsDpkgStatuses(t *testing.T) { + for status, want := range map[string]string{"ii": "1.0", "hi": "1.0", "iF": "1.0", "it": "1.0", "rc": "", "un": "", "pn": "", "": ""} { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{dockerCE: "1.0", dockerCEStatus: status}).command + if status == "" { + updater.command = (&aptBox{}).command + } + if got := updater.installedVersion(context.Background(), "docker-ce"); got != want { + t.Errorf("status %q: installedVersion = %q, want %q", status, got, want) + } } } @@ -165,24 +203,29 @@ func startedCommand(t *testing.T, updater *systemPackageUpdater) string { func TestSystemPackageUpdateInstallsTheListAndLeavesDockerOut(t *testing.T) { updater := newTestSystemPackageUpdater(t) box := &aptBox{ - upgradeSimulation: simLibc + simDockerC + simContain + simZlib + "Inst brand-new-dependency (1 Ubuntu:22.04 [amd64])\n", + upgradeSimulation: simLibc + simDockerC + simContain + simZlib + simNew, installSimulation: simLibc + simZlib, + dockerCE: dockerCE, + policy: dockerRepo, } updater.command = box.command started := startedCommand(t, updater) for _, want := range []string{ - "install --only-upgrade --no-install-recommends 'libc6' 'zlib1g';", + "install --only-upgrade --no-install-recommends 'libc6' 'zlib1g'; status=$?; fi;", "-o DPkg::Lock::Timeout=120", "--no-remove", + "CASAOS_PACKAGE_UPDATE_GUARD", } { if !strings.Contains(started, want) { t.Errorf("the unit's command lacks %q: %s", want, started) } } - for _, unwanted := range []string{"docker", "containerd", "brand-new-dependency", " upgrade;"} { - if strings.Contains(started, unwanted) { - t.Errorf("the unit's command has %q: %s", unwanted, started) + // Docker's names appear only in the guard's pattern, which is what keeps them out + withoutGuard := strings.Replace(started, dockerpkg.ContractPattern(), "", -1) + for _, unwanted := range []string{"docker-ce", "containerd", "brand-new-dependency", " upgrade;"} { + if strings.Contains(withoutGuard, unwanted) { + t.Errorf("the unit's command has %q: %s", unwanted, withoutGuard) } } // the list was simulated as it will be installed, before it was started @@ -197,15 +240,25 @@ func TestSystemPackageUpdateInstallsTheListAndLeavesDockerOut(t *testing.T) { } } +func TestSystemPackageUpdateWithoutDockerKeepsEverythingInTheList(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{upgradeSimulation: simLibc + simContain, installSimulation: simLibc + simContain}).command + + started := startedCommand(t, updater) + if !strings.Contains(started, "'containerd.io'") || !strings.Contains(started, "'libc6'") { + t.Errorf("a box with no Docker engine lost containerd.io from its update: %s", started) + } +} + func TestSystemPackageUpdateWithOnlyDockerLeftIsRefusedAndWritesNothing(t *testing.T) { updater := newTestSystemPackageUpdater(t) - updater.command = (&aptBox{upgradeSimulation: simDockerC + simContain}).command + updater.command = (&aptBox{upgradeSimulation: simDockerC + simContain, dockerCE: dockerCE, policy: dockerRepo}).command var started bool updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } _, err := updater.startUpdate() - if !errors.Is(err, ErrSystemPackageNothingToUpdate) { - t.Fatalf("startUpdate() error = %v, want ErrSystemPackageNothingToUpdate", err) + if !errors.Is(err, ErrSystemPackageNothingToUpdate) || !strings.Contains(err.Error(), "Docker") { + t.Fatalf("startUpdate() error = %v, want ErrSystemPackageNothingToUpdate naming Docker", err) } if started { t.Fatal("a unit was started with nothing to install") @@ -215,6 +268,16 @@ func TestSystemPackageUpdateWithOnlyDockerLeftIsRefusedAndWritesNothing(t *testi } } +func TestSystemPackageUpdateWithNothingLeftDoesNotBlameDocker(t *testing.T) { + updater := newTestSystemPackageUpdater(t) + updater.command = (&aptBox{}).command + + _, err := updater.startUpdate() + if !errors.Is(err, ErrSystemPackageNothingToUpdate) || strings.Contains(err.Error(), "Docker") { + t.Fatalf("startUpdate() error = %v, want a plain 'nothing to update'", err) + } +} + func TestSystemPackageUpdateRefusesAListThatWouldChangeDocker(t *testing.T) { updater := newTestSystemPackageUpdater(t) updater.command = (&aptBox{ @@ -234,24 +297,34 @@ func TestSystemPackageUpdateRefusesAListThatWouldChangeDocker(t *testing.T) { } } -func TestSystemPackageUpdateRefusesAListThatWouldRemoveSomething(t *testing.T) { - updater := newTestSystemPackageUpdater(t) - updater.command = (&aptBox{upgradeSimulation: simLibc, installSimulation: simLibc + "Remv oldthing [1.0]\n"}).command - _, err := updater.startUpdate() - if err == nil || !strings.Contains(err.Error(), "oldthing") { - t.Fatalf("startUpdate() error = %v, want a refusal naming oldthing", err) +func TestSystemPackageUpdateRefusesAListThatIsNotTheCheckedUpgrades(t *testing.T) { + cases := map[string]aptBox{ + "a removal": {installSimulation: simLibc + "Remv oldthing [1.0]\n"}, + "apt refusing to remove": {installFailure: "E: Packages need to be removed but remove is disabled."}, + "a package that is not new": {installSimulation: simLibc + simNew}, + } + for name, c := range cases { + box := c + box.upgradeSimulation = simLibc + updater := newTestSystemPackageUpdater(t) + updater.command = box.command + var started bool + updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + + _, err := updater.startUpdate() + if !errors.Is(err, ErrSystemPackageListChanged) || started { + t.Errorf("%s: startUpdate() error = %v, started = %v", name, err, started) + } } } -func TestSystemPackageUpdateNeverPutsAFunnyNameInAShellLine(t *testing.T) { +func TestSystemPackageUpdateLeavesAFunnyNameOutAndGoesOn(t *testing.T) { updater := newTestSystemPackageUpdater(t) - updater.command = (&aptBox{upgradeSimulation: "Inst foo;rm [1] (2 Ubuntu [amd64])\n"}).command - var started bool - updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + updater.command = (&aptBox{upgradeSimulation: "Inst foo;rm [1] (2 Ubuntu [amd64])\n" + simLibc, installSimulation: simLibc}).command - _, err := updater.startUpdate() - if err == nil || started { - t.Fatalf("startUpdate() error = %v, started = %v: a name with a semicolon got through", err, started) + started := startedCommand(t, updater) + if strings.Contains(started, "foo") || !strings.Contains(started, "'libc6'") { + t.Fatalf("the command = %s: a name with a semicolon must be left out, the others kept", started) } if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", []string{"zlib1g", "x; rm -rf /"}); err == nil { t.Fatal("systemPackageUpdateCommand() accepted a name with a semicolon") @@ -262,17 +335,21 @@ func TestSystemPackageUpdateNeverPutsAFunnyNameInAShellLine(t *testing.T) { } func TestSystemPackageUpdateWaitsForOtherMaintenance(t *testing.T) { - cases := map[string]aptBox{ - "a ReCasaOS update": {activeUnits: map[string]bool{common.UPDATE_UNIT + ".service": true}}, - "an update of Docker": {activeUnits: map[string]bool{systemDockerUpdateUnit: true}}, - "a package manager's lock": {dpkgLocked: true}, + cases := map[string]struct { + box aptBox + locked bool + }{ + "a ReCasaOS update": {aptBox{activeUnits: map[string]bool{common.UPDATE_UNIT + ".service": true}}, false}, + "an update of Docker": {aptBox{activeUnits: map[string]bool{systemDockerUpdateUnit: true}}, false}, + "a package manager's lock": {aptBox{}, true}, } for name, c := range cases { - box := c + box := c.box box.upgradeSimulation = simLibc box.installSimulation = simLibc updater := newTestSystemPackageUpdater(t) updater.command = box.command + updater.dpkgLocked = func() bool { return c.locked } var started bool updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } @@ -283,6 +360,24 @@ func TestSystemPackageUpdateWaitsForOtherMaintenance(t *testing.T) { } } +func TestSystemPackageUpdateLooksAgainRightBeforeItStarts(t *testing.T) { + // a ReCasaOS update starts while the list is being simulated + updater := newTestSystemPackageUpdater(t) + box := &aptBox{upgradeSimulation: simLibc, installSimulation: simLibc, activeUnits: map[string]bool{}} + box.onInstallSimulation = func() { box.activeUnits[common.UPDATE_UNIT+".service"] = true } + updater.command = box.command + var started bool + updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil } + + _, err := updater.startUpdate() + if !errors.Is(err, ErrSystemMaintenanceBusy) || started { + t.Fatalf("startUpdate() error = %v, started = %v: the box changed during the simulations and nothing looked again", err, started) + } + if _, statErr := os.Stat(updater.logPath()); !os.IsNotExist(statErr) { + t.Fatalf("a log was written for an update that was not started: %v", statErr) + } +} + func TestMaintenanceBusy(t *testing.T) { updater := newTestSystemPackageUpdater(t) for _, unit := range systemMaintenanceUnits { @@ -296,19 +391,25 @@ func TestMaintenanceBusy(t *testing.T) { t.Errorf("%s active but excepted: busy", unit) } } - for name, box := range map[string]*aptBox{ - "nothing running": {}, - "flock not installed": {flockMissing: true}, - "lock held": {dpkgLocked: true}, - "another unit stopped": {activeUnits: map[string]bool{}}, - } { - updater.command = box.command - _, busy := updater.maintenanceBusy(context.Background(), "") - if want := box.dpkgLocked; busy != want { - t.Errorf("%s: busy = %v, want %v", name, busy, want) - } + + updater.command = (&aptBox{}).command + if _, busy := updater.maintenanceBusy(context.Background(), ""); busy { + t.Error("an idle box is busy") } + + // dpkg's lock, only on a host that has dpkg + updater.dpkgLocked = func() bool { return true } + if reason, busy := updater.maintenanceBusy(context.Background(), ""); !busy || !strings.Contains(reason, "dpkg") { + t.Errorf("a held lock: busy = %v, reason = %q", busy, reason) + } + updater.readOSRelease = func() (map[string]string, error) { return map[string]string{"ID": "arch"}, nil } + if _, busy := updater.maintenanceBusy(context.Background(), ""); busy { + t.Error("a host without dpkg was refused for a lock it cannot have: the ReCasaOS update would never start there") + } + // systemd that does not answer is not a reason to refuse + updater.readOSRelease = func() (map[string]string, error) { return map[string]string{"ID": "debian"}, nil } + updater.dpkgLocked = nil updater.command = func(_ context.Context, name string, _ ...string) ([]byte, error) { if name == "systemctl" { return nil, errors.New("systemctl: not found") @@ -337,3 +438,54 @@ func TestAReCasaOSUpdateWaitsForAPackageUpdate(t *testing.T) { t.Fatal("MaintenanceBusy() = true on an idle box") } } + +// The unit's own shell line is run, with a fake apt-get, to see what it does and does not do. +func runUnitCommand(t *testing.T, plan string) (log string, installed bool) { + t.Helper() + if runtime.GOOS != "linux" { + t.Skip("the unit's command is for a Linux shell") + } + dir := t.TempDir() + plans := filepath.Join(dir, "plan.txt") + real := filepath.Join(dir, "real-install.txt") + if err := os.WriteFile(plans, []byte(plan), 0o600); err != nil { + t.Fatal(err) + } + apt := filepath.Join(dir, "apt-get") + script := "#!/bin/sh\ncase \" $* \" in\n*\" -s \"*) cat \"" + plans + "\" ;;\n*) echo \"$@\" >> \"" + real + "\" ;;\nesac\n" + if err := os.WriteFile(apt, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + logPath := filepath.Join(dir, "package-update.log") + command, err := systemPackageUpdateCommand(apt, logPath, []string{"libc6", "zlib1g"}) + if err != nil { + t.Fatal(err) + } + if err := exec.Command("/bin/bash", "-o", "pipefail", "-c", command).Run(); err != nil { + t.Logf("the unit's command ended with %v", err) + } + data, _ := os.ReadFile(logPath) + _, statErr := os.Stat(real) + return string(data), statErr == nil +} + +func TestTheUnitInstallsWhenTheListIsStillClean(t *testing.T) { + log, installed := runUnitCommand(t, simLibc+simZlib) + if !installed || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_SUCCESS") || strings.Contains(log, "GUARD") { + t.Fatalf("installed = %v, log = %q", installed, log) + } +} + +func TestTheUnitInstallsNothingWhenTheListNowTouchesDockerOrRemoves(t *testing.T) { + for name, plan := range map[string]string{ + "docker-ce": simLibc + simDockerC, + "containerd": simLibc + simContain, + "a removal": simLibc + "Remv oldthing [1.0]\n", + "docker-ce:a": "Inst containerd.io:armhf [2.3.5] (2.3.6 Docker CE [armhf])\n", + } { + log, installed := runUnitCommand(t, plan) + if installed || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_GUARD") || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_FAILED") || strings.Contains(log, "SUCCESS") { + t.Errorf("%s: installed = %v, log = %q", name, installed, log) + } + } +} From 10f1124c0865e473d3725205cff91e5102c5376b Mon Sep 17 00:00:00 2001 From: Gary Date: Thu, 8 Oct 2026 20:17:07 +0200 Subject: [PATCH 3/4] fix(packages): the Docker guard only where there is a Docker engine to protect A box with no Docker engine (a Kubernetes node's containerd, say) keeps containerd.io in its update, and the unit's own guard stops only on a removal there. --- pkg/dockerpkg/dockerpkg.go | 12 ++++++++---- pkg/dockerpkg/dockerpkg_test.go | 11 ++++++++--- service/system_package.go | 12 ++++++------ service/system_package_docker.go | 23 +++++++++++------------ service/system_package_docker_test.go | 27 ++++++++++++++++++++------- service/system_package_test.go | 4 ++-- 6 files changed, 55 insertions(+), 34 deletions(-) diff --git a/pkg/dockerpkg/dockerpkg.go b/pkg/dockerpkg/dockerpkg.go index ad90093..65b7c7f 100644 --- a/pkg/dockerpkg/dockerpkg.go +++ b/pkg/dockerpkg/dockerpkg.go @@ -189,10 +189,14 @@ func Touches(simulation string) Touched { } // ContractPattern is an extended regular expression (for grep -E) that matches a line of -// an apt simulation that breaks the contract of the update: an install or upgrade of a -// package of the Docker family, or any removal. The unit's shell line runs the simulation -// again just before it installs, and installs nothing if a line matches. -func ContractPattern() string { +// an apt simulation that breaks the contract of the update: any removal, and, when the box +// has a Docker engine to protect, an install or upgrade of a package of the Docker family. +// The unit's shell line runs the simulation again just before it installs, and installs +// nothing if a line matches. +func ContractPattern(protectDocker bool) string { + if !protectDocker { + return `^Remv ` + } names := make([]string, 0, len(family)) for name := range family { names = append(names, strings.ReplaceAll(name, ".", `\.`)) diff --git a/pkg/dockerpkg/dockerpkg_test.go b/pkg/dockerpkg/dockerpkg_test.go index 9587bd2..b93ee6d 100644 --- a/pkg/dockerpkg/dockerpkg_test.go +++ b/pkg/dockerpkg/dockerpkg_test.go @@ -150,7 +150,7 @@ Inst docker-ce-extra-thing [1] (2 x [amd64]) } func TestContractPatternMatchesWhatTouchesFinds(t *testing.T) { - pattern := regexp.MustCompile(ContractPattern()) + pattern := regexp.MustCompile(ContractPattern(true)) for _, line := range []string{ "Inst docker-ce [5:29.8.1] (5:29.8.2 Docker CE:jammy [amd64])", "Inst containerd.io [2.3.5] (2.3.6 Docker CE [amd64])", @@ -175,7 +175,12 @@ func TestContractPatternMatchesWhatTouchesFinds(t *testing.T) { } } // no character of it needs more than single quotes in a shell - if strings.Contains(ContractPattern(), "'") { - t.Errorf("the pattern holds a single quote: %q", ContractPattern()) + if strings.Contains(ContractPattern(true), "'") || strings.Contains(ContractPattern(false), "'") { + t.Errorf("a pattern holds a single quote: %q", ContractPattern(true)) + } + // on a box with no Docker engine only removals break the contract + plain := regexp.MustCompile(ContractPattern(false)) + if plain.MatchString("Inst containerd.io [1] (2 x [amd64])") || !plain.MatchString("Remv anything [1.0]") { + t.Errorf("the pattern without Docker to protect = %q", ContractPattern(false)) } } diff --git a/service/system_package.go b/service/system_package.go index d280b06..ab3e211 100644 --- a/service/system_package.go +++ b/service/system_package.go @@ -250,7 +250,7 @@ func (u *systemPackageUpdater) startUpdate() (SystemPackageUpdateStatus, error) // status and the check can still answer, and the box is looked at again afterwards. u.mu.Unlock() listCtx, cancelList := context.WithTimeout(context.Background(), systemPackageCheckTimeout) - names, err := u.upgradeNames(listCtx, support) + names, protectDocker, err := u.upgradeNames(listCtx, support) cancelList() u.mu.Lock() if err != nil { @@ -267,7 +267,7 @@ func (u *systemPackageUpdater) startUpdate() (SystemPackageUpdateStatus, error) } logPath := u.logPath() - args, err := systemPackageUpdateArgs(support.aptPath, logPath, names) + args, err := systemPackageUpdateArgs(support.aptPath, logPath, names, protectDocker) if err != nil { status.Error = err.Error() return status, err @@ -415,8 +415,8 @@ func parseAPTUpgradeSimulation(output string) []SystemPackageUpdate { return result } -func systemPackageUpdateArgs(aptPath, logPath string, names []string) ([]string, error) { - command, err := systemPackageUpdateCommand(aptPath, logPath, names) +func systemPackageUpdateArgs(aptPath, logPath string, names []string, protectDocker bool) ([]string, error) { + command, err := systemPackageUpdateCommand(aptPath, logPath, names, protectDocker) if err != nil { return nil, err } @@ -438,7 +438,7 @@ func systemPackageUpdateArgs(aptPath, logPath string, names []string) ([]string, // systemPackageUpdateCommand installs exactly the packages in names, as upgrades: a name // that is not a package name is refused, never put into the shell line. -func systemPackageUpdateCommand(aptPath, logPath string, names []string) (string, error) { +func systemPackageUpdateCommand(aptPath, logPath string, names []string, protectDocker bool) (string, error) { if len(names) == 0 { return "", ErrSystemPackageNothingToUpdate } @@ -457,7 +457,7 @@ func systemPackageUpdateCommand(aptPath, logPath string, names []string) (string // the same simulation is made again just before the install, and nothing is installed if // it now touches Docker's packages or removes one. guarded := "plan=\"$(" + quotedAPTPath + " -s --no-remove -o Debug::NoLocking=true -o Dpkg::Use-Pty=0 " + installArgs + " 2>&1)\"; " + - "if printf '%s\\n' \"$plan\" | grep -Eq " + shellQuote(dockerpkg.ContractPattern()) + "; then " + + "if printf '%s\\n' \"$plan\" | grep -Eq " + shellQuote(dockerpkg.ContractPattern(protectDocker)) + "; then " + "printf 'CASAOS_PACKAGE_UPDATE_GUARD the list now changes Docker or removes a package: nothing was installed\\n'; status=1; else " + quotedAPTPath + " -y --no-remove -o Dpkg::Use-Pty=0 -o Dpkg::Options::=--force-confold -o DPkg::Lock::Timeout=120 " + installArgs + "; status=$?; fi; " return "set -o pipefail; exec >> " + quotedLogPath + " 2>&1; printf 'CASAOS_PACKAGE_UPDATE_STARTED %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; " + guarded + "if [ -f /var/run/reboot-required ]; then printf 'CASAOS_PACKAGE_UPDATE_REBOOT_REQUIRED\\n'; fi; if [ \"$status\" -eq 0 ]; then printf 'CASAOS_PACKAGE_UPDATE_SUCCESS %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\"; else printf 'CASAOS_PACKAGE_UPDATE_FAILED %s %s\\n' \"$(/bin/date -u +%Y-%m-%dT%H:%M:%SZ)\" \"$status\"; fi; exit \"$status\"", nil diff --git a/service/system_package_docker.go b/service/system_package_docker.go index fcc9e4e..f436e4d 100644 --- a/service/system_package_docker.go +++ b/service/system_package_docker.go @@ -179,13 +179,12 @@ var aptRemovalRefused = regexp.MustCompile(`(?i)remove is disabled|packages need // second simulation of installing exactly that list must change nothing of Docker's, // remove nothing and install nothing that is not an upgrade. The unit's own shell line // runs the same simulation again right before it installs. -func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemPackageSupport) ([]string, error) { +func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemPackageSupport) (names []string, protectDocker bool, err error) { output, err := u.command(ctx, support.aptPath, systemPackageSimulateArgs("upgrade")...) if err != nil { - return nil, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) + return nil, false, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) } engine := u.engineInstalled(ctx) - var names []string var keptDocker int for _, update := range parseAPTUpgradeSimulation(string(output)) { switch { @@ -201,25 +200,25 @@ func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemP } if len(names) == 0 { if keptDocker > 0 { - return nil, fmt.Errorf("%w: what is left is Docker's, which is updated on its own", ErrSystemPackageNothingToUpdate) + return nil, engine, fmt.Errorf("%w: what is left is Docker's, which is updated on its own", ErrSystemPackageNothingToUpdate) } - return nil, ErrSystemPackageNothingToUpdate + return nil, engine, ErrSystemPackageNothingToUpdate } args := systemPackageSimulateArgs(append([]string{"install", "--only-upgrade", "--no-install-recommends"}, names...)...) output, err = u.command(ctx, support.aptPath, args...) if err != nil { if aptRemovalRefused.Match(output) { - return nil, fmt.Errorf("%w: it would have to remove packages, which the System packages update never does", ErrSystemPackageListChanged) + return nil, engine, fmt.Errorf("%w: it would have to remove packages, which the System packages update never does", ErrSystemPackageListChanged) } - return nil, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) + return nil, engine, fmt.Errorf("apt package update check failed: %s", trimSystemPackageOutput(output)) } touched := dockerpkg.Touches(string(output)) - if len(touched.Docker) > 0 { - return nil, fmt.Errorf("%w: %s", ErrSystemPackageTouchesDocker, strings.Join(touched.Docker, ", ")) + if engine && len(touched.Docker) > 0 { + return nil, engine, fmt.Errorf("%w: %s", ErrSystemPackageTouchesDocker, strings.Join(touched.Docker, ", ")) } if len(touched.Removed) > 0 { - return nil, fmt.Errorf("%w: it would remove %s, which the System packages update never does", ErrSystemPackageListChanged, strings.Join(touched.Removed, ", ")) + return nil, engine, fmt.Errorf("%w: it would remove %s, which the System packages update never does", ErrSystemPackageListChanged, strings.Join(touched.Removed, ", ")) } var added []string for _, update := range parseAPTUpgradeSimulation(string(output)) { @@ -228,7 +227,7 @@ func (u *systemPackageUpdater) upgradeNames(ctx context.Context, support systemP } } if len(added) > 0 { - return nil, fmt.Errorf("%w: it would install %s, which are not upgrades", ErrSystemPackageListChanged, strings.Join(added, ", ")) + return nil, engine, fmt.Errorf("%w: it would install %s, which are not upgrades", ErrSystemPackageListChanged, strings.Join(added, ", ")) } - return names, nil + return names, engine, nil } diff --git a/service/system_package_docker_test.go b/service/system_package_docker_test.go index 06f6c32..5887667 100644 --- a/service/system_package_docker_test.go +++ b/service/system_package_docker_test.go @@ -222,7 +222,7 @@ func TestSystemPackageUpdateInstallsTheListAndLeavesDockerOut(t *testing.T) { } } // Docker's names appear only in the guard's pattern, which is what keeps them out - withoutGuard := strings.Replace(started, dockerpkg.ContractPattern(), "", -1) + withoutGuard := strings.Replace(started, dockerpkg.ContractPattern(true), "", -1) for _, unwanted := range []string{"docker-ce", "containerd", "brand-new-dependency", " upgrade;"} { if strings.Contains(withoutGuard, unwanted) { t.Errorf("the unit's command has %q: %s", unwanted, withoutGuard) @@ -326,10 +326,10 @@ func TestSystemPackageUpdateLeavesAFunnyNameOutAndGoesOn(t *testing.T) { if strings.Contains(started, "foo") || !strings.Contains(started, "'libc6'") { t.Fatalf("the command = %s: a name with a semicolon must be left out, the others kept", started) } - if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", []string{"zlib1g", "x; rm -rf /"}); err == nil { + if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", []string{"zlib1g", "x; rm -rf /"}, true); err == nil { t.Fatal("systemPackageUpdateCommand() accepted a name with a semicolon") } - if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", nil); !errors.Is(err, ErrSystemPackageNothingToUpdate) { + if _, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/x.log", nil, true); !errors.Is(err, ErrSystemPackageNothingToUpdate) { t.Fatalf("systemPackageUpdateCommand(nil) error = %v", err) } } @@ -440,7 +440,7 @@ func TestAReCasaOSUpdateWaitsForAPackageUpdate(t *testing.T) { } // The unit's own shell line is run, with a fake apt-get, to see what it does and does not do. -func runUnitCommand(t *testing.T, plan string) (log string, installed bool) { +func runUnitCommand(t *testing.T, plan string, protectDocker bool) (log string, installed bool) { t.Helper() if runtime.GOOS != "linux" { t.Skip("the unit's command is for a Linux shell") @@ -457,7 +457,7 @@ func runUnitCommand(t *testing.T, plan string) (log string, installed bool) { t.Fatal(err) } logPath := filepath.Join(dir, "package-update.log") - command, err := systemPackageUpdateCommand(apt, logPath, []string{"libc6", "zlib1g"}) + command, err := systemPackageUpdateCommand(apt, logPath, []string{"libc6", "zlib1g"}, protectDocker) if err != nil { t.Fatal(err) } @@ -470,7 +470,7 @@ func runUnitCommand(t *testing.T, plan string) (log string, installed bool) { } func TestTheUnitInstallsWhenTheListIsStillClean(t *testing.T) { - log, installed := runUnitCommand(t, simLibc+simZlib) + log, installed := runUnitCommand(t, simLibc+simZlib, true) if !installed || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_SUCCESS") || strings.Contains(log, "GUARD") { t.Fatalf("installed = %v, log = %q", installed, log) } @@ -483,9 +483,22 @@ func TestTheUnitInstallsNothingWhenTheListNowTouchesDockerOrRemoves(t *testing.T "a removal": simLibc + "Remv oldthing [1.0]\n", "docker-ce:a": "Inst containerd.io:armhf [2.3.5] (2.3.6 Docker CE [armhf])\n", } { - log, installed := runUnitCommand(t, plan) + log, installed := runUnitCommand(t, plan, true) if installed || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_GUARD") || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_FAILED") || strings.Contains(log, "SUCCESS") { t.Errorf("%s: installed = %v, log = %q", name, installed, log) } } } + +func TestTheUnitOfABoxWithoutDockerStillInstallsContainerd(t *testing.T) { + // the guard protects a Docker engine; on a box that has none, containerd.io is an ordinary package + log, installed := runUnitCommand(t, simLibc+simContain, false) + if !installed || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_SUCCESS") || strings.Contains(log, "GUARD") { + t.Fatalf("installed = %v, log = %q", installed, log) + } + // ... but a removal still stops it + log, installed = runUnitCommand(t, simLibc+"Remv oldthing [1.0]\n", false) + if installed || !strings.Contains(log, "CASAOS_PACKAGE_UPDATE_GUARD") { + t.Fatalf("installed = %v, log = %q", installed, log) + } +} diff --git a/service/system_package_test.go b/service/system_package_test.go index 4443ed3..f131f3d 100644 --- a/service/system_package_test.go +++ b/service/system_package_test.go @@ -60,7 +60,7 @@ func TestParseAPTUpgradeSimulationNoUpdates(t *testing.T) { } func TestSystemPackageUpdateCommand(t *testing.T) { - command, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/casaos/package update.log", []string{"libc6", "zlib1g"}) + command, err := systemPackageUpdateCommand("/usr/bin/apt-get", "/var/log/casaos/package update.log", []string{"libc6", "zlib1g"}, true) if err != nil { t.Fatalf("systemPackageUpdateCommand() error = %v", err) } @@ -81,7 +81,7 @@ func TestSystemPackageUpdateCommand(t *testing.T) { } func TestSystemPackageUpdateArgsAreDetached(t *testing.T) { - args, err := systemPackageUpdateArgs("/usr/bin/apt-get", "/var/log/casaos/package-update.log", []string{"libc6"}) + args, err := systemPackageUpdateArgs("/usr/bin/apt-get", "/var/log/casaos/package-update.log", []string{"libc6"}, true) if err != nil { t.Fatalf("systemPackageUpdateArgs() error = %v", err) } From 9a67c12528a932e48e18b4c1b6c29f91a68644a4 Mon Sep 17 00:00:00 2001 From: Gary Date: Thu, 8 Oct 2026 20:19:41 +0200 Subject: [PATCH 4/4] test: the Docker refusal needs a Docker engine on the box --- service/system_package_docker_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/service/system_package_docker_test.go b/service/system_package_docker_test.go index 5887667..5531a3a 100644 --- a/service/system_package_docker_test.go +++ b/service/system_package_docker_test.go @@ -284,6 +284,7 @@ func TestSystemPackageUpdateRefusesAListThatWouldChangeDocker(t *testing.T) { upgradeSimulation: simLibc, // something in the list depends on a newer docker-ce installSimulation: simLibc + simDockerC, + dockerCE: dockerCE, }).command var started bool updater.start = func(string, string, ...string) ([]byte, error) { started = true; return nil, nil }