build, upgrade: Disable upgrades on unsupported OS version (fixes #9290) #9656

calmh · 2024-08-20T09:36:31Z

Simplification of #9635. Mechanism in short:

We maintain (in the source repo) a compat.yaml which holds the Go runtime requirements for the Go versions we currently support building. (Yaml, because it supports comments and is a bit friendlier to use than raw json.)
When building for distribution (tar/zip) we also build a condensed version of this in compat.json, holding the requirements for the actual Go version used for the build. (Lacking data in compat.yaml at this point is a build failure.) (JSON because this is just for machine consumption.)
We upload this compat.json together with all other release artifacts.
The upgrade metadata server is smart enough to load this compat.json and include it in the response for clients. This is really just an optimisation -- the code for loading a release is shared between the client and the upgrade server, so if the upgrade server didn't do this the client would look for the compat.json separately.
The upgrade code compares the requirements in compat.json to the current operating system / kernel version, and based on this accepts or rejects a release.

The end result should be that machines that do not support upgrading to a given release will ignore it and get stuck on the last supported release.

1. The build process creates a compatability.json for the runtime used. 2. The release process adds compatability.json as a release artifact 3. The upgrade server reads compatability.yaml and merges it into its /meta.json response. 4. The upgrading client reads the compatability.json file in the release bundle, and using the go runtime setting found in it, looks up the minimum OS versions listed in the upgrade server's /meta.json response. 5. If the upgrading client's OS version is lower than the minimum OS version listed, the upgrade is aborted.

…-smaller * upstream/main: build: use Go 1.23, require minimum 1.22 (syncthing#9651) gui, man, authors: Update docs, translations, and contributors

* main: lib/protocol: Further interface refactor (syncthing#9396) gui: Replace global "Panel padding decrease" style with targeted class (syncthing#9659)

AudriusButkevicius · 2024-08-25T10:51:13Z

lib/upgrade/upgrade_compat.go

+		if CompareVersions(minKernel, currentKernel) > Equal {
+			return fmt.Errorf("runtime %v requires OS version %v but this system is version %v", runtimeReqs.Runtime, minKernel, currentKernel)
+		}
+	}


Do we think yolo upgrading because we can't find the goos/arch combo is right?

I think we have to, otherwise we need to cover all possible OSes. And we currently don't and I doubt we even could if we wanted to, there's always going to be something esoteric we'll miss.

AudriusButkevicius · 2024-08-25T10:54:43Z

Couldn't we flip this on it's head? Post goos goarch kernel version when looking for upgrades to the upgrade server, and handle it there? Return nothing if its the last version supported.

Feels like scooping up that mess would be easier than asking everyone to upgrade because we broke autoupgrade.

Also, when we find something critical, we could retro-fix it.

imsodin · 2024-08-25T11:36:46Z

lib/upgrade/upgrade_supported.go

+			if asset.Name == "compat.json" {
+				resp, err := insecureGet(asset.BrowserURL, current)
+				if err != nil {
+					l.Infoln("Fetching compat.json:", err)


Maybe worth knowing which release failed (same for the error below):

Suggested change

l.Infoln("Fetching compat.json:", err)

l.Infof("Fetching compat.json for %v: %v", rel.Tag, err)

imsodin · 2024-08-25T11:53:07Z

lib/upgrade/upgrade_compat.go

+		if CompareVersions(minKernel, currentKernel) > Equal {
+			return fmt.Errorf("runtime %v requires OS version %v but this system is version %v", runtimeReqs.Runtime, minKernel, currentKernel)
+		}
+	}


I think we have to, otherwise we need to cover all possible OSes. And we currently don't and I doubt we even could if we wanted to, there's always going to be something esoteric we'll miss.

imsodin

I feel like we shouldn't silently stop upgrading, but log a warning (and thus generate a banner in the UI) if we detect a newer version that isn't compatible anymore. The user should be aware that they are out of date. If just for our own sake when getting bug reports.

imsodin · 2024-08-25T11:57:12Z

lib/upgrade/upgrade_compat.go

+		if found && arch != runtime.GOARCH {
+			continue
+		}


What's the intent of the arch check? Asking because currently I don't see arch in any arch requirements/input anywhere else. Is it for future extensibility if go's OS version requirements start to differ between arches? I'd tend towards adding that only once we use it. And if we already want to have it now, it should also feature in the error message below besides the version.

@imsodin I added that per @calmh's example, but I agree, let's implement it if/when it's needed.

imsodin · 2024-08-25T12:11:15Z

lib/upgrade/upgrade_compat.go

+// normalizeKernelVersion strips off anything after the kernel version's patch
+// level. So Windows' "10.0.22631.3880 Build 22631.3880" becomes "10.0.22631",
+// and Linux's "6.8.0-39-generic" becomes "6.8.0".
+func normalizeKernelVersion(version string) string {
+	version, _, _ = strings.Cut(version, " ")
+	version, _, _ = strings.Cut(version, "-")
+
+	parts := strings.Split(version, ".")
+	maxParts := min(len(parts), 3)
+	return strings.Join(parts[:maxParts], ".")
+}


I feel a bit squeamish about this generic way of parsing a version string we don't control. I don't see why the format would change, but if it did it could break auto-upgrading. E.g. if the host version got a string prefix (other than "v"), our comparison would try to parse that as an int, ignore the error and use 0. Which is then guaranteed lower than min requirement and auto-upgrade is broken. I'd prefer to auto-upgrade when in doubt.
One option to do that (and it feels wrong to suggest that as "more safe" :P ), would be to have a quite strict regex per OS for the expected version format. If the regex doesn't match, we upgrade (and log a failure so we know about it happening). I first wanted to have a unit test to find this, but test runners likely upgrade much later than user machines.

rasa · 2024-08-26T19:12:00Z

lib/upgrade/upgrade_common.go

@@ -38,6 +39,13 @@ type Asset struct {
 	BrowserURL string `json:"browser_download_url,omitempty"`
 }

+// RuntimeReqs defines the structure of compat.json, which is included
+// with each elease.


Suggested change

// with each elease.

// with each release.

This is to add the generation of `compat.json` as a release artifact. It describes the runtime requirements of the release in question. The next step is to have the upgrade server use this information to filter releases provided to clients. This is per the discussion in #9656 --------- Co-authored-by: Ross Smith II <ross@smithii.com>

rasa · 2024-09-29T18:54:47Z

Why was this closed? I closed #9635 in favor of this PR. Should we close #9290 as well?

calmh · 2024-09-29T20:22:32Z

We will, shortly. This is currently implemented in the upgrade server on the infrastructure branch.

rasa and others added 30 commits August 7, 2024 06:03

Disable upgrades on unsupported kernel (fixes syncthing#9290)

b88e44a

Compare compatible.json with host, too

6a9bbcc

Add more error checking

f0bc6ac

Rename kernelVersion to OSVersion

b9a5676

Remove dependency on go1.22's go/version

86c2830

Fix TestCompatibilityJson on CI

e5748d1

Generate compatibility.json from compatibility.yaml

e2ef25f

Fix lint error

9aecd0a

Fix Debian build (-no-upgrade)

2dad3a3

Fix deepsource errors

911255b

Merge branch 'main' into feature/9290-disable-upgrades

73927fa

Fix darwin version in compatibility.yaml

b3185b4

Fix Debian build

aac034c

Revert uneeded change

5335f5a

Lower case first char of error messages

700d550

Simplify code per @calm's comments.

ef1b243

go mod tidy

e16949a

Fix typo

89ac5d1

Fix debian build

e7f0f93

Fix typos

d0e0729

Merge branch 'main' into feature/9290-disable-upgrades

d4d86b1

Fix typo in build.go

a574593

wip

f9abf79

Merge remote-tracking branch 'upstream/main' into jb/disable-upgrades…

68665a2

…-smaller * upstream/main: build: use Go 1.23, require minimum 1.22 (syncthing#9651) gui, man, authors: Update docs, translations, and contributors

wip

51c206a

wip

b41abcf

wip

7649203

wip

38495ab

wip

172a439

wip

09ee8ea

calmh mentioned this pull request Aug 20, 2024

Disable upgrades on unsupported kernel (fixes #9290) #9635

Closed

calmh requested review from imsodin and AudriusButkevicius August 24, 2024 11:01

calmh added 3 commits August 25, 2024 12:12

Merge branch 'main' into jb/disable-upgrades-smaller

b0a5a0b

* main: lib/protocol: Further interface refactor (syncthing#9396) gui: Replace global "Panel padding decrease" style with targeted class (syncthing#9659)

wip

d5db74c

wip

16bc9ef

AudriusButkevicius reviewed Aug 25, 2024

View reviewed changes

imsodin reviewed Aug 25, 2024

View reviewed changes

rasa reviewed Aug 26, 2024

View reviewed changes

calmh mentioned this pull request Sep 11, 2024

build: add generating compat.json #9700

Merged

calmh closed this Sep 25, 2024

calmh added the build Issues caused by or requiring changes to the build system (scripts or Docker image) label May 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

build, upgrade: Disable upgrades on unsupported OS version (fixes #9290) #9656

build, upgrade: Disable upgrades on unsupported OS version (fixes #9290) #9656

Uh oh!

calmh commented Aug 20, 2024 •

edited

Loading

Uh oh!

AudriusButkevicius Aug 25, 2024

Uh oh!

imsodin Aug 25, 2024

Uh oh!

AudriusButkevicius commented Aug 25, 2024 •

edited

Loading

Uh oh!

imsodin Aug 25, 2024

Uh oh!

imsodin Aug 25, 2024

Uh oh!

imsodin left a comment

Uh oh!

imsodin Aug 25, 2024

Uh oh!

rasa Aug 25, 2024

Uh oh!

imsodin Aug 25, 2024

Uh oh!

rasa Aug 26, 2024

Uh oh!

rasa commented Sep 29, 2024

Uh oh!

calmh commented Sep 29, 2024

Uh oh!

Uh oh!

	l.Infoln("Fetching compat.json:", err)
	l.Infof("Fetching compat.json for %v: %v", rel.Tag, err)

Uh oh!

build, upgrade: Disable upgrades on unsupported OS version (fixes #9290) #9656

build, upgrade: Disable upgrades on unsupported OS version (fixes #9290) #9656

Uh oh!

Conversation

calmh commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AudriusButkevicius commented Aug 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imsodin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rasa commented Sep 29, 2024

Uh oh!

calmh commented Sep 29, 2024

Uh oh!

Uh oh!

calmh commented Aug 20, 2024 •

edited

Loading

AudriusButkevicius commented Aug 25, 2024 •

edited

Loading