From d33d16d246dc73f9b07d18a9aa641b82f94d4dfd Mon Sep 17 00:00:00 2001 From: Vishnu Bijukumar Date: Fri, 21 Aug 2026 14:34:28 +0530 Subject: [PATCH 1/2] Fix cluster quota handler registration Signed-off-by: Vishnu Bijukumar --- .../aacrq-controller/aacrq-controller.go | 4 +-- .../aacrq-controller_constructor_test.go | 36 +++++++++++++++++++ .../crq-controller/crq-controller.go | 4 +-- .../crq-controller_constructor_test.go | 36 +++++++++++++++++++ 4 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go create mode 100644 pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go index a2d552f0f..a348c2941 100644 --- a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go @@ -2,7 +2,6 @@ package aacrq_controller import ( "context" - "fmt" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -60,6 +59,7 @@ func NewAacrqController(aaqCli client.AAQClient, AddFunc: ctrl.addAcrq, }) if err != nil { + panic("failed to register ApplicationAwareClusterResourceQuota event handler") } return &ctrl @@ -167,7 +167,7 @@ func (ctrl *AacrqController) Execute() bool { err, enqueueState := ctrl.execute(key.(string)) if err != nil { - log.Log.Infof(fmt.Sprintf("AacrqController: Error with key: %v err: %v", key, err)) + log.Log.Infof("AacrqController: Error with key: %v err: %v", key, err) } switch enqueueState { case BackOff: diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go new file mode 100644 index 000000000..074f85d63 --- /dev/null +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go @@ -0,0 +1,36 @@ +package aacrq_controller + +import ( + "errors" + "testing" + + "k8s.io/client-go/tools/cache" + testsutils "kubevirt.io/application-aware-quota/pkg/tests-utils" +) + +type erroringSharedIndexInformer struct { + testsutils.FakeSharedIndexInformer + err error +} + +func (i erroringSharedIndexInformer) AddEventHandler(handler cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) { + return nil, i.err +} + +func TestNewAacrqControllerPanicsOnAcrqHandlerRegistrationError(t *testing.T) { + t.Helper() + + aacrqInformer := testsutils.NewFakeSharedIndexInformer(nil) + acrqInformer := erroringSharedIndexInformer{ + FakeSharedIndexInformer: testsutils.NewFakeSharedIndexInformer(nil), + err: errors.New("boom"), + } + + defer func() { + if recover() == nil { + t.Fatal("expected panic when acrq informer handler registration fails") + } + }() + + NewAacrqController(nil, aacrqInformer, acrqInformer, make(chan struct{})) +} diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go index 2bf85b9ca..54411848b 100644 --- a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go @@ -2,7 +2,6 @@ package crq_controller import ( "context" - "fmt" v12 "github.com/openshift/api/quota/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -67,6 +66,7 @@ func NewCRQController(aaqCli client.AAQClient, AddFunc: ctrl.addAcrq, }) if err != nil { + panic("failed to register ApplicationAwareClusterResourceQuota event handler") } return &ctrl @@ -154,7 +154,7 @@ func (ctrl *CRQController) Execute() bool { err, enqueueState := ctrl.execute(key.(string)) if err != nil { - log.Log.Infof(fmt.Sprintf("CRQController: Error with key: %v err: %v", key, err)) + log.Log.Infof("CRQController: Error with key: %v err: %v", key, err) } switch enqueueState { case BackOff: diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go new file mode 100644 index 000000000..446c88c99 --- /dev/null +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go @@ -0,0 +1,36 @@ +package crq_controller + +import ( + "errors" + "testing" + + "k8s.io/client-go/tools/cache" + testsutils "kubevirt.io/application-aware-quota/pkg/tests-utils" +) + +type erroringSharedIndexInformer struct { + testsutils.FakeSharedIndexInformer + err error +} + +func (i erroringSharedIndexInformer) AddEventHandler(handler cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) { + return nil, i.err +} + +func TestNewCRQControllerPanicsOnAcrqHandlerRegistrationError(t *testing.T) { + t.Helper() + + crqInformer := testsutils.NewFakeSharedIndexInformer(nil) + acrqInformer := erroringSharedIndexInformer{ + FakeSharedIndexInformer: testsutils.NewFakeSharedIndexInformer(nil), + err: errors.New("boom"), + } + + defer func() { + if recover() == nil { + t.Fatal("expected panic when acrq informer handler registration fails") + } + }() + + NewCRQController(nil, crqInformer, acrqInformer, make(chan struct{})) +} From ae9536e990e2174b64749feaa8d41e22901c206d Mon Sep 17 00:00:00 2001 From: Vishnu Bijukumar Date: Fri, 21 Aug 2026 14:47:09 +0530 Subject: [PATCH 2/2] Improve handler registration error reporting Signed-off-by: Vishnu Bijukumar --- .../aacrq-controller/aacrq-controller.go | 3 ++- .../aacrq-controller_constructor_test.go | 16 ++-------------- .../crq-controller/crq-controller.go | 3 ++- .../crq-controller_constructor_test.go | 16 ++-------------- pkg/tests-utils/fake-informers.go | 5 +++-- 5 files changed, 11 insertions(+), 32 deletions(-) diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go index a348c2941..bba83f519 100644 --- a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller.go @@ -2,6 +2,7 @@ package aacrq_controller import ( "context" + "fmt" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -59,7 +60,7 @@ func NewAacrqController(aaqCli client.AAQClient, AddFunc: ctrl.addAcrq, }) if err != nil { - panic("failed to register ApplicationAwareClusterResourceQuota event handler") + panic(fmt.Errorf("failed to register ApplicationAwareClusterResourceQuota event handler: %w", err)) } return &ctrl diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go index 074f85d63..69960aa11 100644 --- a/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/aacrq-controller/aacrq-controller_constructor_test.go @@ -4,27 +4,15 @@ import ( "errors" "testing" - "k8s.io/client-go/tools/cache" testsutils "kubevirt.io/application-aware-quota/pkg/tests-utils" ) -type erroringSharedIndexInformer struct { - testsutils.FakeSharedIndexInformer - err error -} - -func (i erroringSharedIndexInformer) AddEventHandler(handler cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) { - return nil, i.err -} - func TestNewAacrqControllerPanicsOnAcrqHandlerRegistrationError(t *testing.T) { t.Helper() aacrqInformer := testsutils.NewFakeSharedIndexInformer(nil) - acrqInformer := erroringSharedIndexInformer{ - FakeSharedIndexInformer: testsutils.NewFakeSharedIndexInformer(nil), - err: errors.New("boom"), - } + acrqInformer := testsutils.NewFakeSharedIndexInformer(nil) + acrqInformer.AddEventHandlerErr = errors.New("boom") defer func() { if recover() == nil { diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go index 54411848b..ca0f52fdc 100644 --- a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller.go @@ -2,6 +2,7 @@ package crq_controller import ( "context" + "fmt" v12 "github.com/openshift/api/quota/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -66,7 +67,7 @@ func NewCRQController(aaqCli client.AAQClient, AddFunc: ctrl.addAcrq, }) if err != nil { - panic("failed to register ApplicationAwareClusterResourceQuota event handler") + panic(fmt.Errorf("failed to register ApplicationAwareClusterResourceQuota event handler: %w", err)) } return &ctrl diff --git a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go index 446c88c99..a4979f6a6 100644 --- a/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go +++ b/pkg/aaq-controller/additional-cluster-quota-controllers/crq-controller/crq-controller_constructor_test.go @@ -4,27 +4,15 @@ import ( "errors" "testing" - "k8s.io/client-go/tools/cache" testsutils "kubevirt.io/application-aware-quota/pkg/tests-utils" ) -type erroringSharedIndexInformer struct { - testsutils.FakeSharedIndexInformer - err error -} - -func (i erroringSharedIndexInformer) AddEventHandler(handler cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) { - return nil, i.err -} - func TestNewCRQControllerPanicsOnAcrqHandlerRegistrationError(t *testing.T) { t.Helper() crqInformer := testsutils.NewFakeSharedIndexInformer(nil) - acrqInformer := erroringSharedIndexInformer{ - FakeSharedIndexInformer: testsutils.NewFakeSharedIndexInformer(nil), - err: errors.New("boom"), - } + acrqInformer := testsutils.NewFakeSharedIndexInformer(nil) + acrqInformer.AddEventHandlerErr = errors.New("boom") defer func() { if recover() == nil { diff --git a/pkg/tests-utils/fake-informers.go b/pkg/tests-utils/fake-informers.go index 21ccece17..00ac7ca0a 100644 --- a/pkg/tests-utils/fake-informers.go +++ b/pkg/tests-utils/fake-informers.go @@ -13,6 +13,7 @@ import ( type FakeSharedIndexInformer struct { indexer cache.Indexer InternalGetIndexer func(cache.Indexer) cache.Indexer + AddEventHandlerErr error } func (i FakeSharedIndexInformer) AddEventHandlerWithOptions(handler cache.ResourceEventHandler, options cache.HandlerOptions) (cache.ResourceEventHandlerRegistration, error) { @@ -53,10 +54,10 @@ func (i FakeSharedIndexInformer) GetIndexer() cache.Indexer { } func (i FakeSharedIndexInformer) AddEventHandler(handler cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) { - return nil, nil + return nil, i.AddEventHandlerErr } func (i FakeSharedIndexInformer) AddEventHandlerWithResyncPeriod(handler cache.ResourceEventHandler, resyncPeriod time.Duration) (cache.ResourceEventHandlerRegistration, error) { - return nil, nil + return nil, i.AddEventHandlerErr } func (i FakeSharedIndexInformer) GetStore() cache.Store { return nil } func (i FakeSharedIndexInformer) GetController() cache.Controller { return nil }