Auto import images for containerd image store #10973

vitorsavian · 2024-10-02T15:46:51Z

Proposed Changes

ADR
Auto import images form .txt files and tar files

Types of Changes

New Feature

Verification

run a k3s server

k3s server

create the images folder

mkdir /var/lib/rancher/k3s/agent/images

create a .txt file and add a docker.io image path to the .txt file

docker.io/library/mysql:latest

move the file to /var/lib/rancher/k3s/agent/images

cp example.txt /var/lib/rancher/k3s/agent/images

then see if the file was imported in the containerd image store

k3s ctr images list | grep "mysql"

you can also change the file inside var/lib/rancher/k3s/agent/images

nano /var/lib/rancher/k3s/agent/images/example.txt

then you can add

docker.io/library/redis:latest

the controller will see the change and then import the redis image

k3s ctr images list | grep "redis"

Testing

Linked Issues

User-Facing Change

Users can now auto import images to containerd by just throwing the image in the agent/images folder while k3s is running

Further Comments

docs/adrs/add-auto-import-embedded-registry.md

brandond

Users can now add images in the embedded registry auto import folder while k3s is running

To avoid confusion, remove any references to importing things into the embedded registry - the embedded registry does not have a discrete image store to import into. We import images into the containerd image store, and images in the containerd image store can be used by Kubernetes pods without needing to be pulled from a remote registry, and they can also be shared between nodes by the embedded registry.

docs/adrs/add-auto-import-embedded-registry.md

codecov · 2024-10-10T15:56:43Z

Codecov Report

Attention: Patch coverage is 45.20548% with 120 lines in your changes missing coverage. Please review.

Project coverage is 45.37%. Comparing base (4ec2617) to head (e5e2332).
Report is 51 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/agent/containerd/watcher.go	46.63%	79 Missing and 24 partials ⚠️
pkg/agent/containerd/containerd.go	34.61%	11 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10973      +/-   ##
==========================================
- Coverage   46.98%   45.37%   -1.61%     
==========================================
  Files         179      180       +1     
  Lines       18610    18812     +202     
==========================================
- Hits         8743     8536     -207     
- Misses       8505     8972     +467     
+ Partials     1362     1304      -58

