From c6f181ba7c2f0954accba777accd50bed4d56edb Mon Sep 17 00:00:00 2001 From: Traian Schiau <55734665+trasc@users.noreply.github.com> Date: Thu, 8 Aug 2024 16:55:59 +0300 Subject: [PATCH] [jobframework] Add integration's framework dependencies. (#2768) * [jobframework] Add integration's framework dependencies. * Review Remarks. * Don't check for circular dependency --- .../jobframework/integrationmanager.go | 24 +++++++++ .../jobframework/integrationmanager_test.go | 53 +++++++++++++++++++ pkg/controller/jobframework/setup.go | 4 ++ 3 files changed, 81 insertions(+) diff --git a/pkg/controller/jobframework/integrationmanager.go b/pkg/controller/jobframework/integrationmanager.go index dd634057bf..021e4e04df 100644 --- a/pkg/controller/jobframework/integrationmanager.go +++ b/pkg/controller/jobframework/integrationmanager.go @@ -20,12 +20,14 @@ import ( "context" "errors" "fmt" + "slices" "sort" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "k8s.io/utils/set" ctrl "sigs.k8s.io/controller-runtime" @@ -37,6 +39,9 @@ var ( errDuplicateFrameworkName = errors.New("duplicate framework name") errMissingMandatoryField = errors.New("mandatory field missing") errFrameworkNameFormat = errors.New("misformatted external framework name") + + errIntegrationNotFound = errors.New("integration not found") + errDependecncyIntegrationNotEnabled = errors.New("integration not enabled") ) type JobReconcilerInterface interface { @@ -69,6 +74,8 @@ type IntegrationCallbacks struct { CanSupportIntegration func(opts ...Option) (bool, error) // The job's MultiKueue adapter (optional) MultiKueueAdapter MultiKueueAdapter + // The list of integration that need to be enabled along with the current one. + DependencyList []string } type integrationManager struct { @@ -179,6 +186,23 @@ func (m *integrationManager) getJobTypeForOwner(ownerRef *metav1.OwnerReference) return nil } +func (m *integrationManager) checkEnabledListDependencies(enabledSet sets.Set[string]) error { + enabled := enabledSet.UnsortedList() + slices.Sort(enabled) + for _, integration := range enabled { + cbs, found := m.integrations[integration] + if !found { + return fmt.Errorf("%q %w", integration, errIntegrationNotFound) + } + for _, dep := range cbs.DependencyList { + if !enabledSet.Has(dep) { + return fmt.Errorf("%q %w %q", integration, errDependecncyIntegrationNotEnabled, dep) + } + } + } + return nil +} + // RegisterIntegration registers a new framework, returns an error when // attempting to register multiple frameworks with the same name or if a // mandatory callback is missing. diff --git a/pkg/controller/jobframework/integrationmanager_test.go b/pkg/controller/jobframework/integrationmanager_test.go index 9375773e04..b7db4b7231 100644 --- a/pkg/controller/jobframework/integrationmanager_test.go +++ b/pkg/controller/jobframework/integrationmanager_test.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -445,3 +446,55 @@ func TestGetJobTypeForOwner(t *testing.T) { }) } } + +func TestEnabledIntegrationsDependencies(t *testing.T) { + cases := map[string]struct { + integrationsDependencies map[string][]string + enabled []string + wantError error + }{ + "empty": {}, + "not found": { + enabled: []string{"i1"}, + wantError: errIntegrationNotFound, + }, + "dependecncy not enabled": { + integrationsDependencies: map[string][]string{ + "i1": {"i2"}, + }, + enabled: []string{"i1"}, + wantError: errDependecncyIntegrationNotEnabled, + }, + "dependecncy not found": { + integrationsDependencies: map[string][]string{ + "i1": {"i2"}, + }, + enabled: []string{"i1", "i2"}, + wantError: errIntegrationNotFound, + }, + "no error": { + integrationsDependencies: map[string][]string{ + "i1": {"i2", "i3"}, + "i2": {"i3"}, + "i3": nil, + }, + enabled: []string{"i1", "i2", "i3"}, + }, + } + for tcName, tc := range cases { + t.Run(tcName, func(t *testing.T) { + manager := integrationManager{ + integrations: map[string]IntegrationCallbacks{}, + } + for inegration, deps := range tc.integrationsDependencies { + manager.integrations[inegration] = IntegrationCallbacks{ + DependencyList: deps, + } + } + gotError := manager.checkEnabledListDependencies(sets.New(tc.enabled...)) + if diff := cmp.Diff(tc.wantError, gotError, cmpopts.EquateErrors()); diff != "" { + t.Errorf("Unexpected check error (-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/controller/jobframework/setup.go b/pkg/controller/jobframework/setup.go index ebc4bf864e..cc07471ccb 100644 --- a/pkg/controller/jobframework/setup.go +++ b/pkg/controller/jobframework/setup.go @@ -51,6 +51,10 @@ func SetupControllers(mgr ctrl.Manager, log logr.Logger, opts ...Option) error { func (m *integrationManager) setupControllers(mgr ctrl.Manager, log logr.Logger, opts ...Option) error { options := ProcessOptions(opts...) + if err := m.checkEnabledListDependencies(options.EnabledFrameworks); err != nil { + return fmt.Errorf("check enabled frameworks list: %w", err) + } + for fwkName := range options.EnabledExternalFrameworks { if err := RegisterExternalJobType(fwkName); err != nil { return err