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 9, 2024
1 parent f63de87 commit 92537ef
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 14 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
13 changes: 9 additions & 4 deletions pkg/config/reload.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
)

// Reload reloads the configuration for the single crio.conf and the drop-in
// configuration directory.
func (c *Config) Reload() error {
// configuration directory. Method takes a function to update PinnedImages as parameter.
func (c *Config) Reload(updateImageCache func([]string) error) error {
logrus.Infof("Reloading configuration")

// Reload the config
Expand Down Expand Up @@ -51,7 +51,11 @@ func (c *Config) Reload() error {
if err := c.ReloadPauseImage(newConfig); err != nil {
return err
}
c.ReloadPinnedImages(newConfig)
if c.ReloadPinnedImages(newConfig) {
if err := updateImageCache(c.PinnedImages); err != nil {
return err
}
}
if err := c.ReloadRegistries(); err != nil {
return err
}
Expand Down Expand Up @@ -143,7 +147,7 @@ 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))
for i, image := range newConfig.PinnedImages {
if i < len(c.PinnedImages) && image == c.PinnedImages[i] {
Expand All @@ -154,6 +158,7 @@ func (c *Config) ReloadPinnedImages(newConfig *Config) {
}
logrus.Infof("Updated new pinned images: %+v", updatedPinnedImages)
c.PinnedImages = updatedPinnedImages
return len(updatedPinnedImages) > 0
}

// ReloadRegistries reloads the registry configuration from the Configs
Expand Down
14 changes: 8 additions & 6 deletions pkg/config/reload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = t.Describe("Config", func() {
Expect(sut.UpdateFromFile(filePath)).To(Succeed())

// When
err := sut.Reload()
err := sut.Reload(updateCacheImageFunc)

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

// When
err := sut.Reload()
err := sut.Reload(updateCacheImageFunc)

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

// When
err := sut.Reload()
err := sut.Reload(updateCacheImageFunc)

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

// When
err := sut.Reload()
err := sut.Reload(updateCacheImageFunc)

// Then
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -427,15 +427,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
10 changes: 7 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
validDirPath string
)

const (
Expand Down Expand Up @@ -50,6 +51,9 @@ var _ = AfterSuite(func() {

func beforeEach() {
sut = defaultConfig()
updateCacheImageFunc = func([]string) error {
return nil
}
}

func defaultConfig() *config.Config {
Expand Down
2 changes: 1 addition & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ func (s *Server) startReloadWatcher(ctx context.Context) {
for {
// Block until the signal is received
<-ch
if err := s.config.Reload(); err != nil {
if err := s.config.Reload(s.StorageImageServer().UpdateImageCache); err != nil {
logrus.Errorf("Unable to reload configuration: %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.

0 comments on commit 92537ef

Please sign in to comment.