Flag	Coverage Δ
e2etests	`39.67% <45.20%> (-1.74%)`	⬇️
inttests	`34.63% <19.17%> (-0.13%)`	⬇️
unittests	`13.67% <0.00%> (-0.15%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/agent/containerd/containerd.go

brandond · 2024-10-21T23:04:03Z

We probably shouldn't do it in this PR, but it would be nice if whatever we used for watching image file changes was generic, and could be reused by the deploy controller - right now it just re-scans everything once a minute:

k3s/pkg/deploy/controller.go

Lines 85 to 100 in 8ce04d3

    
           // start calls listFiles at regular intervals to trigger application of manifests that have changed on disk. 
        
           func (w *watcher) start(ctx context.Context, client kubernetes.Interface) { 
        
           	w.recorder = pkgutil.BuildControllerEventRecorder(client, ControllerName, metav1.NamespaceSystem) 
        
           	force := true 
        
           	for { 
        
           		if err := w.listFiles(force); err == nil { 
        
           			force = false 
        
           		} else { 
        
           			logrus.Errorf("Failed to process config: %v", err) 
        
           		} 
        
           		select { 
        
           		case <-ctx.Done(): 
        
           			return 
        
           		case <-time.After(15 * time.Second): 
        
           		} 
        
           	}

brandond · 2024-10-30T21:15:42Z

pkg/agent/containerd/containerd.go

-	if os.IsNotExist(err) {
+	err := os.MkdirAll(cfg.Images, 0744)
+	if err != nil {
+		logrus.Errorf("Unable to create agent/images folder in %s: %v", cfg.Images, err)


Why are we creating this now? Must the folder exist in order to watch it? Is there any other way to handle this?

What happens if the user makes the images dir a symlink to another path? What happens if the image dir is deleted and recreated while the watcher is going? These are all things that users are going to try at some point.

The behavior after the changes now are that the image watcher will watch the k3s/agent and only work with the events that contains the agent/images, if the image folder is already created, the preloadImages will load the images, so the watcher will not need to load, but if the image folder is created or symlink with a folder with images inside, the watcher will send to the workqueue for the images to be loaded.

If the folder is deleted, fsnotify can auto delete and because we are watching the /k3s/agent, if the user creates the folder again, we will watch it.

pkg/agent/containerd/watcher.go

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Signed-off-by: Vitor Savian <vitor.savian@suse.com> Set local false in the test Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

brandond

minor nit, lgtm otherwise!

pkg/agent/containerd/containerd.go

go.mod

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

manuelbuil

Sorry for the late review. I'm a bit confused about the use case. My guess is: we have a cluster where we would like only one (or more) node(s) to pull the images for the whole cluster. The rest of the nodes would pull the images from these selected nodes by using the embedded registry mirror. We can't easily do this today because containerd will only pull the images if there is a local pod requiring it. This feature would "force" containerd to pull the image, even if no local pod requires it. Is my understanding correct?

manuelbuil · 2024-12-18T15:03:46Z

docs/adrs/add-auto-import-containerd.md

+
+## Context
+
+Since the feature for embedded registry, the users appeared with a question about having to manually import images, specially in edge environments.


Why would users have to manually import images? Aren't they automatically pulled by containerd once the pod is scheduled in the node?

This is an improvement to the existing mechanism of importing images via the existingagent/images folder. Prior to this this change, you could specify the image tar or image list only at k3s boot. now it'll be possible to continuously check for changes to the image list. This is very helpful in cases like airgap installs where you'd want to update the images for a new deployment instead of restarting k3s or doing outproc processing to import images manually.

manuelbuil · 2024-12-18T15:11:20Z

tests/e2e/autoimport/autoimport_test.go

+
+			Eventually(func(g Gomega) {
+				cmd := `k3s ctr images list | grep library/redis`
+				g.Expect(e2e.RunCmdOnNode(cmd, serverNodeNames[0])).Should(ContainSubstring("io.cattle.k3s.pinned=pinned"))


What does the pinned tag mean? That is was manually pulled?

Kubelet garbage collector removes the image if it's not used, so to prevent images that k3s imported from being removed, we pin them.

You can see more here -> https://kubernetes.io/docs/concepts/architecture/garbage-collection/#containers-images

manuelbuil · 2024-12-18T15:13:01Z

tests/e2e/autoimport/autoimport_test.go

+		})
+
+		It("Restarts normally", func() {
+			errRestart := e2e.RestartCluster(append(serverNodeNames, agentNodeNames...))


Why do we need to restart the cluster?

The first restart is to ensure the image file was imported and pinned as expected, the second restart is to ensure if the image that was removed from the /agent/images was not pinned by k3s.

Signed-off-by: Derek Nola <derek.nola@suse.com>

vitorsavian requested a review from a team as a code owner October 2, 2024 15:46

harsimranmaan reviewed Oct 2, 2024

View reviewed changes

docs/adrs/add-auto-import-embedded-registry.md Outdated Show resolved Hide resolved

brandond requested changes Oct 2, 2024

View reviewed changes

brandond requested changes Oct 3, 2024

View reviewed changes

docs/adrs/add-auto-import-embedded-registry.md Outdated Show resolved Hide resolved

docs/adrs/add-auto-import-embedded-registry.md Outdated Show resolved Hide resolved

vitorsavian force-pushed the auto-import-embedded-registry branch from 88b30cb to d605c59 Compare October 10, 2024 20:34

brandond requested changes Oct 11, 2024

View reviewed changes

vitorsavian changed the title ~~[ADR][WIP] Auto import images for embedded registry~~ [WIP] Auto import images for embedded registry Oct 18, 2024

vitorsavian marked this pull request as draft October 18, 2024 14:42

vitorsavian force-pushed the auto-import-embedded-registry branch 4 times, most recently from fa7e775 to 10251df Compare October 28, 2024 07:33

vitorsavian changed the title ~~[WIP] Auto import images for embedded registry~~ [WIP] Auto import images for containerd image store Oct 28, 2024

vitorsavian force-pushed the auto-import-embedded-registry branch 2 times, most recently from 5218084 to 1893d58 Compare October 29, 2024 15:05

vitorsavian changed the title ~~[WIP] Auto import images for containerd image store~~ Auto import images for containerd image store Oct 29, 2024

vitorsavian marked this pull request as ready for review October 29, 2024 16:14

vitorsavian requested review from brandond and a team October 29, 2024 18:18

vitorsavian force-pushed the auto-import-embedded-registry branch 4 times, most recently from e9755b6 to c806d29 Compare October 30, 2024 19:27

brandond reviewed Oct 30, 2024

View reviewed changes

brandond requested changes Oct 30, 2024

View reviewed changes

vitorsavian force-pushed the auto-import-embedded-registry branch 3 times, most recently from 7f194f4 to 28919f5 Compare November 12, 2024 14:12

vitorsavian added 13 commits December 2, 2024 12:24

Add label for auto import when image is already pulled

55e1bd0

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Add steps for pinned images

9558dd8

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Add more steps to verify restart

6fe5cc8

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Add more steps to verify restart

c7d8e1d

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Remove daemonset from restart

9cfc7e4

Signed-off-by: Vitor Savian <vitor.savian@suse.com> Set local false in the test Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Remove auto import label

ec26517

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Fix EOF error log when importing tarball files

926186d

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Delaying queue

62b4244

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Use two seconds instead of one

6de98d9

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Change to event cache and file cache

d9854b6

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Remove useless workqueue.Done

e7659d0

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Refactor

2a71842

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

Add parse for images

98f3c9b

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

vitorsavian force-pushed the auto-import-embedded-registry branch from c8f01d2 to 98f3c9b Compare December 2, 2024 15:25

brandond requested changes Dec 2, 2024

View reviewed changes

pkg/agent/containerd/containerd.go Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

Addressing comments

e5e2332

Signed-off-by: Vitor Savian <vitor.savian@suse.com>

brandond approved these changes Dec 3, 2024

View reviewed changes

vitorsavian requested review from harsimranmaan, dereknola and a team and removed request for harsimranmaan December 3, 2024 02:45

harsimranmaan approved these changes Dec 3, 2024

View reviewed changes

dereknola approved these changes Dec 5, 2024

View reviewed changes

manuelbuil reviewed Dec 18, 2024

View reviewed changes

vitorsavian merged commit 7e18c69 into k3s-io:master Jan 9, 2025
39 checks passed

dereknola added a commit to dereknola/k3s that referenced this pull request Jan 9, 2025

Fix go.mod from k3s-io#10973

24b2c68

Signed-off-by: Derek Nola <derek.nola@suse.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto import images for containerd image store #10973

Auto import images for containerd image store #10973

vitorsavian commented Oct 2, 2024 •

edited by dereknola

Loading

brandond left a comment •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading

brandond commented Oct 21, 2024 •

edited

Loading

brandond Oct 30, 2024

brandond Oct 30, 2024 •

edited

Loading

vitorsavian Nov 11, 2024

brandond left a comment

manuelbuil left a comment

manuelbuil Dec 18, 2024

harsimranmaan Jan 3, 2025

manuelbuil Dec 18, 2024

vitorsavian Jan 6, 2025

manuelbuil Dec 18, 2024

vitorsavian Jan 6, 2025


		## Context

		Since the feature for embedded registry, the users appeared with a question about having to manually import images, specially in edge environments.

Auto import images for containerd image store #10973

Auto import images for containerd image store #10973

Conversation

vitorsavian commented Oct 2, 2024 • edited by dereknola Loading

Proposed Changes

Types of Changes

Verification

Testing

Linked Issues

User-Facing Change

Further Comments

brandond left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Oct 10, 2024 • edited Loading

Codecov Report

brandond commented Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

brandond Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandond left a comment

Choose a reason for hiding this comment

manuelbuil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitorsavian commented Oct 2, 2024 •

edited by dereknola

Loading

brandond left a comment •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading

brandond commented Oct 21, 2024 •

edited

Loading

brandond Oct 30, 2024 •

edited

Loading