Skip to content

Commit 66a0fed

Browse files
bfourniebfournie
committed
AGENT-1366: Report InternalReleaseImageController errors in IRI status
Implemented status condition reporting for the InternalReleaseImageController to report controller errors/issues in the InternalReleaseImage (IRI) status.
1 parent f86723e commit 66a0fed

File tree

2 files changed

+164
-8
lines changed

2 files changed

+164
-8
lines changed

‎pkg/controller/internalreleaseimage/internalreleaseimage_controller.go‎

Lines changed: 91 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
corev1 "k8s.io/api/core/v1"
1010
"k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/api/meta"
1112
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1213
"k8s.io/apimachinery/pkg/util/wait"
1314
clientset "k8s.io/client-go/kubernetes"
@@ -331,14 +332,25 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
331332
return nil
332333
}
333334

335+
// Track sync error to update status at the end
336+
var syncErr error
337+
334338
cconfig, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
335339
if err != nil {
336-
return fmt.Errorf("could not get ControllerConfig %w", err)
340+
syncErr = fmt.Errorf("could not get ControllerConfig: %w", err)
341+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
342+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
343+
}
344+
return syncErr
337345
}
338346

339347
iriSecret, err := ctrl.secretLister.Secrets(ctrlcommon.MCONamespace).Get(ctrlcommon.InternalReleaseImageTLSSecretName)
340348
if err != nil {
341-
return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err)
349+
syncErr = fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err)
350+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
351+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
352+
}
353+
return syncErr
342354
}
343355

344356
for _, role := range SupportedRoles {
@@ -347,31 +359,60 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
347359
mc, err := ctrl.mcLister.Get(r.GetMachineConfigName())
348360
isNotFound := errors.IsNotFound(err)
349361
if err != nil && !isNotFound {
350-
return err // syncStatus, could not find MachineConfig
362+
syncErr = fmt.Errorf("could not get MachineConfig: %w", err)
363+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
364+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
365+
}
366+
return syncErr
351367
}
352368
if isNotFound {
353369
mc, err = r.CreateEmptyMachineConfig()
354370
if err != nil {
355-
return err // syncStatusOnly, could not create MachineConfig
371+
syncErr = fmt.Errorf("could not create MachineConfig: %w", err)
372+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
373+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
374+
}
375+
return syncErr
356376
}
357377
}
358378

359379
err = r.RenderAndSetIgnition(mc)
360380
if err != nil {
361-
return err // syncStatus, could not generate IRI configs
381+
syncErr = fmt.Errorf("could not generate IRI configs: %w", err)
382+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
383+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
384+
}
385+
return syncErr
362386
}
363387
err = ctrl.createOrUpdateMachineConfig(isNotFound, mc)
364388
if err != nil {
365-
return err // syncStatus, could not Create/Update MachineConfig
389+
syncErr = fmt.Errorf("could not create/update MachineConfig: %w", err)
390+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
391+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
392+
}
393+
return syncErr
366394
}
367395
if err := ctrl.addFinalizerToInternalReleaseImage(iri, mc); err != nil {
368-
return err // syncStatus , could not add finalizers
396+
syncErr = fmt.Errorf("could not add finalizer: %w", err)
397+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
398+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
399+
}
400+
return syncErr
369401
}
370402
}
371403

372404
// Initialize status if empty
373405
if err := ctrl.initializeInternalReleaseImageStatus(iri); err != nil {
374-
return err
406+
syncErr = fmt.Errorf("could not initialize status: %w", err)
407+
if statusErr := ctrl.updateInternalReleaseImageStatus(iri, syncErr); statusErr != nil {
408+
klog.Warningf("Error updating InternalReleaseImage status: %v", statusErr)
409+
}
410+
return syncErr
411+
}
412+
413+
// Update status to indicate successful sync (Degraded=False)
414+
if err := ctrl.updateInternalReleaseImageStatus(iri, nil); err != nil {
415+
klog.Warningf("Error updating InternalReleaseImage status: %v", err)
375416
}
376417

377418
return nil
@@ -431,6 +472,48 @@ func (ctrl *Controller) initializeInternalReleaseImageStatus(iri *mcfgv1alpha1.I
431472
return nil
432473
}
433474

