Updates pinned images list on config reload

Signed-off-by: roman-kiselenko <roman.kiselenko.dev@gmail.com>

contrib/test/ci/integration.yml

@@ -54,6 +54,7 @@
         state: present
       loop: "{{ ['cgroups.bats'] | product(kata_skip_cgroups_tests) \
         + ['command.bats'] | product(kata_skip_command_tests) \
+        + ['reload_config.bats'] | product(kata_skip_reload_config_tests) \
         + ['config.bats'] | product(kata_skip_config_tests) \
         + ['config_migrate.bats'] | product(kata_skip_config_migrate_tests) \
         + ['crio-wipe.bats'] | product(kata_skip_crio_wipe_tests) \

contrib/test/ci/vars.yml

@@ -98,6 +98,10 @@ kata_skip_cgroups_tests:
 kata_skip_command_tests:
   - 'test "crio commands"'
   - 'test "log max boundary testing"'
+kata_skip_reload_config_tests:
+  - "test \"reload config should update 'pinned_images'\""
+  - "test \"reload config should update 'pinned_images' and only 'pause_image' is pinned\""
+  - "test \"reload config should update 'pause_image' and it becomes 'pinned_images'\""
 kata_skip_config_tests:
   - 'test "choose different default runtime should succeed"'
   - 'test "runc not existing when default_runtime changed should succeed"'

internal/storage/image.go

@@ -152,6 +152,9 @@ type ImageServer interface {
 	// CandidatesForPotentiallyShortImageName resolves an image name into a set of fully-qualified image names (domain/repo/image:tag|@digest).
 	// It will only return an empty slice if err != nil.
 	CandidatesForPotentiallyShortImageName(systemContext *types.SystemContext, imageName string) ([]RegistryImageReference, error)
+
+	// UpdatePinnedImagesList updates pinned and pause images list in imageService.
+	UpdatePinnedImagesList(imageList []string)
 }
 
 func parseImageNames(image *storage.Image) (someName *RegistryImageReference, tags []reference.NamedTagged, digests []reference.Canonical, err error) {
@@ -898,6 +901,11 @@ func (st nativeStorageTransport) ResolveReference(ref types.ImageReference) (typ
 	return istorage.ResolveReference(ref)
 }
 
+// UpdatePinnedImagesList updates pinned images list in imageService.
+func (svc *imageService) UpdatePinnedImagesList(pinnedImages []string) {
+	svc.regexForPinnedImages = CompileRegexpsForPinnedImages(pinnedImages)
+}
+
 // FilterPinnedImage checks if the given image needs to be pinned
 // and excluded from kubelet's image GC.
 func FilterPinnedImage(image string, pinnedImages []*regexp.Regexp) bool {

pkg/config/reload.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"strconv"
+	"strings"
 
 	"github.com/containers/image/v5/pkg/sysregistriesv2"
 	"github.com/cri-o/cri-o/internal/log"
@@ -20,7 +21,7 @@ func (c *Config) Reload() error {
 	// Reload the config
 	newConfig, err := DefaultConfig()
 	if err != nil {
-		return errors.New("unable to create default config")
+		return fmt.Errorf("unable to create default config: %w", err)
 	}
 
 	if _, err := os.Stat(c.singleConfigPath); !os.IsNotExist(err) {
@@ -145,17 +146,23 @@ func (c *Config) ReloadPauseImage(newConfig *Config) error {
 }
 
 // ReloadPinnedImages updates the PinnedImages with the provided `newConfig`.
+// The method print log in case of any updates.
 func (c *Config) ReloadPinnedImages(newConfig *Config) {
 	updatedPinnedImages := make([]string, len(newConfig.PinnedImages))
+	updatedPinnedImageList := false
+
 	for i, image := range newConfig.PinnedImages {
 		if i < len(c.PinnedImages) && image == c.PinnedImages[i] {
-			updatedPinnedImages[i] = c.PinnedImages[i]
-		} else {
-			updatedPinnedImages[i] = image
+			continue
 		}
+		updatedPinnedImages[i] = image
+		updatedPinnedImageList = true
+	}
+
+	if updatedPinnedImageList {
+		logConfig("pinned_images", strings.Join(updatedPinnedImages, ", "))
+		c.PinnedImages = updatedPinnedImages
 	}
-	logrus.Infof("Updated new pinned images: %+v", updatedPinnedImages)
-	c.PinnedImages = updatedPinnedImages
 }
 
 // ReloadRegistries reloads the registry configuration from the Configs

server/server.go

@@ -577,6 +577,9 @@ func (s *Server) startReloadWatcher(ctx context.Context) {
 				logrus.Errorf("Unable to reload configuration: %v", err)
 				continue
 			}
+			// ImageServer compiles the list with regex for both
+			// pinned and sandbox/pause images, we need to update them
+			s.StorageImageServer().UpdatePinnedImagesList(append(s.config.PinnedImages, s.config.PauseImage))
 		}
 	}()
 

test/reload_config.bats

@@ -230,3 +230,38 @@ EOF
 	#then
 	wait_for_log '"updating runtime configuration"'
 }
+
+@test "reload config should update 'pinned_images'" {
+	OPTION="pinned_images"
+	EXAMPLE_IMAGE=quay.io/crio/fedora-crio-ci:latest
+	# add pinned_images config
+	printf '[crio.image]\npinned_images = [""]\n' > "$CRIO_CONFIG_DIR"/00-default
+	printf '[crio.image]\npinned_images = ["%s"]\n' $EXAMPLE_IMAGE > "$CRIO_CONFIG_DIR"/01-overwrite
+	# image is not pinned
+	output=$(crictl images -o json | jq ".images[] | select(.repoTags[] == \"$EXAMPLE_IMAGE\") |.pinned")
+	[ "$output" == "false" ]
+	reload_crio
+	# image becomes pinned
+	expect_log_success $OPTION $EXAMPLE_IMAGE
+	output=$(crictl images -o json | jq ".images[] | select(.repoTags[] == \"$EXAMPLE_IMAGE\") |.pinned")
+	[ "$output" == "true" ]
+}
+
+@test "reload config should update 'pinned_images' and only 'pause_image' is pinned" {
+	printf '[crio.image]\npinned_images = [""]\n' > "$CRIO_CONFIG_DIR"/00-default
+	reload_crio
+	output=$(crictl images -o json | jq '.images[] | select(.pinned == true) | .repoTags[]')
+	# only pause image is pinned
+	[[ "$output" == *"pause"* ]]
+}
+
+@test "reload config should update 'pause_image' and it becomes 'pinned_images'" {
+	OPTION="pause_image"
+	EXAMPLE_IMAGE=quay.io/crio/fedora-crio-ci:latest
+	printf '[crio.image]\npinned_images = [""]\npause_image = "%s"\n' $EXAMPLE_IMAGE > "$CRIO_CONFIG_DIR"/04-overwrite
+	reload_crio
+	expect_log_success $OPTION $EXAMPLE_IMAGE
+	output=$(crictl images -o json | jq '.images[] | select(.pinned == true)')
+	# pause image is pinned
+	[[ "$output" == *"fedora-crio-ci"* ]]
+}