Skip to content

Commit

Permalink
[jobframework] Add integration's framework dependencies. (#2768)
Browse files Browse the repository at this point in the history
* [jobframework] Add integration's framework dependencies.

* Review Remarks.

* Don't check for circular dependency
  • Loading branch information
trasc authored Aug 8, 2024
1 parent 00111d9 commit c6f181b
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 0 deletions.
24 changes: 24 additions & 0 deletions pkg/controller/jobframework/integrationmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
53 changes: 53 additions & 0 deletions pkg/controller/jobframework/integrationmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/controller/jobframework/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c6f181b

Please sign in to comment.