fix: Always refresh the default hooks during upgrades #5009

petrutlucian94 · 2025-04-03T08:41:29Z

Always refresh the default hooks during upgrades

At the moment, the default hooks are not updated when refreshing
the snap[1].

There's an explicit check so this was most probably by design so that
custom hooks would not get overridden[2]. However, it means that bug
fixes are not applied automatically and the users need to copy the
files manually, which is undocumented and not very user friendly.

Warning:
This change will always refresh the default hooks, replacing
the existing files. Custom hooks are expected to use separate files
so that they won't get overridden during upgrades.

[1] #4819
[2] #4819 (comment)

Summary

Changes

Testing

Possible Regressions

Checklist

Read the contributions page.
Submitted the CLA form, if you are a first time contributor.
The introduced changes are covered by unit and/or integration tests.

Notes

petrutlucian94 · 2025-04-04T07:20:00Z

Here's an example that reproduces and then fixes the issue:

sudo snap install microk8s --channel=1.31/stable --classic
sudo microk8s status --wait-ready
# IMPORTANT: the issue only occurs when having the "rbac" extension enabled.
sudo microk8s enable rbac
sudo snap refresh microk8s --channel=1.32/edge --classic

sudo snap logs microk8s.daemon-apiserver-kicker -n 20
# output #
2025-04-04T07:04:16Z sudo[3821984]: pam_unix(sudo:session): session closed for user root
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821993]: Error from server (Forbidden): pods is forbidden: User "system:node:ubuntusnaptests" cannot list resource "pods" in API group "" at the cluster scope: can only list/watch pods with spec.nodeName field selector
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]: Traceback (most recent call last):
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/scripts/kill-host-pods.py", line 104, in <module>
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     main()
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/usr/lib/python3/dist-packages/click/core.py", line 764, in __call__
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     return self.main(*args, **kwargs)
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/usr/lib/python3/dist-packages/click/core.py", line 717, in main
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     rv = self.invoke(ctx)
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/usr/lib/python3/dist-packages/click/core.py", line 956, in invoke
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     return ctx.invoke(self.callback, **ctx.params)
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/usr/lib/python3/dist-packages/click/core.py", line 555, in invoke
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     return callback(*args, **kwargs)
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/scripts/kill-host-pods.py", line 84, in main
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     out = subprocess.check_output([*KUBECTL, "get", "pod", "-o", "json", *selector])
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/usr/lib/python3.8/subprocess.py", line 415, in check_output
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:   File "/snap/microk8s/8004/usr/lib/python3.8/subprocess.py", line 516, in run
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]:     raise CalledProcessError(retcode, process.args,
2025-04-04T07:04:16Z microk8s.daemon-apiserver-kicker[3821973]: subprocess.CalledProcessError: Command '['/snap/microk8s/8004/kubectl', '--kubeconfig=/var/snap/microk8s/8004/credentials/kubelet.config', 'get', 'pod', '-o', 'json', '-A']' returned non-zero exit status 1.

# fix:
sudo cp -r /snap/microk8s/current/default-hooks/* /var/snap/microk8s/common/hooks/
# double check the restart hook uses a node selector
sudo cat /var/snap/microk8s/common/hooks/reconcile.d/10-pods-restart

sudo snap restart microk8s

Another PR fixed one of the default hooks [1], however we're not copying over the hooks from $SNAP/default-hooks to $SNAP/hooks when refreshing existing snap installations [2]. This is probably by design so that we don't override user defined hooks However, it doesn't seem to be documented anywhere. As suggested by the team, this patch will copy over the reconcile.d/10-pods-restart hook. Downsides: * copying just one of the hooks seems a bit unintuitive and inconsistent * we may override user hooks, which can be unexpected Alternatives: * document the fact that these hooks are not refreshed automatically and that users can/should copy over the default hooks * always refresh all hooks [1] #4819 [2] #4819 (comment)

At the moment, the default hooks are not updated when refreshing the snap. There's an explicit check so this was most probably by design so that custom hooks would not get overriden. However, it means that bug fixes are not applied automatically and the users need to copy the files manually, which is undocumented and not very user friendly. This change will always refresh the default hooks, replacing the existing files. Custom hooks are expected to use separate files so that they won't get overriden during upgrades.

louiseschmidtgen

LGTM

berkayoz

LGTM, one final change

build-scripts/addons/repositories.sh

louiseschmidtgen

Nice work @petrutlucian94!

louiseschmidtgen

LGTM

We'll use the same workaround as k8s-snap.

berkayoz · 2025-04-15T13:03:18Z

/backport

cdkbot · 2025-04-15T13:03:33Z

Backport failed for 1.32, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 1.32
git worktree add -d .worktree/backport-5009-to-1.32 origin/1.32
cd .worktree/backport-5009-to-1.32
git switch --create backport-5009-to-1.32
git cherry-pick -x 0f715dbef8fed519de7e97cf2b4e862135e00944

cdkbot · 2025-04-15T13:03:34Z

Backport failed for 1.32-strict, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 1.32-strict
git worktree add -d .worktree/backport-5009-to-1.32-strict origin/1.32-strict
cd .worktree/backport-5009-to-1.32-strict
git switch --create backport-5009-to-1.32-strict
git cherry-pick -x 0f715dbef8fed519de7e97cf2b4e862135e00944

(cherry picked from commit 0f715db)

petrutlucian94 changed the title ~~rfc: copy one of the hooks when refreshing the snap~~ Always refresh the default hooks during upgrades Apr 7, 2025

petrutlucian94 added 3 commits April 7, 2025 13:36

Increase the timeout for community addon tests

e65071b

petrutlucian94 force-pushed the lpetrut/copy-hook-on-refresh branch from 375bb78 to e65071b Compare April 7, 2025 13:37

petrutlucian94 changed the title ~~Always refresh the default hooks during upgrades~~ fix: Always refresh the default hooks during upgrades Apr 8, 2025

louiseschmidtgen approved these changes Apr 8, 2025

View reviewed changes

berkayoz requested changes Apr 9, 2025

View reviewed changes

build-scripts/addons/repositories.sh Outdated Show resolved Hide resolved

petrutlucian94 force-pushed the lpetrut/copy-hook-on-refresh branch 2 times, most recently from 356b413 to 94105c1 Compare April 9, 2025 17:05

petrutlucian94 added 2 commits April 10, 2025 05:58

test: Include additional information when a pod is unavailable

0a118b2

Update dashboard validator

4509992

petrutlucian94 force-pushed the lpetrut/copy-hook-on-refresh branch from 94105c1 to 4509992 Compare April 10, 2025 06:02

louiseschmidtgen approved these changes Apr 10, 2025

View reviewed changes

petrutlucian94 force-pushed the lpetrut/copy-hook-on-refresh branch 2 times, most recently from 88e914d to 7ed45dc Compare April 10, 2025 14:29

petrutlucian94 added 2 commits April 10, 2025 15:25

avoid newgrp err

35ae4e5

We'll use the same workaround as k8s-snap.

Use ubuntu-latest

a0b851d

petrutlucian94 force-pushed the lpetrut/copy-hook-on-refresh branch from 7ed45dc to a0b851d Compare April 10, 2025 15:25

petrutlucian94 merged commit 0f715db into master Apr 10, 2025
12 checks passed

petrutlucian94 deleted the lpetrut/copy-hook-on-refresh branch April 10, 2025 16:22

berkayoz added backport 1.32 backport 1.32-strict labels Apr 15, 2025

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

3687cc3

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

5b3c425

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

374d044

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

33e2667

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

bf5382f

(cherry picked from commit 0f715db)

berkayoz mentioned this pull request Apr 15, 2025

[1.32] Backport CI fixes from main #5029

Merged

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

d047e01

(cherry picked from commit 0f715db)

berkayoz mentioned this pull request Apr 15, 2025

[1.32-strict] Backport CI fixes from main #5030

Merged

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

c37e284

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

c5787c8

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

e7a93f7

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

c4ec4e3

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 15, 2025

fix: Always refresh the default hooks during upgrades (#5009)

7c47a8b

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

18f88c2

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

0177f3b

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

c8f9e48

(cherry picked from commit 0f715db)

berkayoz mentioned this pull request Apr 16, 2025

[1.31] Backport CI fixes from main #5039

Merged

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

5717a26

(cherry picked from commit 0f715db)

berkayoz mentioned this pull request Apr 16, 2025

[1.31-strict] Backport CI fixes from main #5040

Merged

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

712e26d

(cherry picked from commit 0f715db)

berkayoz mentioned this pull request Apr 16, 2025

[1.30] Backport CI fixes from main #5041

Merged

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

de1f62b

(cherry picked from commit 0f715db)

berkayoz mentioned this pull request Apr 16, 2025

[1.30-strict] Backport CI fixes from main #5042

Merged

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

a7f4658

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

1828f6e

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

0901143

(cherry picked from commit 0f715db)

berkayoz pushed a commit that referenced this pull request Apr 16, 2025

fix: Always refresh the default hooks during upgrades (#5009)

61da67f

(cherry picked from commit 0f715db)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Always refresh the default hooks during upgrades #5009

fix: Always refresh the default hooks during upgrades #5009

Uh oh!

petrutlucian94 commented Apr 3, 2025 •

edited by louiseschmidtgen

Loading

Uh oh!

petrutlucian94 commented Apr 4, 2025 •

edited

Loading

Uh oh!

louiseschmidtgen left a comment

Uh oh!

berkayoz left a comment

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Uh oh!

louiseschmidtgen left a comment

Uh oh!

Uh oh!

berkayoz commented Apr 15, 2025

Uh oh!

cdkbot commented Apr 15, 2025

Uh oh!

cdkbot commented Apr 15, 2025

Uh oh!

Uh oh!

fix: Always refresh the default hooks during upgrades #5009

fix: Always refresh the default hooks during upgrades #5009

Uh oh!

Conversation

petrutlucian94 commented Apr 3, 2025 • edited by louiseschmidtgen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Always refresh the default hooks during upgrades

Summary

Changes

Testing

Possible Regressions

Checklist

Notes

Uh oh!

petrutlucian94 commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Choose a reason for hiding this comment

Uh oh!

berkayoz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

louiseschmidtgen left a comment

Choose a reason for hiding this comment

Uh oh!

louiseschmidtgen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

berkayoz commented Apr 15, 2025

Uh oh!

cdkbot commented Apr 15, 2025

Uh oh!

cdkbot commented Apr 15, 2025

Uh oh!

Uh oh!

petrutlucian94 commented Apr 3, 2025 •

edited by louiseschmidtgen

Loading

petrutlucian94 commented Apr 4, 2025 •

edited

Loading