Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 52 additions & 61 deletions cmd/machine-config-controller/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,28 +92,32 @@ func runStartCmd(_ *cobra.Command, _ []string) {
return
}

var inspectorFactory osimagestream.ImagesInspectorFactory
var inspectionCache *imageutils.FileInspectionCache
if startOpts.streamsCache != "" {
inspectionCache = imageutils.NewFileInspectionCache(path.Join(startOpts.streamsCache, "image-inspection.json"), 48*time.Hour)
}
syncer := imageutils.NewConfigMapCacheSyncer(
ctrlctx.KubeNamespacedInformerFactory.Core().V1().ConfigMaps(),
ctrlctx.ClientBuilder.KubeClientOrDie("inspection-cache-syncer"),
ctrlcommon.MCONamespace,
ctrlcommon.InspectionCacheConfigMapName,
osimagestream.NewCacheEntryFilter(),
osimagestream.NewImageStreamFileTransformer(),
)
inspectionCache := imageutils.NewFileInspectionCache(
path.Join(startOpts.streamsCache, "image-inspection.json"), 48*time.Hour, syncer,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// OSImageStream must be the first controller to run: the blocking
// EnsureOSImageStream call guarantees the CR exists before any other
// controller starts, since render, node, and template depend on it
// for OS image URLs.
var inspectorFactory osimagestream.ImagesInspectorFactory
var cacheWarmer *pinnedimageset.CacheWarmer
var osImageStreamCtrl *osistreamctrl.Controller
if osimagestream.IsFeatureEnabled(ctrlctx.FeatureGatesHandler) {
if inspectionCache != nil {
inspectorFactory = osimagestream.NewCachedImagesInspectorFactory(
&osimagestream.DefaultImagesInspectorFactory{},
inspectionCache,
)
} else {
inspectorFactory = &osimagestream.DefaultImagesInspectorFactory{}
}
inspectorFactory = osimagestream.NewCachedImagesInspectorFactory(
&osimagestream.DefaultImagesInspectorFactory{},
inspectionCache,
)

osImageStreamCtrl := osistreamctrl.New(
osImageStreamCtrl = osistreamctrl.New(
ctrlctx.InformerFactory.Machineconfiguration().V1().OSImageStreams(),
ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(),
Expand All @@ -128,41 +132,26 @@ func runStartCmd(_ *cobra.Command, _ []string) {
ctrlctx.FeatureGatesHandler,
inspectorFactory,
)
inspectionCache.RegisterEvicter(osImageStreamCtrl)

ctrlctx.InformerFactory.Start(ctx.Done())
ctrlctx.ConfigInformerFactory.Start(ctx.Done())
ctrlctx.OpenShiftConfigKubeNamespacedInformerFactory.Start(ctx.Done())
ctrlctx.KubeNamespacedInformerFactory.Start(ctx.Done())
ctrlctx.OperatorInformerFactory.Start(ctx.Done())

go osImageStreamCtrl.Run(ctx, 1)

if err := osImageStreamCtrl.EnsureOSImageStream(ctx); err != nil {
klog.Fatalf("Failed to ensure OSImageStream: %v", err)
}

if inspectionCache != nil {
inspectionCache.RegisterEvicter(osImageStreamCtrl)

cacheWarmer = pinnedimageset.NewCacheWarmer(
ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets().Lister(),
inspectorFactory,
ctrlcommon.NewSysContextFactory(
ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs().Lister(),
ctrlctx.OpenShiftConfigKubeNamespacedInformerFactory.Core().V1().Secrets().Lister(),
ctrlctx.ConfigInformerFactory.Config().V1().Images().Lister(),
ctrlctx.OperatorInformerFactory.Operator().V1alpha1().ImageContentSourcePolicies().Lister(),
ctrlctx.ConfigInformerFactory.Config().V1().ImageDigestMirrorSets().Lister(),
ctrlctx.ConfigInformerFactory.Config().V1().ImageTagMirrorSets().Lister(),
),
)
inspectionCache.RegisterEvicter(cacheWarmer)
}
cacheWarmer = pinnedimageset.NewCacheWarmer(
ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets().Lister(),
inspectorFactory,
ctrlcommon.NewSysContextFactory(
ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs().Lister(),
ctrlctx.OpenShiftConfigKubeNamespacedInformerFactory.Core().V1().Secrets().Lister(),
ctrlctx.ConfigInformerFactory.Config().V1().Images().Lister(),
ctrlctx.OperatorInformerFactory.Operator().V1alpha1().ImageContentSourcePolicies().Lister(),
ctrlctx.ConfigInformerFactory.Config().V1().ImageDigestMirrorSets().Lister(),
ctrlctx.ConfigInformerFactory.Config().V1().ImageTagMirrorSets().Lister(),
),
)
inspectionCache.RegisterEvicter(cacheWarmer)
}

go ctrlcommon.StartMetricsListener(startOpts.promMetricsListenAddress, ctx.Done(), ctrlcommon.RegisterMCCMetrics, startOpts.tlsMinVersion, startOpts.tlsCipherSuites)

controllers := createControllers(ctrlctx, inspectionCache, inspectorFactory)
controllers := createControllers(ctrlctx, inspectionCache, inspectorFactory, cacheWarmer)
draincontroller := drain.New(
drain.DefaultConfig(),
ctrlctx.KubeInformerFactory.Core().V1().Nodes(),
Expand All @@ -189,15 +178,6 @@ func runStartCmd(_ *cobra.Command, _ []string) {
klog.Fatalf("unable to start cert rotation controller: %v", err)
}

pinnedImageSet := pinnedimageset.New(
ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(),
ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
ctrlctx.ClientBuilder.KubeClientOrDie("pinned-image-set-controller"),
ctrlctx.ClientBuilder.MachineConfigClientOrDie("pinned-image-set-controller"),
cacheWarmer,
)
go pinnedImageSet.Run(ctx, 2)

// Start the shared factory informers that you need to use in your controller
ctrlctx.InformerFactory.Start(ctrlctx.Stop)
ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
Expand All @@ -211,6 +191,16 @@ func runStartCmd(_ *cobra.Command, _ []string) {

close(ctrlctx.InformersStarted)

// Start the cache before any controller that consumes it has a chance to run.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PinnedImageSet controller seems to be the one controller that starts before this. Just to check, this PR seems targetted for that use case, should this start before the PIS controller does?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed.

inspectionCache.Start(ctx, 24*time.Hour, 10*time.Minute, 30*time.Second)

if osImageStreamCtrl != nil {
go osImageStreamCtrl.Run(ctx, 1)
if err := osImageStreamCtrl.EnsureOSImageStream(ctx); err != nil {
klog.Fatalf("Failed to ensure OSImageStream: %v", err)
}
}

if ctrlcommon.IsBootImageControllerRequired(ctrlctx) {
bootImageController := bootimagecontroller.New(
ctrlctx.ClientBuilder.KubeClientOrDie("machine-set-boot-image-controller"),
Expand Down Expand Up @@ -239,10 +229,6 @@ func runStartCmd(_ *cobra.Command, _ []string) {
go draincontroller.Run(ctx, 5)
go certrotationcontroller.Run(ctx, 1)

if inspectionCache != nil {
inspectionCache.StartEviction(ctx, 24*time.Hour, 10*time.Minute)
}

// wait here in this function until the context gets cancelled (which tells us when we are being shut down)
<-ctx.Done()
}
Expand All @@ -266,7 +252,7 @@ func runStartCmd(_ *cobra.Command, _ []string) {
panic("unreachable")
}

func createControllers(ctx *ctrlcommon.ControllerContext, inspectionCache *imageutils.FileInspectionCache, inspectorFactory osimagestream.ImagesInspectorFactory) []ctrlcommon.Controller {
func createControllers(ctx *ctrlcommon.ControllerContext, inspectionCache *imageutils.FileInspectionCache, inspectorFactory osimagestream.ImagesInspectorFactory, pisCacheWarmer *pinnedimageset.CacheWarmer) []ctrlcommon.Controller {
renderCtrl := render.New(
ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
ctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
Expand All @@ -285,9 +271,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext, inspectionCache *image
ctx.FeatureGatesHandler,
inspectorFactory,
)
if inspectionCache != nil {
inspectionCache.RegisterEvicter(renderCtrl)
}
inspectionCache.RegisterEvicter(renderCtrl)

var controllers []ctrlcommon.Controller
controllers = append(controllers,
Expand Down Expand Up @@ -363,6 +347,13 @@ func createControllers(ctx *ctrlcommon.ControllerContext, inspectionCache *image
ctx.ClientBuilder.KubeClientOrDie("internalreleaseimage-controller"),
ctx.ClientBuilder.MachineConfigClientOrDie("internalreleaseimage-controller"),
),
pinnedimageset.New(
ctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(),
ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
ctx.ClientBuilder.KubeClientOrDie("pinned-image-set-controller"),
ctx.ClientBuilder.MachineConfigClientOrDie("pinned-image-set-controller"),
pisCacheWarmer,
),
)

return controllers
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/common/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,6 @@ const (
MachineConfigOperatorImagesConfigMapName string = "machine-config-operator-images"
// The name of the machine-config-osimageurl ConfigMap.
MachineConfigOSImageURLConfigMapName string = "machine-config-osimageurl"
// The name of the ConfigMap used to persist the image inspection cache across Pod restarts.
InspectionCacheConfigMapName string = "machine-config-image-inspection-cache"
)
2 changes: 1 addition & 1 deletion pkg/controller/pinnedimageset/cache_warmer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestCacheWarmerWarmsOnPISChange(t *testing.T) {
defer cancel()

cache := imageutils.NewFileInspectionCache(
filepath.Join(t.TempDir(), "cache.json"), 48*time.Hour)
filepath.Join(t.TempDir(), "cache.json"), 48*time.Hour, nil)

inspector := &fakeInspector{
inspectData: map[string]*types.ImageInspectInfo{
Expand Down
51 changes: 51 additions & 0 deletions pkg/imageutils/cache_entry_transformer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package imageutils

// CacheEntryFilter decides whether a cache entry should be included in
// external persistence. Implementations must not mutate the input entry.
type CacheEntryFilter func(digest string, entry *InspectionCacheEntry) bool

// CacheEntryTransformer returns a (possibly reduced) copy of an
// InspectionCacheEntry for external persistence. Implementations must not
// mutate the input entry.
type CacheEntryTransformer func(digest string, entry *InspectionCacheEntry) *InspectionCacheEntry

// NewCacheFileTransformer returns a CacheEntryTransformer that applies a
// transformation function to a cached file matching the given path. Other
// files and labels are preserved.
func NewCacheFileTransformer(path string, transform func([]byte) ([]byte, error)) CacheEntryTransformer {
return func(_ string, entry *InspectionCacheEntry) *InspectionCacheEntry {
if entry.Files == nil {
return entry
}
_, ok := entry.Files[path]
if !ok {
return entry
}

cp := entry.DeepCopy()
transformed, err := transform(cp.Files[path])
if err != nil {
return entry
}

cp.Files[path] = transformed
return cp
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// NewCacheEntryFilter returns a filter that accepts entries having at least
// one of the specified label keys.
func NewCacheEntryFilter(requiredLabelKeys ...string) CacheEntryFilter {
keys := make(map[string]struct{}, len(requiredLabelKeys))
for _, k := range requiredLabelKeys {
keys[k] = struct{}{}
}
return func(_ string, entry *InspectionCacheEntry) bool {
for k := range entry.Labels {
if _, ok := keys[k]; ok {
return true
}
}
return false
}
}
40 changes: 40 additions & 0 deletions pkg/imageutils/cache_entry_transformer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package imageutils

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestEntryFilter_KeepsEntryWithMatchingLabel(t *testing.T) {
filter := NewCacheEntryFilter("io.openshift.os.streamclass", "io.openshift.release")

entry := &InspectionCacheEntry{
Labels: map[string]string{
"io.openshift.os.streamclass": "rhel-9",
"vendor": "Red Hat",
},
}

assert.True(t, filter("sha256:aaa", entry))
}

func TestEntryFilter_ExcludesEntryWithoutMatchingLabels(t *testing.T) {
filter := NewCacheEntryFilter("io.openshift.os.streamclass", "io.openshift.release")

entry := &InspectionCacheEntry{
Labels: map[string]string{
"vendor": "Red Hat",
"version": "9.4",
},
}

assert.False(t, filter("sha256:aaa", entry))
}

func TestEntryFilter_EmptyLabels(t *testing.T) {
filter := NewCacheEntryFilter("io.openshift.os.streamclass")

assert.False(t, filter("sha256:aaa", &InspectionCacheEntry{Labels: nil}))
assert.False(t, filter("sha256:aaa", &InspectionCacheEntry{Labels: map[string]string{}}))
}
Loading