Add addon version check with the repo #750

w13915984028 · 2024-06-03T16:13:24Z

Problem:

Addon & repo chart version mis-matching (e.g. harvester/harvester#5840)

Solution:

Check version matching in the step of build-bundle

In amd64 architecture, the mismatching will cause build failure
In arm64 architecture, the mismatching will print some WARNING messages but build continue

Related Issue:
harvester/harvester#4937
harvester/harvester#5840

Test plan:

make iso output:
amd64

...
charts packed in Harvester repo
total 1.3M
drwxr-xr-x 2 root root 4.0K Jun  3 18:35 .
-rw-r--r-- 1 root root  12K Jun  3 18:35 index.yaml
-rw-r--r-- 1 root root 2.5K Jun  3 18:35 nvidia-driver-runtime-0.1.1.tgz
-rw-r--r-- 1 root root 3.0K Jun  3 18:35 harvester-seeder-0.1.1.tgz
-rw-r--r-- 1 root root 2.8K Jun  3 18:35 harvester-pcidevices-controller-0.3.2.tgz
-rw-r--r-- 1 root root 2.6K Jun  3 18:35 harvester-vm-import-controller-0.1.8.tgz
-rw-r--r-- 1 root root  15K Jun  3 18:35 rancher-logging-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root  82K Jun  3 18:35 rancher-logging-crd-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root 398K Jun  3 18:35 rancher-monitoring-crd-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root 416K Jun  3 18:35 rancher-monitoring-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root  66K Jun  3 18:34 harvester-crd-0.0.0-master-f9464f06.tgz
-rw-r--r-- 1 root root 211K Jun  3 18:34 harvester-0.0.0-master-f9464f06.tgz
drwxrwxr-x 1 root root 4.0K Jun  3 18:34 ..
addon template files
total 16K
drwxr-xr-x 2 root root 4.0K Jun  3 18:34 .
drwxr-xr-x 4 root root 4.0K Jun  3 18:34 ..
-rw-r--r-- 1 root root 7.2K Jun  3 18:34 rancherd-22-addons.yaml
addon: harvester-vm-import-controller version: 0.1.8
addon: harvester-pcidevices-controller version: 0.3.2
addon: rancher-logging version: 103.0.0+up3.17.10
addon: rancher-monitoring version: 103.0.3+up45.31.1
addon: harvester-seeder version: 0.1.1
addon: nvidia-driver-runtime version: 0.1.1

arm64:

...
drwxr-xr-x 2 root root 4.0K Jun  3 18:37 .
-rw-r--r-- 1 root root  11K Jun  3 18:37 index.yaml
-rw-r--r-- 1 root root 2.5K Jun  3 18:37 nvidia-driver-runtime-0.1.1.tgz
-rw-r--r-- 1 root root  15K Jun  3 18:37 rancher-logging-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root  82K Jun  3 18:37 rancher-logging-crd-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root 398K Jun  3 18:37 rancher-monitoring-crd-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root 417K Jun  3 18:37 rancher-monitoring-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root  66K Jun  3 18:37 harvester-crd-0.0.0-master-f9464f06.tgz
-rw-r--r-- 1 root root 215K Jun  3 18:37 harvester-0.0.0-master-f9464f06.tgz
drwxr-xr-x 1 root root 4.0K Jun  3 18:37 ..
addon template files
total 16K
drwxr-xr-x 2 root root 4.0K Jun  3 18:37 .
drwxr-xr-x 4 root root 4.0K Jun  3 18:37 ..
-rw-r--r-- 1 root root 7.2K Jun  3 18:37 rancherd-22-addons.yaml
addon: harvester-vm-import-controller version: 0.1.8
WARNING: addon harvester-vm-import-controller is defined with version 0.1.8 but the chart is not packed into repo in arm64
addon: harvester-pcidevices-controller version: 0.3.2
WARNING: addon harvester-pcidevices-controller is defined with version 0.3.2 but the chart is not packed into repo in arm64
addon: rancher-logging version: 103.0.0+up3.17.10
addon: rancher-monitoring version: 103.0.3+up45.31.1
addon: harvester-seeder version: 0.1.1
WARNING: addon harvester-seeder is defined with version 0.1.1 but the chart is not packed into repo in arm64
addon: nvidia-driver-runtime version: 0.1.1

tserong

I don't understand how this helps with the problem in harvester/harvester#5840. This PR verifies the versions of the addons packed into the ISO, but AFAICT the addons in the v1.2.2 ISO are fine, rather the problem is that there's a mismatch between the seeder chart version in harvester-installer's pkg/config/templates/rancherd-22-addons.yaml (0.1.1):

harvester-installer/pkg/config/templates/rancherd-22-addons.yaml

Line 206 in 3b0b216

version: "0.1.1"

...and in harvester's package/upgrade/addons/harvester-seeder.yaml (0.1.0):

https://github.com/harvester/harvester/blob/ccd7e1cf672b0608d1d50712b31c335d401105a0/package/upgrade/addons/harvester-seeder.yaml#L10

w13915984028 · 2024-06-04T11:13:32Z

@tserong You are right, this PR can't help harvester/harvester#5840. It can only check the matching in the current generated ISO.

For the upgrade path, let's try to find a practical solution, thanks.

w13915984028 · 2024-06-04T11:50:42Z

@tserong The new function check_upgrade_addon_chart_version_matching will check the matching between upgrade addon definition and the chart repo, please take another look, thanks.

tserong

Nicely done. I've tested it against latest master and got the following - it's picking up mismatched rancher-logging and rancher-monitoring versions in the upgrade addons:

charts packed in Harvester repo
total 1.2M
drwxr-xr-x 2 root root 4.0K Jun  5 06:34 .
-rw-r--r-- 1 root root  12K Jun  5 06:34 index.yaml
-rw-r--r-- 1 root root 2.5K Jun  5 06:34 nvidia-driver-runtime-0.1.1.tgz
-rw-r--r-- 1 root root 3.0K Jun  5 06:34 harvester-seeder-0.1.1.tgz
-rw-r--r-- 1 root root 2.8K Jun  5 06:34 harvester-pcidevices-controller-0.3.2.tgz
-rw-r--r-- 1 root root 2.6K Jun  5 06:34 harvester-vm-import-controller-0.1.8.tgz
-rw-r--r-- 1 root root  15K Jun  5 06:34 rancher-logging-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root  82K Jun  5 06:34 rancher-logging-crd-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root 398K Jun  5 06:34 rancher-monitoring-crd-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root 416K Jun  5 06:34 rancher-monitoring-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root  66K Jun  5 06:34 harvester-crd-0.0.0-master-2eeedaad.tgz
-rw-r--r-- 1 root root 209K Jun  5 06:34 harvester-0.0.0-master-2eeedaad.tgz
drwxr-xr-x 1 root root   20 Jun  5 06:34 ..
addon template files
total 8.0K
drwxr-xr-x 2 root root   37 Jun  5 06:34 .
drwxr-xr-x 4 root root   37 Jun  5 06:34 ..
-rw-r--r-- 1 root root 7.2K Jun  5 06:34 rancherd-22-addons.yaml
addon: harvester-vm-import-controller version: 0.1.8
addon: harvester-pcidevices-controller version: 0.3.2
addon: rancher-logging version: 103.0.0+up3.17.10
addon: rancher-monitoring version: 103.0.3+up45.31.1
addon: harvester-seeder version: 0.1.1
addon: nvidia-driver-runtime version: 0.1.1
all addons have matched version with the repo
upgrade addon files
total 28K
drwxr-xr-x 5 root root 4.0K Jun  5 06:34 ..
drwxr-xr-x 2 root root  191 Jun  5 06:34 .
-rw-r--r-- 1 root root  388 Jun  5 06:34 harvester-seeder.yaml
-rw-r--r-- 1 root root 1.8K Jun  5 06:34 logging_addon.yaml
-rw-r--r-- 1 root root 2.3K Jun  5 06:34 monitoring_addon.yaml
-rw-r--r-- 1 root root  475 Jun  5 06:34 nvidia-driver-toolkit.yaml
-rw-r--r-- 1 root root  366 Jun  5 06:34 pcidevices-controller.yaml
-rw-r--r-- 1 root root  362 Jun  5 06:34 vm-import-controller.yaml
upgrade addon: harvester-seeder version: 0.1.1
upgrade addon: rancher-logging version: 102.0.0+up3.17.10
WARNING upgrade addon rancher-logging has version mis-matching: in repo is 103.0.0+up3.17.10 but in addon is 102.0.0+up3.17.10
upgrade addon: rancher-monitoring version: 100.1.0+up19.0.3
WARNING upgrade addon rancher-monitoring has version mis-matching: in repo is 103.0.3+up45.31.1 but in addon is 100.1.0+up19.0.3
upgrade addon: nvidia-driver-runtime version: 0.1.1
upgrade addon: harvester-pcidevices-controller version: 0.3.2
upgrade addon: harvester-vm-import-controller version: 0.1.8
all upgrade addons have matched version with the repo

I also backported it to a local checkout of v1.2.2, and can see it finding the mismatched seeder version:

charts packed in Harvester repo
total 1.2M
drwxr-xr-x 2 root root 4.0K Jun  5 06:28 .
-rw-r--r-- 1 root root  12K Jun  5 06:28 index.yaml
-rw-r--r-- 1 root root 3.0K Jun  5 06:27 harvester-seeder-0.1.1.tgz
-rw-r--r-- 1 root root 2.8K Jun  5 06:27 harvester-pcidevices-controller-0.2.7.tgz
-rw-r--r-- 1 root root 2.6K Jun  5 06:27 harvester-vm-import-controller-0.1.8.tgz
-rw-r--r-- 1 root root  15K Jun  5 06:27 rancher-logging-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root  82K Jun  5 06:27 rancher-logging-crd-103.0.0+up3.17.10.tgz
-rw-r--r-- 1 root root 398K Jun  5 06:27 rancher-monitoring-crd-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root 417K Jun  5 06:27 rancher-monitoring-103.0.3+up45.31.1.tgz
-rw-r--r-- 1 root root  66K Jun  5 06:27 harvester-crd-1.2.2.tgz
-rw-r--r-- 1 root root 190K Jun  5 06:27 harvester-1.2.2.tgz
drwxr-xr-x 1 root root   20 Jun  5 06:27 ..
addon template files
-rw-r--r-- 1 root root 6.5K Jun  4 09:07 ./pkg/config/templates/rancherd-22-addons.yaml
addon: harvester-vm-import-controller version: 0.1.8
addon: harvester-pcidevices-controller version: 0.2.7
addon: rancher-logging version: 103.0.0+up3.17.10
addon: rancher-monitoring version: 103.0.3+up45.31.1
addon: harvester-seeder version: 0.1.1
all addons have matched version with the repo
upgrade addon files
total 24K
drwxr-xr-x 5 root root 4.0K Jun  5 06:27 ..
drwxr-xr-x 2 root root  157 Jun  5 06:27 .
-rw-r--r-- 1 root root  388 Jun  5 06:27 harvester-seeder.yaml
-rw-r--r-- 1 root root 1.8K Jun  5 06:27 logging_addon.yaml
-rw-r--r-- 1 root root 2.3K Jun  5 06:27 monitoring_addon.yaml
-rw-r--r-- 1 root root  366 Jun  5 06:27 pcidevices-controller.yaml
-rw-r--r-- 1 root root  363 Jun  5 06:27 vm-import-controller.yaml
upgrade addon: harvester-seeder version: 0.1.0
WARNING upgrade addon harvester-seeder has version mis-matching: in repo is 0.1.1 but in addon is 0.1.0
upgrade addon: rancher-logging version: 102.0.0+up3.17.10
WARNING upgrade addon rancher-logging has version mis-matching: in repo is 103.0.0+up3.17.10 but in addon is 102.0.0+up3.17.10
upgrade addon: rancher-monitoring version: 100.1.0+up19.0.3
WARNING upgrade addon rancher-monitoring has version mis-matching: in repo is 103.0.3+up45.31.1 but in addon is 100.1.0+up19.0.3
upgrade addon: harvester-pcidevices-controller version: 0.2.7
upgrade addon: harvester-vm-import-controller version: 0.1.8
all upgrade addons have matched version with the repo

tserong · 2024-06-05T06:41:18Z

scripts/build-bundle

+     # the upgrade addon is stored in GH harvester; but the chart repo is stored in GH harvester-installer
+     # a new version bumping generally means two PRs
+     # directly return 1 will cause failure in build
+     echo  WARNING upgrade addon "$chart" has version mis-matching: in repo is "$repover" but in addon is "$version"


I think this should return 1 to fail the build if the upgrade addons version doesn't match. I mean, I assume we don't ever want to build an ISO where the repo addon versions don't match the upgrade addon versions?

The current consideration is:

When bumping a new version, we need:
(1) PR in harvester-installer to add new chart version to repo
(2) Update the addon manifest definition, originally in harvester-installer, but now in addon repository
(3) Update the addon upgrade in Harvester

Three PRs are needed. Before all are merged, the build iso will fail in the middle if we return 1 here, that is a bit not convenient for PR merging.

We may add an option to forcely fail / alternatively continue, and it is used when publishing an ISO.

We may add an option to forcely fail / alternatively continue, and it is used when publishing an ISO.

Perhaps we can check if the harvester version is a tagged version (e.g. v1.3.0), if so, return 1 to fail fast. (Not sure if this is an option, just leave a comment here 😃 )

After harvester/harvester#5859 is done, we can return 1 here.

w13915984028 · 2024-06-06T09:43:59Z

Will rebase code after harvester/harvester#5633 is merged.

w13915984028 · 2024-06-06T10:00:25Z

The current code is manually backported to v1.3 via #754

brandboat · 2024-06-12T02:52:53Z

for previous harvester releases, I think add these checks make sense, we don't want any mismatched version show up. But I'm not sure we still need this in master branch if harvester/harvester#5633 got merged first since after then all addon yamls will be moved to the new addon repos which make sure all the addon version matched.

w13915984028 · 2024-06-12T08:43:55Z

for previous harvester releases, I think add these checks make sense, we don't want any mismatched version show up. But I'm not sure we still need this in master branch if harvester/harvester#5633 got merged first since after then all addon yamls will be moved to the new addon repos which make sure all the addon version matched.

@brandboat

There are 3 places related to addon/chart versions:
(1) Addon manifest
(2) Addon upgrade
(3) Chart repo in Harvester-installer when build-bundle.

The #5633 will move (1) and (2) to same repo, but the files themselves are still split in different folders (?). If true, it it still possible that (1) and (2) are not updated at same time due to kind of mistake. And (1) (2) should be synced with (3) to have identical chart version.

This PR will help us to avoid any potential mismatching.

brandboat

LGTM, the comments are just some nits.

scripts/build-bundle

Signed-off-by: Jian Wang <jian.wang@suse.com>

w13915984028 · 2024-07-04T08:59:09Z

Rebased code after harvester/harvester#6112.

The upgrade addon is generated from the single-source https://github.com/harvester/addons/blob/58ad21070c91bd875df901818c3caddd35a6eefa/pkg/templates/rancherd-22-addons.yaml#L1

This PR will just check the template file once, make sure the charts are matching.

tserong

LGTM! I gave it a quick test too, by setting a version for harvester-seeder in the addons repo's pkg/templates/rancherd-22-addons.yaml which doesn't match HARVESTER_SEEDER_CHART_VERSION in the addons version_info file, and it picks it up nicely:

addon: harvester-vm-import-controller version: 0.3.0
addon: harvester-pcidevices-controller version: 0.3.2
addon: rancher-logging version: 103.0.0+up3.17.10
addon: rancher-monitoring version: 103.0.3+up45.31.1
addon: harvester-seeder version: 0.3.1
addon harvester-seeder has version mis-matching: in repo is 0.3.0 but in addon is 0.3.1
FATA[0037] exit status 1

brandboat

LGTM ! gentle ping @bk201, @ibrokethecloud , could you take a final look ? Many thanks !

bk201 · 2024-07-05T10:32:49Z

@Mergifyio backport v1.4

bk201 · 2024-07-05T10:32:59Z

@w13915984028 Does this also work for v1.3/v1.2?

mergify · 2024-07-05T10:33:18Z

backport v1.4

✅ Backports have been created

#781 Add addon version check with the repo (backport #750) has been created for branch v1.4

w13915984028 · 2024-07-05T10:53:28Z

@bk201 Master-head has harvester/harvester#6112, that's different with v1.3.0

Instead v1.3 needs PR #754 to check version mis-matching.

ibrokethecloud

lgtm. thanks.

ibrokethecloud · 2024-07-09T05:15:03Z

I have also prepared this harvester/addons#4, and should ensure that template and upgrade path manifests are all generated from a common list of environment variables

w13915984028 mentioned this pull request Jun 3, 2024

[BUG] Seeder add-on does not get upgraded during Harvester upgrade v1.2.1 to v1.2.2 harvester/harvester#5840

Open

w13915984028 force-pushed the checkaddon branch 2 times, most recently from ee9d7f9 to 6fc7ada Compare June 3, 2024 18:41

w13915984028 requested review from bk201 and ibrokethecloud June 3, 2024 19:36

bk201 requested a review from tserong June 4, 2024 02:26

tserong reviewed Jun 4, 2024

View reviewed changes

w13915984028 mentioned this pull request Jun 4, 2024

Remove unused monitoring and logging addon upgrade file harvester/harvester#5954

Closed

w13915984028 force-pushed the checkaddon branch 2 times, most recently from b82faac to b78106e Compare June 4, 2024 11:48

w13915984028 requested a review from tserong June 4, 2024 12:53

tserong reviewed Jun 5, 2024

View reviewed changes

w13915984028 mentioned this pull request Jun 5, 2024

[TASK] Packaging ISO from harvester/harvester repo only harvester/harvester#5859

Open

w13915984028 mentioned this pull request Jun 6, 2024

[v1.3] Add addon version check with the repo #754

Open

brandboat approved these changes Jun 12, 2024

View reviewed changes

scripts/build-bundle Show resolved Hide resolved

scripts/build-bundle Outdated Show resolved Hide resolved

scripts/build-bundle Outdated Show resolved Hide resolved

scripts/build-bundle Outdated Show resolved Hide resolved

scripts/build-bundle Outdated Show resolved Hide resolved

Add addon version check with the repo

91e08a2

Signed-off-by: Jian Wang <jian.wang@suse.com>

w13915984028 force-pushed the checkaddon branch from b78106e to 91e08a2 Compare July 4, 2024 08:55

w13915984028 mentioned this pull request Jul 4, 2024

Mark those unavailable addons on ARM64 harvester/docs#605

Open

w13915984028 requested a review from tserong July 4, 2024 09:35

w13915984028 requested a review from brandboat July 4, 2024 09:35

tserong approved these changes Jul 5, 2024

View reviewed changes

brandboat approved these changes Jul 5, 2024

View reviewed changes

bk201 approved these changes Jul 5, 2024

View reviewed changes

ibrokethecloud approved these changes Jul 9, 2024

View reviewed changes

ibrokethecloud approved these changes Jul 11, 2024

View reviewed changes

ibrokethecloud approved these changes Jul 25, 2024

View reviewed changes

tserong merged commit 24a8e73 into harvester:master Jul 26, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add addon version check with the repo #750

Add addon version check with the repo #750

w13915984028 commented Jun 3, 2024 •

edited

Loading

tserong left a comment •

edited

Loading

w13915984028 commented Jun 4, 2024

w13915984028 commented Jun 4, 2024

tserong left a comment

tserong Jun 5, 2024

w13915984028 Jun 5, 2024

brandboat Jun 5, 2024

w13915984028 Jun 5, 2024

w13915984028 commented Jun 6, 2024

w13915984028 commented Jun 6, 2024

brandboat commented Jun 12, 2024

w13915984028 commented Jun 12, 2024

brandboat left a comment

w13915984028 commented Jul 4, 2024

tserong left a comment

brandboat left a comment •

edited

Loading

bk201 commented Jul 5, 2024

bk201 commented Jul 5, 2024

mergify bot commented Jul 5, 2024 •

edited

Loading

w13915984028 commented Jul 5, 2024

ibrokethecloud left a comment

ibrokethecloud commented Jul 9, 2024

Add addon version check with the repo #750

Add addon version check with the repo #750

Conversation

w13915984028 commented Jun 3, 2024 • edited Loading

tserong left a comment • edited Loading

Choose a reason for hiding this comment

w13915984028 commented Jun 4, 2024

w13915984028 commented Jun 4, 2024

tserong left a comment

Choose a reason for hiding this comment

tserong Jun 5, 2024

Choose a reason for hiding this comment

w13915984028 Jun 5, 2024

Choose a reason for hiding this comment

brandboat Jun 5, 2024

Choose a reason for hiding this comment

w13915984028 Jun 5, 2024

Choose a reason for hiding this comment

w13915984028 commented Jun 6, 2024

w13915984028 commented Jun 6, 2024

brandboat commented Jun 12, 2024

w13915984028 commented Jun 12, 2024

brandboat left a comment

Choose a reason for hiding this comment

w13915984028 commented Jul 4, 2024

tserong left a comment

Choose a reason for hiding this comment

brandboat left a comment • edited Loading

Choose a reason for hiding this comment

bk201 commented Jul 5, 2024

bk201 commented Jul 5, 2024

mergify bot commented Jul 5, 2024 • edited Loading

✅ Backports have been created

w13915984028 commented Jul 5, 2024

ibrokethecloud left a comment

Choose a reason for hiding this comment

ibrokethecloud commented Jul 9, 2024

w13915984028 commented Jun 3, 2024 •

edited

Loading

tserong left a comment •

edited

Loading

brandboat left a comment •

edited

Loading

mergify bot commented Jul 5, 2024 •

edited

Loading