475+
// updateInternalReleaseImageStatus updates the InternalReleaseImage status conditions
476+
// based on the provided error. If err is nil, it sets Degraded=False, otherwise Degraded=True.
477+
func (ctrl *Controller) updateInternalReleaseImageStatus(iri *mcfgv1alpha1.InternalReleaseImage, err error) error {
478+
return retry.RetryOnConflict(updateBackoff, func() error {
479+
// Get the latest version of the IRI to avoid conflicts
480+
latestIRI, getErr := ctrl.iriLister.Get(iri.Name)
481+
if getErr != nil {
482+
return getErr
483+
}
484+
newIRI := latestIRI.DeepCopy()
485+
486+
// Prepare the condition based on error state
487+
var condition metav1.Condition
488+
if err != nil {
489+
// Set Degraded=True when there's an error
490+
condition = metav1.Condition{
491+
Type: string(mcfgv1alpha1.InternalReleaseImageStatusConditionTypeDegraded),
492+
Status: metav1.ConditionTrue,
493+
Reason: "SyncError",
494+
Message: fmt.Sprintf("Error syncing InternalReleaseImage: %v", err),
495+
ObservedGeneration: newIRI.Generation,
496+
}
497+
} else {
498+
// Set Degraded=False when sync is successful
499+
condition = metav1.Condition{
500+
Type: string(mcfgv1alpha1.InternalReleaseImageStatusConditionTypeDegraded),
501+
Status: metav1.ConditionFalse,
502+
Reason: "AsExpected",
503+
Message: "InternalReleaseImage controller sync successful",
504+
ObservedGeneration: newIRI.Generation,
505+
}
506+
}
507+
508+
// Update the condition
509+
meta.SetStatusCondition(&newIRI.Status.Conditions, condition)
510+
511+
// Update the status subresource
512+
_, updateErr := ctrl.client.MachineconfigurationV1alpha1().InternalReleaseImages().UpdateStatus(context.TODO(), newIRI, metav1.UpdateOptions{})
513+
return updateErr
514+
})
515+
}
516+
434517
func (ctrl *Controller) createOrUpdateMachineConfig(isNotFound bool, mc *mcfgv1.MachineConfig) error {
435518
return retry.RetryOnConflict(updateBackoff, func() error {
436519
var err error

‎pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,21 @@ func TestInternalReleaseImageCreate(t *testing.T) {
121121
assert.Nil(t, actualWorkerMC)
122122
},
123123
},
124+
{
125+
name: "status condition Degraded=False on successful sync",
126+
initialObjects: objs(
127+
iri().finalizer(masterName(), workerName()),
128+
clusterVersion(), cconfig(), iriCertSecret(),
129+
machineconfigmaster(), machineconfigworker()),
130+
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) {
131+
assert.NotNil(t, actualIRI)
132+
assert.Len(t, actualIRI.Status.Conditions, 1)
133+
assert.Equal(t, string(mcfgv1alpha1.InternalReleaseImageStatusConditionTypeDegraded), actualIRI.Status.Conditions[0].Type)
134+
assert.Equal(t, metav1.ConditionFalse, actualIRI.Status.Conditions[0].Status)
135+
assert.Equal(t, "AsExpected", actualIRI.Status.Conditions[0].Reason)
136+
assert.Equal(t, "InternalReleaseImage controller sync successful", actualIRI.Status.Conditions[0].Message)
137+
},
138+
},
124139
}
125140
for _, tc := range cases {
126141
t.Run(tc.name, func(t *testing.T) {
@@ -160,6 +175,64 @@ func TestInternalReleaseImageCreate(t *testing.T) {
160175
}
161176
}
162177

178+
func TestInternalReleaseImageStatusOnError(t *testing.T) {
179+
cases := []struct {
180+
name string
181+
initialObjects func() []runtime.Object
182+
verify func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage)
183+
}{
184+
{
185+
name: "status condition Degraded=True when ControllerConfig is missing",
186+
initialObjects: objs(
187+
iri(),
188+
clusterVersion(), iriCertSecret()),
189+
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage) {
190+
assert.NotNil(t, actualIRI)
191+
assert.Len(t, actualIRI.Status.Conditions, 1)
192+
assert.Equal(t, string(mcfgv1alpha1.InternalReleaseImageStatusConditionTypeDegraded), actualIRI.Status.Conditions[0].Type)
193+
assert.Equal(t, metav1.ConditionTrue, actualIRI.Status.Conditions[0].Status)
194+
assert.Equal(t, "SyncError", actualIRI.Status.Conditions[0].Reason)
195+
assert.Contains(t, actualIRI.Status.Conditions[0].Message, "could not get ControllerConfig")
196+
},
197+
},
198+
{
199+
name: "status condition Degraded=True when Secret is missing",
200+
initialObjects: objs(
201+
iri(),
202+
clusterVersion(), cconfig()),
203+
verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage) {
204+
assert.NotNil(t, actualIRI)
205+
assert.Len(t, actualIRI.Status.Conditions, 1)
206+
assert.Equal(t, string(mcfgv1alpha1.InternalReleaseImageStatusConditionTypeDegraded), actualIRI.Status.Conditions[0].Type)
207+
assert.Equal(t, metav1.ConditionTrue, actualIRI.Status.Conditions[0].Status)
208+
assert.Equal(t, "SyncError", actualIRI.Status.Conditions[0].Reason)
209+
assert.Contains(t, actualIRI.Status.Conditions[0].Message, "could not get Secret")
210+
},
211+
},
212+
}
213+
214+
for _, tc := range cases {
215+
t.Run(tc.name, func(t *testing.T) {
216+
objs := tc.initialObjects()
217+
f := newFixture(t, objs)
218+
// Run the controller and expect an error
219+
f.runController(ctrlcommon.InternalReleaseImageInstanceName, true)
220+
221+
if tc.verify != nil {
222+
actualIRI, err := f.client.MachineconfigurationV1alpha1().InternalReleaseImages().Get(context.TODO(), ctrlcommon.InternalReleaseImageInstanceName, v1.GetOptions{})
223+
if err != nil {
224+
if !errors.IsNotFound(err) {
225+
t.Errorf("Error getting IRI: %v", err)
226+
} else {
227+
actualIRI = nil
228+
}
229+
}
230+
tc.verify(t, actualIRI)
231+
}
232+
})
233+
}
234+
}
235+
163236
// The fixture used to setup and run the controller.
164237
type fixture struct {
165238
t *testing.T

0 commit comments

Comments
 (0)