Skip to content

Commit

Permalink
Invalidate imageCache if the pinned image from config exists
Browse files Browse the repository at this point in the history
Signed-off-by: roman-kiselenko <roman.kiselenko.dev@gmail.com>
  • Loading branch information
roman-kiselenko committed Apr 10, 2024
1 parent 44b7322 commit c683299
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 25 deletions.
24 changes: 24 additions & 0 deletions internal/storage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ 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)

// UpdateImageCache invalidate imageCache if the pinned image from config exists;
// this method doesn't change the Pinned attribute, only removes the image item.
UpdateImageCache(configPinnedImage []string) error
}

func parseImageNames(image *storage.Image) (someName *RegistryImageReference, tags []reference.NamedTagged, digests []reference.Canonical, err error) {
Expand Down Expand Up @@ -897,6 +901,26 @@ func (st nativeStorageTransport) ResolveReference(ref types.ImageReference) (typ
return istorage.ResolveReference(ref)
}

// UpdateImageCache invalidate imageCache if the pinned image from config exists;
// this method doesn't change the Pinned attribute, only removes the image item.
func (svc *imageService) UpdateImageCache(configPinnedImage []string) error {
images, err := svc.store.Images()
if err != nil {
return err
}
for i := range images {
img := &images[i]
for _, name := range img.Names {
if FilterPinnedImage(name, CompileRegexpsForPinnedImages(configPinnedImage)) {
svc.imageCacheLock.Lock()
delete(svc.imageCache, img.ID)
svc.imageCacheLock.Unlock()
}
}
}
return nil
}

// 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 {
Expand Down
29 changes: 14 additions & 15 deletions pkg/config/reload.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,8 @@ import (

// Reload reloads the configuration for the single crio.conf and the drop-in
// configuration directory.
func (c *Config) Reload() error {
func (c *Config) Reload(newConfig *Config) error {
logrus.Infof("Reloading configuration")

// Reload the config
newConfig, err := DefaultConfig()
if err != nil {
return errors.New("unable to create default config")
}

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 {
Expand Down Expand Up @@ -51,7 +44,6 @@ func (c *Config) Reload() error {
if err := c.ReloadPauseImage(newConfig); err != nil {
return err
}
c.ReloadPinnedImages(newConfig)
if err := c.ReloadRegistries(); err != nil {
return err
}
Expand Down Expand Up @@ -143,17 +135,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
Expand Down
18 changes: 12 additions & 6 deletions pkg/config/reload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ var _ = t.Describe("Config", func() {
Expect(sut.UpdateFromFile(filePath)).To(Succeed())

// When
err := sut.Reload()
c, _ := config.DefaultConfig()

Check failure on line 38 in pkg/config/reload_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `config.DefaultConfig` is not checked (errcheck)
err := sut.Reload(c)

// Then
Expect(err).ToNot(HaveOccurred())
Expand All @@ -49,7 +50,8 @@ var _ = t.Describe("Config", func() {
)

// When
err := sut.Reload()
c, _ := config.DefaultConfig()

Check failure on line 53 in pkg/config/reload_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `config.DefaultConfig` is not checked (errcheck)
err := sut.Reload(c)

// Then
Expect(err).To(HaveOccurred())
Expand All @@ -63,7 +65,8 @@ var _ = t.Describe("Config", func() {
)

// When
err := sut.Reload()
c, _ := config.DefaultConfig()

Check failure on line 68 in pkg/config/reload_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `config.DefaultConfig` is not checked (errcheck)
err := sut.Reload(c)

// Then
Expect(err).To(HaveOccurred())
Expand All @@ -77,7 +80,8 @@ var _ = t.Describe("Config", func() {
)

// When
err := sut.Reload()
c, _ := config.DefaultConfig()
err := sut.Reload(c)

// Then
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -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"}))
})
})
Expand Down
7 changes: 4 additions & 3 deletions pkg/config/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ func TestLibConfig(t *testing.T) {
}

var (
t *TestFramework
sut *config.Config
validDirPath string
t *TestFramework
sut *config.Config
updateCacheImageFunc func([]string) error

Check failure on line 23 in pkg/config/suite_test.go

View workflow job for this annotation

GitHub Actions / lint

var `updateCacheImageFunc` is unused (unused)
validDirPath string
)

const (
Expand Down
13 changes: 12 additions & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,10 +573,21 @@ func (s *Server) startReloadWatcher(ctx context.Context) {
for {
// Block until the signal is received
<-ch
if err := s.config.Reload(); err != nil {
newConfig, err := libconfig.DefaultConfig()
if err != nil {
logrus.Error("unable to create default config")
continue
}
if err := s.config.Reload(newConfig); err != nil {
logrus.Errorf("Unable to reload configuration: %v", err)
continue
}
if s.config.ReloadPinnedImages(newConfig) {
if err := s.StorageImageServer().UpdateImageCache(s.config.PinnedImages); err != nil {
logrus.Errorf("Unable to update pinned images: %v", err)
continue
}
}
}
}()

Expand Down
14 changes: 14 additions & 0 deletions test/mocks/criostorage/criostorage.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions test/reload_config.bats
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,17 @@ EOF
#then
wait_for_log '"updating runtime configuration"'
}

@test "reload config should update 'pinned_images'" {
# given
cat << EOF > "$CRIO_CONFIG_DIR/99-pinned-image.conf"
[crio.image]
pinned_images = [ "quay.io/crio/hello-wasm:latest" ]
EOF

# when
reload_crio

#then
wait_for_log 'Updated new pinned images'
}

0 comments on commit c683299

Please sign in to comment.