Updates pinned images list on config reload

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

internal/storage/image.go

@@ -151,6 +151,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 images list in imageService.
+	UpdatePinnedImagesList(configPinnedImage []string)
 }
 
 func parseImageNames(image *storage.Image) (someName *RegistryImageReference, tags []reference.NamedTagged, digests []reference.Canonical, err error) {
@@ -897,6 +900,11 @@ func (st nativeStorageTransport) ResolveReference(ref types.ImageReference) (typ
 	return istorage.ResolveReference(ref)
 }
 
+// UpdatePinnedImagesList updates pinned images list in imageService.
+func (svc *imageService) UpdatePinnedImagesList(configPinnedImage []string) {
+	svc.regexForPinnedImages = append(svc.regexForPinnedImages, CompileRegexpsForPinnedImages(configPinnedImage)...)
+}
+
 // 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

@@ -13,20 +13,20 @@ import (
 )
 
 // Reload reloads the configuration for the single crio.conf and the drop-in
-// configuration directory.
-func (c *Config) Reload() error {
+// configuration directory. Returns updated Config struct.
+func (c *Config) Reload() (*Config, error) {
 	logrus.Infof("Reloading configuration")
 
 	// Reload the config
 	newConfig, err := DefaultConfig()
 	if err != nil {
-		return errors.New("unable to create default config")
+		return nil, fmt.Errorf("unable to create default config: %w", err)
 	}
 
 	if _, err := os.Stat(c.singleConfigPath); !os.IsNotExist(err) {
 		logrus.Infof("Updating config from file %s", c.singleConfigPath)
 		if err := newConfig.UpdateFromFile(c.singleConfigPath); err != nil {
-			return err
+			return newConfig, err
 		}
 	} else {
 		logrus.Infof("Skipping not-existing config file %q", c.singleConfigPath)
@@ -35,45 +35,44 @@ func (c *Config) Reload() error {
 	if _, err := os.Stat(c.dropInConfigDir); !os.IsNotExist(err) {
 		logrus.Infof("Updating config from path %s", c.dropInConfigDir)
 		if err := newConfig.UpdateFromPath(c.dropInConfigDir); err != nil {
-			return err
+			return newConfig, err
 		}
 	} else {
 		logrus.Infof("Skipping not-existing config path %q", c.dropInConfigDir)
 	}
 
 	// Reload all available options
 	if err := c.ReloadLogLevel(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
 	if err := c.ReloadLogFilter(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
 	if err := c.ReloadPauseImage(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
-	c.ReloadPinnedImages(newConfig)
 	if err := c.ReloadRegistries(); err != nil {
-		return err
+		return newConfig, err
 	}
 	c.ReloadDecryptionKeyConfig(newConfig)
 	if err := c.ReloadSeccompProfile(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
 	if err := c.ReloadAppArmorProfile(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
 	if err := c.ReloadBlockIOConfig(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
 	if err := c.ReloadRdtConfig(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
 	if err := c.ReloadRuntimes(newConfig); err != nil {
-		return err
+		return newConfig, err
 	}
-	cdi.GetRegistry(cdi.WithSpecDirs(newConfig.CDISpecDirs...))
 
-	return nil
+	cdi.GetRegistry(cdi.WithSpecDirs(newConfig.CDISpecDirs...))
+	return newConfig, nil
 }
 
 // logConfig logs a config set operation as with info verbosity. Please always
@@ -143,17 +142,24 @@ func (c *Config) ReloadPauseImage(newConfig *Config) error {
 }
 
 // ReloadPinnedImages updates the PinnedImages with the provided `newConfig`.
-func (c *Config) ReloadPinnedImages(newConfig *Config) {
+func (c *Config) ReloadPinnedImages(newConfig *Config) bool {
 	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 {
+		logrus.Infof("Updated new pinned images: %+v", updatedPinnedImages)
+		c.PinnedImages = updatedPinnedImages
 	}
-	logrus.Infof("Updated new pinned images: %+v", updatedPinnedImages)
-	c.PinnedImages = updatedPinnedImages
+
+	return updatedPinnedImageList
 }
 
 // ReloadRegistries reloads the registry configuration from the Configs

pkg/config/reload_test.go

@@ -35,9 +35,10 @@ var _ = t.Describe("Config", func() {
 			Expect(sut.UpdateFromFile(filePath)).To(Succeed())
 
 			// When
-			err := sut.Reload()
+			cfg, err := sut.Reload()
 
 			// Then
+			Expect(cfg).ToNot(BeNil())
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -49,9 +50,10 @@ var _ = t.Describe("Config", func() {
 			)
 
 			// When
-			err := sut.Reload()
+			cfg, err := sut.Reload()
 
 			// Then
+			Expect(cfg).ToNot(BeNil())
 			Expect(err).To(HaveOccurred())
 		})
 
@@ -63,9 +65,10 @@ var _ = t.Describe("Config", func() {
 			)
 
 			// When
-			err := sut.Reload()
+			cfg, err := sut.Reload()
 
 			// Then
+			Expect(cfg).ToNot(BeNil())
 			Expect(err).To(HaveOccurred())
 		})
 
@@ -77,9 +80,10 @@ var _ = t.Describe("Config", func() {
 			)
 
 			// When
-			err := sut.Reload()
+			cfg, err := sut.Reload()
 
 			// Then
+			Expect(cfg).ToNot(BeNil())
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})
@@ -427,15 +431,17 @@ var _ = t.Describe("Config", func() {
 			sut.PinnedImages = []string{"image1", "image4", "image3"}
 			newConfig := &config.Config{}
 			newConfig.PinnedImages = []string{"image5"}
-			sut.ReloadPinnedImages(newConfig)
+			ok := sut.ReloadPinnedImages(newConfig)
+			Expect(ok).To(BeTrue())
 			Expect(sut.PinnedImages).To(Equal([]string{"image5"}))
 		})
 
 		It("should not update PinnedImages if they are the same as newConfig's PinnedImages", func() {
 			sut.PinnedImages = []string{"image1", "image2", "image3"}
 			newConfig := &config.Config{}
 			newConfig.PinnedImages = []string{"image1", "image2", "image3"}
-			sut.ReloadPinnedImages(newConfig)
+			ok := sut.ReloadPinnedImages(newConfig)
+			Expect(ok).To(BeFalse())
 			Expect(sut.PinnedImages).To(Equal([]string{"image1", "image2", "image3"}))
 		})
 	})

server/server.go

@@ -573,10 +573,14 @@ func (s *Server) startReloadWatcher(ctx context.Context) {
 		for {
 			// Block until the signal is received
 			<-ch
-			if err := s.config.Reload(); err != nil {
+			newConfig, err := s.config.Reload()
+			if err != nil {
 				logrus.Errorf("Unable to reload configuration: %v", err)
 				continue
 			}
+			if s.config.ReloadPinnedImages(newConfig) {
+				s.StorageImageServer().UpdatePinnedImagesList(s.config.PinnedImages)
+			}
 		}
 	}()
 

test/reload_config.bats

@@ -230,3 +230,18 @@ EOF
 	#then
 	wait_for_log '"updating runtime configuration"'
 }
+
+@test "reload config should update 'pinned_images'" {
+	# given
+	printf '[crio.image]\npinned_images = [""]\n' > "$CRIO_CONFIG_DIR"/00-default
+	printf '[crio.image]\npinned_images = ["quay.io/crio/hello-wasm:latest"]\n' > "$CRIO_CONFIG_DIR"/01-overwrite
+	# when
+	crictl pull quay.io/crio/hello-wasm:latest
+	output=$(crictl images -o json | jq '.images[] | select(.repoTags[] == "quay.io/crio/hello-wasm:latest")  | .pinned')
+	[ "$output" == "false" ]
+	reload_crio
+	#then
+	wait_for_log 'Updated new pinned images'
+	output=$(crictl images -o json | jq '.images[] | select(.repoTags[] == "quay.io/crio/hello-wasm:latest")  | .pinned')
+	[ "$output" == "true" ]
+}