Skip to content

Commit 43fe37f

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #5562 from andfasano/iri-followup-changes
AGENT-1395: IRI followup changes
2 parents fa7eb75 + a223cc7 commit 43fe37f

6 files changed

Lines changed: 143 additions & 49 deletions

File tree

‎cmd/machine-config-controller/start.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,15 @@ func runStartCmd(_ *cobra.Command, _ []string) {
144144
ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
145145
ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
146146
ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(),
147+
ctrlctx.KubeInformerFactory.Core().V1().Secrets(),
147148
ctrlctx.ClientBuilder.KubeClientOrDie("internalreleaseimage-controller"),
148149
ctrlctx.ClientBuilder.MachineConfigClientOrDie("internalreleaseimage-controller"))
149150

150151
go iriController.Run(2, ctrlctx.Stop)
151152
// start the informers again to enable feature gated types.
152153
// see comments in SharedInformerFactory interface.
153154
ctrlctx.InformerFactory.Start(ctrlctx.Stop)
155+
ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
154156
}
155157

156158
if ctrlcommon.IsBootImageControllerRequired(ctrlctx) {

‎pkg/controller/internalreleaseimage/internalreleaseimage_controller.go‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import (
3232
templatectrl "github.com/openshift/machine-config-operator/pkg/controller/template"
3333
"github.com/openshift/machine-config-operator/pkg/osimagestream"
3434
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
35+
coreinformersv1 "k8s.io/client-go/informers/core/v1"
36+
corelistersv1 "k8s.io/client-go/listers/core/v1"
3537
)
3638

3739
const (
@@ -52,7 +54,6 @@ var (
5254
// Controller defines the InternalReleaseImage controller.
5355
type Controller struct {
5456
client mcfgclientset.Interface
55-
kubeClient clientset.Interface
5657
eventRecorder record.EventRecorder
5758

5859
syncHandler func(mcp string) error
@@ -70,6 +71,9 @@ type Controller struct {
7071
clusterVersionLister configlistersv1.ClusterVersionLister
7172
clusterVersionListerSynced cache.InformerSynced
7273

74+
secretLister corelistersv1.SecretLister
75+
secretListerSynced cache.InformerSynced
76+
7377
queue workqueue.TypedRateLimitingInterface[string]
7478
}
7579

@@ -79,6 +83,7 @@ func New(
7983
ccInformer mcfginformersv1.ControllerConfigInformer,
8084
mcInformer mcfginformersv1.MachineConfigInformer,
8185
clusterVersionInformer configinformersv1.ClusterVersionInformer,
86+
secretInformer coreinformersv1.SecretInformer,
8287
kubeClient clientset.Interface,
8388
mcfgClient mcfgclientset.Interface,
8489
) *Controller {
@@ -88,7 +93,6 @@ func New(
8893

8994
ctrl := &Controller{
9095
client: mcfgClient,
91-
kubeClient: kubeClient,
9296
eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-internalreleaseimagecontroller"})),
9397
queue: workqueue.NewTypedRateLimitingQueueWithConfig(
9498
workqueue.DefaultTypedControllerRateLimiter[string](),
@@ -113,6 +117,10 @@ func New(
113117
DeleteFunc: ctrl.deleteMachineConfig,
114118
})
115119

120+
secretInformer.Informer().AddEventHandler(cache.ResourceEventHandlerDetailedFuncs{
121+
UpdateFunc: ctrl.updateSecret,
122+
})
123+
116124
ctrl.iriLister = iriInformer.Lister()
117125
ctrl.iriListerSynced = iriInformer.Informer().HasSynced
118126

@@ -125,6 +133,9 @@ func New(
125133
ctrl.clusterVersionLister = clusterVersionInformer.Lister()
126134
ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced
127135

136+
ctrl.secretLister = secretInformer.Lister()
137+
ctrl.secretListerSynced = secretInformer.Informer().HasSynced
138+
128139
return ctrl
129140
}
130141

@@ -133,7 +144,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) {
133144
defer utilruntime.HandleCrash()
134145
defer ctrl.queue.ShutDown()
135146

136-
if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced) {
147+
if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced, ctrl.secretListerSynced) {
137148
return
138149
}
139150

@@ -263,6 +274,18 @@ func (ctrl *Controller) processMachineConfigEvent(obj interface{}, logMsg string
263274
ctrl.queue.Add(ctrlcommon.InternalReleaseImageInstanceName)
264275
}
265276

277+
func (ctrl *Controller) updateSecret(obj, _ interface{}) {
278+
secret := obj.(*corev1.Secret)
279+
280+
// Skip any event not related to the InternalReleaseImage secrets
281+
if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName {
282+
return
283+
}
284+
285+
klog.V(4).Infof("Secret %s update", secret.Name)
286+
ctrl.queue.Add(ctrlcommon.InternalReleaseImageInstanceName)
287+
}
288+
266289
func (ctrl *Controller) enqueue(iri *mcfgv1alpha1.InternalReleaseImage) {
267290
key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(iri)
268291
if err != nil {
@@ -288,7 +311,7 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
288311
}
289312

290313
// Fetch the InternalReleaseImage
291-
iri, err := ctrl.client.MachineconfigurationV1alpha1().InternalReleaseImages().Get(context.TODO(), name, metav1.GetOptions{})
314+
iri, err := ctrl.iriLister.Get(name)
292315
if errors.IsNotFound(err) {
293316
klog.V(2).Infof("InternalReleaseImage %v has been deleted", key)
294317
return nil
@@ -308,20 +331,20 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
308331
return nil
309332
}
310333

311-
cconfig, err := ctrl.client.MachineconfigurationV1().ControllerConfigs().Get(context.TODO(), ctrlcommon.ControllerConfigName, metav1.GetOptions{})
334+
cconfig, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
312335
if err != nil {
313336
return fmt.Errorf("could not get ControllerConfig %w", err)
314337
}
315338

316-
iriSecret, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{})
339+
iriSecret, err := ctrl.secretLister.Secrets(ctrlcommon.MCONamespace).Get(ctrlcommon.InternalReleaseImageTLSSecretName)
317340
if err != nil {
318341
return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err)
319342
}
320343

321344
for _, role := range SupportedRoles {
322345
r := NewRendererByRole(role, iri, iriSecret, cconfig)
323346

324-
mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), r.GetMachineConfigName(), metav1.GetOptions{})
347+
mc, err := ctrl.mcLister.Get(r.GetMachineConfigName())
325348
isNotFound := errors.IsNotFound(err)
326349
if err != nil && !isNotFound {
327350
return err // syncStatus, could not find MachineConfig

‎pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go‎

Lines changed: 74 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,21 @@ import (
55
"testing"
66
"time"
77

8+
configv1 "github.com/openshift/api/config/v1"
89
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
910
mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
1011
configinformers "github.com/openshift/client-go/config/informers/externalversions"
1112
"github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake"
12-
informers "github.com/openshift/client-go/machineconfiguration/informers/externalversions"
13+
mcfginformers "github.com/openshift/client-go/machineconfiguration/informers/externalversions"
1314
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
1415
"github.com/stretchr/testify/assert"
1516

1617
corev1 "k8s.io/api/core/v1"
1718
"k8s.io/apimachinery/pkg/api/errors"
19+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1820
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1921
"k8s.io/apimachinery/pkg/runtime"
22+
"k8s.io/client-go/informers"
2023
k8sfake "k8s.io/client-go/kubernetes/fake"
2124
"k8s.io/client-go/tools/record"
2225

@@ -40,14 +43,30 @@ func TestInternalReleaseImageCreate(t *testing.T) {
4043
},
4144
{
4245
name: "add finalizer if not present",
43-
initialObjects: objs(iri(), cconfig(), iriCertSecret()),
46+
initialObjects: objs(iri(), clusterVersion(), cconfig(), iriCertSecret()),
4447
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
45-
assert.Equal(t, iri().finalizer(masterName(), workerName()).build(), actualIRI)
48+
assert.Len(t, actualIRI.Finalizers, 2)
49+
assert.Contains(t, actualIRI.Finalizers, masterName())
50+
assert.Contains(t, actualIRI.Finalizers, workerName())
51+
},
52+
},
53+
{
54+
name: "update status if not set",
55+
initialObjects: objs(
56+
iri().finalizer(masterName(), workerName()),
57+
clusterVersion(), cconfig(), iriCertSecret()),
58+
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
59+
assert.Len(t, actualIRI.Status.Releases, 1)
60+
assert.Equal(t, actualIRI.Status.Releases[0].Name, "ocp-release-bundle-4.21.5-x86_64")
61+
assert.Equal(t, actualIRI.Status.Releases[0].Image, "ocp-4.21-release-pullspec")
62+
assert.Equal(t, actualIRI.Status.Releases[0].Conditions[0].Type, string(mcfgv1alpha1.InternalReleaseImageConditionTypeAvailable))
63+
assert.Equal(t, actualIRI.Status.Releases[0].Conditions[0].Status, metav1.ConditionTrue)
64+
assert.Equal(t, actualIRI.Status.Releases[0].Conditions[0].Message, "Release bundle is available")
4665
},
4766
},
4867
{
4968
name: "generate iri machine-config if not present",
50-
initialObjects: objs(iri(), cconfig(), iriCertSecret()),
69+
initialObjects: objs(iri(), clusterVersion(), cconfig(), iriCertSecret()),
5170
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
5271
verifyInternalReleaseMasterMachineConfig(t, actualMasterMC)
5372
verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC)
@@ -57,7 +76,7 @@ func TestInternalReleaseImageCreate(t *testing.T) {
5776
name: "avoid machine-config drifting",
5877
initialObjects: objs(
5978
iri().finalizer(masterName(), workerName()),
60-
cconfig(), iriCertSecret(),
79+
clusterVersion(), cconfig(), iriCertSecret(),
6180
machineconfigmaster().ignition("some garbage"),
6281
machineconfigworker().ignition("other garbage")),
6382
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
@@ -69,7 +88,7 @@ func TestInternalReleaseImageCreate(t *testing.T) {
6988
name: "refresh machine-config on controllerConfig update",
7089
initialObjects: objs(
7190
iri().finalizer(masterName(), workerName()),
72-
cconfig().dockerRegistryImage("a-new-docker-registry-image-pullspec"), iriCertSecret(),
91+
clusterVersion(), cconfig().dockerRegistryImage("a-new-docker-registry-image-pullspec"), iriCertSecret(),
7392
machineconfigmaster(), machineconfigworker()),
7493
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
7594
verifyInternalReleaseMasterMachineConfig(t, actualMasterMC)
@@ -80,7 +99,7 @@ func TestInternalReleaseImageCreate(t *testing.T) {
8099
name: "machine-config cascade delete on iri removal - removes the first machineconfig",
81100
initialObjects: objs(
82101
iri().finalizer(masterName(), workerName()).setDeletionTimestamp(),
83-
cconfig(), iriCertSecret(),
102+
clusterVersion(), cconfig(), iriCertSecret(),
84103
machineconfigmaster(), machineconfigworker()),
85104
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
86105
assert.NotNil(t, iri)
@@ -93,7 +112,7 @@ func TestInternalReleaseImageCreate(t *testing.T) {
93112
name: "machine-config cascade delete on iri removal - then removes the remaining machineconfig",
94113
initialObjects: objs(
95114
iri().finalizer(workerName()).setDeletionTimestamp(),
96-
cconfig(), iriCertSecret(),
115+
clusterVersion(), cconfig(), iriCertSecret(),
97116
machineconfigworker()),
98117
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
99118
assert.NotNil(t, iri)
@@ -107,17 +126,6 @@ func TestInternalReleaseImageCreate(t *testing.T) {
107126
t.Run(tc.name, func(t *testing.T) {
108127
objs := tc.initialObjects()
109128
f := newFixture(t, objs)
110-
for _, obj := range objs {
111-
switch o := obj.(type) {
112-
case *mcfgv1alpha1.InternalReleaseImage:
113-
f.iriLister = append(f.iriLister, o)
114-
case *mcfgv1.ControllerConfig:
115-
f.ccLister = append(f.ccLister, o)
116-
case *mcfgv1.MachineConfig:
117-
f.mcLister = append(f.mcLister, o)
118-
}
119-
}
120-
121129
f.run(ctrlcommon.InternalReleaseImageInstanceName)
122130

123131
if tc.verify != nil {
@@ -159,15 +167,17 @@ type fixture struct {
159167
client *fake.Clientset
160168
k8sClient *k8sfake.Clientset
161169
configClient *fakeconfigv1client.Clientset
162-
iriLister []*mcfgv1alpha1.InternalReleaseImage
163-
ccLister []*mcfgv1.ControllerConfig
164-
mcLister []*mcfgv1.MachineConfig
165-
166-
controller *Controller
167-
objects []runtime.Object
168-
k8sObjects []runtime.Object
169-
configObjects []runtime.Object
170-
configInformer configinformers.SharedInformerFactory
170+
171+
iriLister []*mcfgv1alpha1.InternalReleaseImage
172+
ccLister []*mcfgv1.ControllerConfig
173+
mcLister []*mcfgv1.MachineConfig
174+
secretLister []*corev1.Secret
175+
clusterVersionLister []*configv1.ClusterVersion
176+
177+
controller *Controller
178+
objects []runtime.Object
179+
k8sObjects []runtime.Object
180+
configObjects []runtime.Object
171181
}
172182

173183
func newFixture(t *testing.T, objects []runtime.Object) *fixture {
@@ -178,12 +188,26 @@ func newFixture(t *testing.T, objects []runtime.Object) *fixture {
178188
}
179189

180190
func (f *fixture) setupObjects(objs []runtime.Object) {
181-
for _, o := range objs {
182-
switch o.(type) {
191+
for _, obj := range objs {
192+
switch obj.(type) {
183193
case *corev1.Secret, *corev1.ConfigMap, *corev1.Pod:
184-
f.k8sObjects = append(f.k8sObjects, o)
194+
f.k8sObjects = append(f.k8sObjects, obj)
195+
switch o := obj.(type) {
196+
case *corev1.Secret:
197+
f.secretLister = append(f.secretLister, o)
198+
}
199+
case *configv1.ClusterVersion:
200+
f.configObjects = append(f.configObjects, obj)
185201
default:
186-
f.objects = append(f.objects, o)
202+
f.objects = append(f.objects, obj)
203+
switch o := obj.(type) {
204+
case *mcfgv1alpha1.InternalReleaseImage:
205+
f.iriLister = append(f.iriLister, o)
206+
case *mcfgv1.ControllerConfig:
207+
f.ccLister = append(f.ccLister, o)
208+
case *mcfgv1.MachineConfig:
209+
f.mcLister = append(f.mcLister, o)
210+
}
187211
}
188212
}
189213
}
@@ -192,14 +216,17 @@ func (f *fixture) newController() *Controller {
192216
f.client = fake.NewSimpleClientset(f.objects...)
193217
f.k8sClient = k8sfake.NewSimpleClientset(f.k8sObjects...)
194218
f.configClient = fakeconfigv1client.NewSimpleClientset(f.configObjects...)
195-
i := informers.NewSharedInformerFactory(f.client, func() time.Duration { return 0 }())
196-
f.configInformer = configinformers.NewSharedInformerFactory(f.configClient, func() time.Duration { return 0 }())
219+
220+
i := mcfginformers.NewSharedInformerFactory(f.client, func() time.Duration { return 0 }())
221+
k := informers.NewSharedInformerFactory(f.k8sClient, func() time.Duration { return 0 }())
222+
ci := configinformers.NewSharedInformerFactory(f.configClient, func() time.Duration { return 0 }())
197223

198224
c := New(
199225
i.Machineconfiguration().V1alpha1().InternalReleaseImages(),
200226
i.Machineconfiguration().V1().ControllerConfigs(),
201227
i.Machineconfiguration().V1().MachineConfigs(),
202-
f.configInformer.Config().V1().ClusterVersions(),
228+
ci.Config().V1().ClusterVersions(),
229+
k.Core().V1().Secrets(),
203230
f.k8sClient,
204231
f.client,
205232
)
@@ -209,14 +236,18 @@ func (f *fixture) newController() *Controller {
209236
c.ccListerSynced = alwaysReady
210237
c.mcListerSynced = alwaysReady
211238
c.clusterVersionListerSynced = alwaysReady
239+
c.secretListerSynced = alwaysReady
212240
c.eventRecorder = &record.FakeRecorder{}
213241

214242
stopCh := make(chan struct{})
215243
defer close(stopCh)
244+
216245
i.Start(stopCh)
217246
i.WaitForCacheSync(stopCh)
218-
f.configInformer.Start(stopCh)
219-
f.configInformer.WaitForCacheSync(stopCh)
247+
k.Start(stopCh)
248+
k.WaitForCacheSync(stopCh)
249+
ci.Start(stopCh)
250+
ci.WaitForCacheSync(stopCh)
220251

221252
for _, c := range f.iriLister {
222253
i.Machineconfiguration().V1alpha1().InternalReleaseImages().Informer().GetIndexer().Add(c)
@@ -227,6 +258,12 @@ func (f *fixture) newController() *Controller {
227258
for _, c := range f.mcLister {
228259
i.Machineconfiguration().V1().MachineConfigs().Informer().GetIndexer().Add(c)
229260
}
261+
for _, c := range f.secretLister {
262+
k.Core().V1().Secrets().Informer().GetIndexer().Add(c)
263+
}
264+
for _, c := range f.clusterVersionLister {
265+
ci.Config().V1().ClusterVersions().Informer().GetIndexer().Add(c)
266+
}
230267

231268
return c
232269
}

0 commit comments

Comments
 (0)