Merge pull request cri-o#8213 from roman-kiselenko/bugfix/config-relo…

…ad-pinned-images

Reload config should remove pinned images when an empty list is provided

contrib/test/ci/integration.yml

@@ -56,6 +56,7 @@
  + ['command.bats'] | product(kata_skip_command_tests) \
  + ['config.bats'] | product(kata_skip_config_tests) \
  + ['config_migrate.bats'] | product(kata_skip_config_migrate_tests) \
+ + ['reload_config.bats'] | product(kata_skip_reload_config) \
  + ['crio-wipe.bats'] | product(kata_skip_crio_wipe_tests) \
  + ['ctr.bats'] | product(kata_skip_ctr_tests) \
  + ['devices.bats'] | product(kata_skip_devices_tests) \

contrib/test/ci/vars.yml

@@ -106,6 +106,8 @@ kata_skip_config_tests:
 kata_skip_config_migrate_tests:
  - 'test "config migrate should succeed with default config"'
  - 'test "config migrate should succeed with 1.17 config"'
+kata_skip_reload_config:
+ - 'test "reload config should remove pinned images when an empty list is provided"'
 kata_skip_crio_wipe_tests:
  - 'test "clear neither when remove persist"'
  - "test \"don't clear containers on a forced restart of crio\""

go.mod

@@ -39,6 +39,7 @@ require (
  github.com/godbus/dbus/v5 v5.1.1-0.20230522191255-76236955d466
  github.com/gogo/protobuf v1.3.2
  github.com/golang/mock v1.6.0
+ github.com/google/go-cmp v0.6.0
  github.com/google/renameio v1.0.1
  github.com/google/uuid v1.6.0
  github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
@@ -138,7 +139,6 @@ require (
  github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
  github.com/golang/protobuf v1.5.4 // indirect
  github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
- github.com/google/go-cmp v0.6.0 // indirect
  github.com/google/go-containerregistry v0.19.1 // indirect
  github.com/google/go-github/v60 v60.0.0 // indirect
  github.com/google/go-intervals v0.0.2 // indirect

pkg/config/reload.go

@@ -9,6 +9,8 @@ import (
 
  "github.com/containers/image/v5/pkg/sysregistriesv2"
  "github.com/cri-o/cri-o/internal/log"
+ "github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
  "github.com/sirupsen/logrus"
  "tags.cncf.io/container-device-interface/pkg/cdi"
 )
@@ -145,24 +147,34 @@ func (c *Config) ReloadPauseImage(newConfig *Config) error {
  return nil
 }
 
-// ReloadPinnedImages updates the PinnedImages with the provided `newConfig`.
-// The method print log in case of any updates.
+// ReloadPinnedImages replace the PinnedImages
+// with the provided `newConfig.PinnedImages`.
+// The method skips empty items and prints a log message.
 func (c *Config) ReloadPinnedImages(newConfig *Config) {
- updatedPinnedImages := make([]string, len(newConfig.PinnedImages))
- updatedPinnedImageList := false
+ if len(newConfig.PinnedImages) == 0 {
+ c.PinnedImages = []string{}
+ logConfig("pinned_images", "[]")
+ return
+ }
 
- for i, image := range newConfig.PinnedImages {
- if i < len(c.PinnedImages) && image == c.PinnedImages[i] {
- continue
- }
-  updatedPinnedImages[i] = image
- updatedPinnedImageList = true
+ if cmp.Equal(c.PinnedImages, newConfig.PinnedImages,
+ cmpopts.SortSlices(func(a, b string) bool {
+ return a < b
+ }),
+ ) {
+ return
  }
 
- if updatedPinnedImageList {
- logConfig("pinned_images", strings.Join(updatedPinnedImages, ", "))
- c.PinnedImages = updatedPinnedImages
+ pinnedImages := []string{}
+ for _, img := range newConfig.PinnedImages {
+ if img != "" {
+ pinnedImages = append(pinnedImages, img)
+ }
  }
+
+ logConfig("pinned_images", strings.Join(pinnedImages, ","))
+
+ c.PinnedImages = pinnedImages
 }
 
 // ReloadRegistries reloads the registry configuration from the Configs

test/reload_config.bats

@@ -269,3 +269,29 @@ EOF
  # pause image is pinned
  [[ "$output" == *"fedora-crio-ci"* ]]
 }
+
+@test "reload config should remove pinned images when an empty list is provided" {
+ EXAMPLE_IMAGE=quay.io/crio/fedora-crio-ci:latest
+
+ # Add a pinned image to the configuration
+ printf '[crio.image]\npinned_images = ["%s", ""]\n' $EXAMPLE_IMAGE > "$CRIO_CONFIG_DIR"/01-overwrite
+ reload_crio
+ wait_for_log 'Set config pinned_images to \\"quay.io/crio/fedora-crio-ci:latest\\"'
+ wait_for_log "Configuration reload completed"
+ wait_for_log 'pinned_images = \[\\"quay.io/crio/fedora-crio-ci:latest\\"\]'
+
+ # Verify that the image is pinned
+ output=$(crictl images -o json | jq ".images[] | select(.repoTags[] == \"$EXAMPLE_IMAGE\") |.pinned")
+ [ "$output" == "true" ]
+
+ # Remove the pinned image from the configuration
+ printf '[crio.image]\npinned_images = []\n' > "$CRIO_CONFIG_DIR"/01-overwrite
+ reload_crio
+ wait_for_log 'Set config pinned_images to \\"\[\]\\"'
+ wait_for_log "Configuration reload completed"
+ wait_for_log 'pinned_images = \[\]'
+
+ # Verify that the image is no longer pinned
+ output=$(crictl images -o json | jq ".images[] | select(.repoTags[] == \"$EXAMPLE_IMAGE\") |.pinned")
+ [ "$output" == "false" ]
+}