[pve-devel] [PATCH-SERIES proxmox-apt/widget-toolkit] prepare for major upgrade

[pve-devel] [PATCH-SERIES proxmox-apt/widget-toolkit] prepare for major upgrade

[Fiona Ebner]

There's a confusing issue when the suites are already updated,
because the UI will not detect any Proxmox VE repository anymore then.

This is fixed by the proxmox-apt patch intended for the stable branch.

The UI patches make sure the ignore-pre-upgrade-warning is actually
ignored and add a check for mixed repositories when upgrade is allowed.

The final UI patch is intended for the stable branch only and makes
sure that the warning handling expects a major upgrade to be allowed.


proxmox-apt:

Fiona Ebner (1):
  repositories: also detect repository with next suite as configured

 src/repositories/mod.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


proxmox-widget-toolkit:

Fiona Ebner (5):
  apt repositories: actually ignore ignore-pre-upgrade-warning
  apt repositories: just ignore unknown info rather than throwing an
    error
  apt repositories: add classifyOrigin helper
  apt repositories: detect mixed suites before major upgrade
  apt repositoires: allow major upgrade

 src/node/APTRepositories.js | 63 ++++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 11 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH stable proxmox-apt 1/1] repositories: also detect repository with next suite as configured

[Fiona Ebner]

This avoids that no standard Proxmox repository can be detected during
upgrade anymore. There is a 'ignore-pre-upgrade-warning' about the
suite already, that the frontend can display when upgrading is not
allowed yet.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

It's not ideal, for the time between when the update for proxmox-apt
is rolled out and when the Bookworm repositories are actually made
available. And it would need to be done for each major upgrade.

For next time, I'll try to come up with something better. Probably,
the callers should let the backend here know when upgrade is allowed
and then the backend can act accordingly.

 src/repositories/mod.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index d8848b8..6588b90 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -106,7 +106,12 @@ pub fn standard_repositories(
                     continue;
                 }
 
-                if repo.is_referenced_repository(entry.handle, product, &suite.to_string()) {
+                if repo.is_referenced_repository(entry.handle, product, &suite.to_string())
+                    || repo.is_referenced_repository(
+                        entry.handle,
+                        product,
+                        &suite.next().unwrap().to_string(),
+                ) {
                     entry.status = Some(repo.enabled);
                 }
             }
-- 
2.30.2

Re: [pve-devel] [PATCH stable proxmox-apt 1/1] repositories: also detect repository with next suite as configured

[Wolfgang Bumiller]

FYI, wrong git repo, should be done in proxmox.git

Is this purely for stable-only?

On Mon, Jun 05, 2023 at 05:43:08PM +0200, Fiona Ebner wrote:
> This avoids that no standard Proxmox repository can be detected during
> upgrade anymore. There is a 'ignore-pre-upgrade-warning' about the
> suite already, that the frontend can display when upgrading is not
> allowed yet.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> It's not ideal, for the time between when the update for proxmox-apt
> is rolled out and when the Bookworm repositories are actually made
> available. And it would need to be done for each major upgrade.
> 
> For next time, I'll try to come up with something better. Probably,
> the callers should let the backend here know when upgrade is allowed
> and then the backend can act accordingly.
> 
>  src/repositories/mod.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
> index d8848b8..6588b90 100644
> --- a/src/repositories/mod.rs
> +++ b/src/repositories/mod.rs
> @@ -106,7 +106,12 @@ pub fn standard_repositories(
>                      continue;
>                  }
>  
> -                if repo.is_referenced_repository(entry.handle, product, &suite.to_string()) {
> +                if repo.is_referenced_repository(entry.handle, product, &suite.to_string())
> +                    || repo.is_referenced_repository(
> +                        entry.handle,
> +                        product,
> +                        &suite.next().unwrap().to_string(),
> +                ) {
>                      entry.status = Some(repo.enabled);
>                  }
>              }
> -- 
> 2.30.2

Re: [pve-devel] [PATCH stable proxmox-apt 1/1] repositories: also detect repository with next suite as configured

[Fiona Ebner]

Am 09.06.23 um 10:45 schrieb Wolfgang Bumiller:
> FYI, wrong git repo, should be done in proxmox.git
> 
> Is this purely for stable-only?

Yes, sorry if I didn't make that clear. In the Bookworm/master branch,
we still want to warn if somebody put the next suite. See also the quick
comment about "for next time" below.

> 
> On Mon, Jun 05, 2023 at 05:43:08PM +0200, Fiona Ebner wrote:
>> This avoids that no standard Proxmox repository can be detected during
>> upgrade anymore. There is a 'ignore-pre-upgrade-warning' about the
>> suite already, that the frontend can display when upgrading is not
>> allowed yet.
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> It's not ideal, for the time between when the update for proxmox-apt
>> is rolled out and when the Bookworm repositories are actually made
>> available. And it would need to be done for each major upgrade.
>>
>> For next time, I'll try to come up with something better. Probably,
>> the callers should let the backend here know when upgrade is allowed
>> and then the backend can act accordingly.
>>

[pve-devel] applied: [PATCH stable proxmox-apt 1/1] repositories: also detect repository with next suite as configured

[Wolfgang Bumiller]

applied & bumped proxmox-apt/pve-rs/pmg-rs

[pve-devel] [PATCH widget-toolkit 1/5] apt repositories: actually ignore ignore-pre-upgrade-warning

[Fiona Ebner]

when upgrading is possible rather than throwing an error by reaching
the else branch.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/node/APTRepositories.js | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 1fb627c..cb08bb6 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -654,10 +654,12 @@ Ext.define('Proxmox.node.APTRepositories', {
 
 		    if (info.kind === 'origin') {
 			infos[path][idx].origin = info.message;
-		    } else if (info.kind === 'warning' ||
-			(info.kind === 'ignore-pre-upgrade-warning' && !repoGrid.majorUpgradeAllowed)
-		    ) {
+		    } else if (info.kind === 'warning') {
 			infos[path][idx].warnings.push(info);
+		    } else if (info.kind === 'ignore-pre-upgrade-warning') {
+			if (!repoGrid.majorUpgradeAllowed) {
+			    infos[path][idx].warnings.push(info);
+			}
 		    } else {
 			throw 'unknown info';
 		    }
-- 
2.39.2

[pve-devel] [PATCH/RFC widget-toolkit 2/5] apt repositories: just ignore unknown info rather than throwing an error

[Fiona Ebner]

This will avoid breaking older UI when extending the backend.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/node/APTRepositories.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index cb08bb6..ed58e5b 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -660,8 +660,6 @@ Ext.define('Proxmox.node.APTRepositories', {
 			if (!repoGrid.majorUpgradeAllowed) {
 			    infos[path][idx].warnings.push(info);
 			}
-		    } else {
-			throw 'unknown info';
 		    }
 		}
 
-- 
2.39.2

Re: [pve-devel] [PATCH/RFC widget-toolkit 2/5] apt repositories: just ignore unknown info rather than throwing an error

[Thomas Lamprecht]

Am 05/06/2023 um 17:43 schrieb Fiona Ebner:
> This will avoid breaking older UI when extending the backend.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/node/APTRepositories.js | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
> index cb08bb6..ed58e5b 100644
> --- a/src/node/APTRepositories.js
> +++ b/src/node/APTRepositories.js
> @@ -660,8 +660,6 @@ Ext.define('Proxmox.node.APTRepositories', {
>  			if (!repoGrid.majorUpgradeAllowed) {
>  			    infos[path][idx].warnings.push(info);
>  			}
> -		    } else {
> -			throw 'unknown info';

otherwise we could use some Ext.Msg or at least log via console.warn (maybe it helps detecting
a missing thing someday), would both not break UI.

>  		    }
>  		}
>  

Re: [pve-devel] [PATCH/RFC widget-toolkit 2/5] apt repositories: just ignore unknown info rather than throwing an error

[Fiona Ebner]

Am 05.06.23 um 19:06 schrieb Thomas Lamprecht:
> Am 05/06/2023 um 17:43 schrieb Fiona Ebner:
>> This will avoid breaking older UI when extending the backend.
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>  src/node/APTRepositories.js | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
>> index cb08bb6..ed58e5b 100644
>> --- a/src/node/APTRepositories.js
>> +++ b/src/node/APTRepositories.js
>> @@ -660,8 +660,6 @@ Ext.define('Proxmox.node.APTRepositories', {
>>  			if (!repoGrid.majorUpgradeAllowed) {
>>  			    infos[path][idx].warnings.push(info);
>>  			}
>> -		    } else {
>> -			throw 'unknown info';
> 
> otherwise we could use some Ext.Msg or at least log via console.warn (maybe it helps detecting
> a missing thing someday), would both not break UI.
> 

Yes, console.warn might be the best, because the message is indicating
an internal error and not very relevant to users.

[pve-devel] [PATCH widget-toolkit 3/5] apt repositories: add classifyOrigin helper

[Fiona Ebner]

to be used again to detect mixed repositories before upgrade.

Needed to convert into an actual function for the 'this' usage.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/node/APTRepositories.js | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index ed58e5b..5971bd8 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -343,14 +343,15 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	    header: gettext('Origin'),
 	    dataIndex: 'Origin',
 	    width: 120,
-	    renderer: (value, meta, rec) => {
+	    renderer: function(value, meta, rec) {
 		if (typeof value !== 'string' || value.length === 0) {
 		    value = gettext('Other');
 		}
 		let cls = 'fa fa-fw fa-question-circle-o';
-		if (value.match(/^\s*Proxmox\s*$/i)) {
+		let originType = this.up('proxmoxNodeAPTRepositories').classifyOrigin(value);
+		if (originType === 'Proxmox') {
 		    cls = 'pmx-itype-icon pmx-itype-icon-proxmox-x';
-		} else if (value.match(/^\s*Debian\s*(:?Backports)?$/i)) {
+		} else if (originType === 'Debian') {
 		    cls = 'pmx-itype-icon pmx-itype-icon-debian-swirl';
 		}
 		return `<i class='${cls}'></i> ${value}`;
@@ -404,6 +405,15 @@ Ext.define('Proxmox.node.APTRepositories', {
 
     product: 'Proxmox VE', // default
 
+    classifyOrigin: function(origin) {
+	if (origin.match(/^\s*Proxmox\s*$/i)) {
+	    return 'Proxmox';
+	} else if (origin.match(/^\s*Debian\s*(:?Backports)?$/i)) {
+	    return 'Debian';
+	}
+	return 'Other';
+    },
+
     controller: {
 	xclass: 'Ext.app.ViewController',
 
-- 
2.39.2

[pve-devel] [PATCH widget-toolkit 4/5] apt repositories: detect mixed suites before major upgrade

[Fiona Ebner]

Usually, differing suites already produce warnings/errors, but before
a major upgrade the current and the next suite are both valid. Mixing
them is an issue though.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/node/APTRepositories.js | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 5971bd8..cba9271 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -460,6 +460,7 @@ Ext.define('Proxmox.node.APTRepositories', {
 	    let nosubscription = vm.get('noSubscriptionRepo');
 	    let test = vm.get('testRepo');
 	    let wrongSuites = vm.get('suitesWarning');
+	    let mixedSuites = vm.get('mixedSuites');
 
 	    if (!enterprise && !nosubscription && !test) {
 		addCritical(
@@ -477,6 +478,10 @@ Ext.define('Proxmox.node.APTRepositories', {
 		addWarn(gettext('Some suites are misconfigured'));
 	    }
 
+	    if (mixedSuites) {
+		addWarn(gettext('Detected mixed suites before upgrade'));
+	    }
+
 	    if (!activeSubscription && enterprise) {
 		addWarn(gettext('The enterprise repository is enabled, but there is no active subscription!'));
 	    }
@@ -507,6 +512,7 @@ Ext.define('Proxmox.node.APTRepositories', {
 	    product: 'Proxmox VE', // default
 	    errors: [],
 	    suitesWarning: false,
+	    mixedSuites: false, // used before major upgrade
 	    subscriptionActive: '',
 	    noSubscriptionRepo: '',
 	    enterpriseRepo: '',
@@ -641,6 +647,11 @@ Ext.define('Proxmox.node.APTRepositories', {
 	    let digest;
 	    let suitesWarning = false;
 
+	    // Usually different suites will give errors anyways, but before a major upgrade the
+	    // current and the next suite are allowed, so it makes sense to check for mixed suites.
+	    let checkMixedSuites = false;
+	    let mixedSuites = false;
+
 	    if (success && records.length > 0) {
 		let data = records[0].data;
 		let files = data.files;
@@ -659,6 +670,9 @@ Ext.define('Proxmox.node.APTRepositories', {
 			infos[path][idx] = {
 			    origin: '',
 			    warnings: [],
+			    // Used as a heuristic to detect mixed repositories pre-upgrade. The
+			    // warning is set on all repositories that do configure the next suite.
+			    gotIgnorePreUpgradeWarning: false,
 			};
 		    }
 
@@ -667,8 +681,11 @@ Ext.define('Proxmox.node.APTRepositories', {
 		    } else if (info.kind === 'warning') {
 			infos[path][idx].warnings.push(info);
 		    } else if (info.kind === 'ignore-pre-upgrade-warning') {
+			infos[path][idx].gotIgnorePreUpgradeWarning = true;
 			if (!repoGrid.majorUpgradeAllowed) {
 			    infos[path][idx].warnings.push(info);
+			} else {
+			    checkMixedSuites = true;
 			}
 		    }
 		}
@@ -683,8 +700,21 @@ Ext.define('Proxmox.node.APTRepositories', {
 			    repo.Origin = infos[file.path][n].origin || Proxmox.Utils.UnknownText;
 			    repo.warnings = infos[file.path][n].warnings || [];
 
-			    if (repo.Enabled && repo.warnings.some(w => w.property === 'Suites')) {
-				suitesWarning = true;
+			    if (repo.Enabled) {
+				if (repo.warnings.some(w => w.property === 'Suites')) {
+				    suitesWarning = true;
+				}
+
+				let originType = me.classifyOrigin(repo.Origin);
+				// Only Proxmox and Debian repositories checked here, because the
+				// warning can be missing for others for a different reason (e.g.
+				// using 'stable' or non-Debian code names).
+				if (checkMixedSuites && repo.Types.includes('deb') &&
+				    (originType === 'Proxmox' || originType === 'Debian') &&
+				    !infos[file.path][n].gotIgnorePreUpgradeWarning
+				) {
+				    mixedSuites = true;
+				}
 			    }
 			}
 			gridData.push(repo);
@@ -700,6 +730,7 @@ Ext.define('Proxmox.node.APTRepositories', {
 
 	    vm.set('errors', errors);
 	    vm.set('suitesWarning', suitesWarning);
+	    vm.set('mixedSuites', mixedSuites);
 	    me.getController().updateState();
 	});
 
-- 
2.39.2

[pve-devel] [PATCH stable-bullseye widget-toolkit 5/5] apt repositoires: allow major upgrade

[Fiona Ebner]

Currently, all this does is silence warnings when the suite after the
current one is configured as well as enabling the check for mixed
repositiories.

Since there is no API call as mentioned in the comment yet, just set
it manually.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Intended only for the stable branch of course.

 src/node/APTRepositories.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index cba9271..1a689b5 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -585,7 +585,7 @@ Ext.define('Proxmox.node.APTRepositories', {
 		nodename: '{nodename}',
 		onlineHelp: '{onlineHelp}',
 	    },
-	    majorUpgradeAllowed: false, // TODO get release information from an API call?
+	    majorUpgradeAllowed: true, // TODO get release information from an API call?
 	    listeners: {
 		selectionchange: 'selectionChange',
 	    },
-- 
2.39.2

[pve-devel] applied-series: [PATCH-SERIES proxmox-apt/widget-toolkit] prepare for major upgrade

[Thomas Lamprecht]

Am 05/06/2023 um 17:43 schrieb Fiona Ebner:
> There's a confusing issue when the suites are already updated,
> because the UI will not detect any Proxmox VE repository anymore then.
> 
> This is fixed by the proxmox-apt patch intended for the stable branch.
> 
> The UI patches make sure the ignore-pre-upgrade-warning is actually
> ignored and add a check for mixed repositories when upgrade is allowed.
> 
> The final UI patch is intended for the stable branch only and makes
> sure that the warning handling expects a major upgrade to be allowed.
> 
> 
> proxmox-apt:
> 
> Fiona Ebner (1):
>   repositories: also detect repository with next suite as configured
> 
>  src/repositories/mod.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 
> proxmox-widget-toolkit:
> 
> Fiona Ebner (5):
>   apt repositories: actually ignore ignore-pre-upgrade-warning
>   apt repositories: just ignore unknown info rather than throwing an
>     error
>   apt repositories: add classifyOrigin helper
>   apt repositories: detect mixed suites before major upgrade
>   apt repositoires: allow major upgrade
> 
>  src/node/APTRepositories.js | 63 ++++++++++++++++++++++++++++++-------
>  1 file changed, 52 insertions(+), 11 deletions(-)
> 


applied series, thanks!