Skip to content

Commit 9b3682b

Browse files
Move ConfigureClusterProfileCache to config pkg (#8124)
Co-authored-by: Michal Szadkowski <[email protected]>
1 parent 5b5aa39 commit 9b3682b

File tree

3 files changed

+158
-30
lines changed

3 files changed

+158
-30
lines changed

cmd/kueue/main.go

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ import (
2929
zaplog "go.uber.org/zap"
3030
"go.uber.org/zap/zapcore"
3131
schedulingv1 "k8s.io/api/scheduling/v1"
32-
apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
33-
apierrors "k8s.io/apimachinery/pkg/api/errors"
3432
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3533
"k8s.io/apimachinery/pkg/runtime"
3634
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -44,9 +42,7 @@ import (
4442
"k8s.io/utils/ptr"
4543
inventoryv1alpha1 "sigs.k8s.io/cluster-inventory-api/apis/v1alpha1"
4644
ctrl "sigs.k8s.io/controller-runtime"
47-
ctrlcache "sigs.k8s.io/controller-runtime/pkg/cache"
4845
"sigs.k8s.io/controller-runtime/pkg/certwatcher"
49-
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
5046
"sigs.k8s.io/controller-runtime/pkg/healthz"
5147
"sigs.k8s.io/controller-runtime/pkg/log/zap"
5248
"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
@@ -221,7 +217,7 @@ func main() {
221217
}
222218

223219
if features.Enabled(features.MultiKueueClusterProfile) {
224-
if err := configureClusterProfileCache(ctx, &options, kubeConfig, cfg); err != nil {
220+
if err := config.ConfigureClusterProfileCache(ctx, setupLog, &options, kubeConfig, cfg); err != nil {
225221
setupLog.Error(err, "Unable to configure cluster profile")
226222
os.Exit(1)
227223
}
@@ -533,27 +529,3 @@ func apply(configFile string) (ctrl.Options, configapi.Configuration, error) {
533529
setupLog.Info("Successfully loaded configuration", "config", cfgStr)
534530
return options, cfg, nil
535531
}
536-
537-
func configureClusterProfileCache(ctx context.Context, options *ctrl.Options, kubeConfig *rest.Config, cfg configapi.Configuration) error {
538-
crdClient, err := apiextensionsclient.NewForConfig(kubeConfig)
539-
if err != nil {
540-
return fmt.Errorf("%w: failed creating the CRD client", err)
541-
}
542-
if _, err := crdClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, "clusterprofiles.multicluster.x-k8s.io", metav1.GetOptions{}); err != nil {
543-
if apierrors.IsNotFound(err) {
544-
setupLog.Info("Skipping MultiKueue ClusterProfile setup as the ClusterProfile CRD is not installed")
545-
return nil
546-
}
547-
return fmt.Errorf("%w: failed loading the ClusterProfile CRD", err)
548-
}
549-
objectKeyClusterProfile := new(inventoryv1alpha1.ClusterProfile)
550-
if options.Cache.ByObject == nil {
551-
options.Cache.ByObject = make(map[ctrlclient.Object]ctrlcache.ByObject)
552-
}
553-
options.Cache.ByObject[objectKeyClusterProfile] = ctrlcache.ByObject{
554-
Namespaces: map[string]ctrlcache.Config{
555-
*cfg.Namespace: {},
556-
},
557-
}
558-
return nil
559-
}

pkg/config/config.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,20 @@ package config
1818

1919
import (
2020
"bytes"
21+
"context"
2122
"fmt"
2223
"os"
2324

25+
"github.com/go-logr/logr"
2426
corev1 "k8s.io/api/core/v1"
27+
apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
2528
"k8s.io/apimachinery/pkg/api/equality"
29+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2630
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2731
"k8s.io/apimachinery/pkg/runtime"
2832
"k8s.io/apimachinery/pkg/runtime/serializer"
33+
"k8s.io/client-go/rest"
34+
inventoryv1alpha1 "sigs.k8s.io/cluster-inventory-api/apis/v1alpha1"
2935
ctrl "sigs.k8s.io/controller-runtime"
3036
ctrlcache "sigs.k8s.io/controller-runtime/pkg/cache"
3137
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -35,7 +41,8 @@ import (
3541
)
3642

3743
var (
38-
objectKeySecret = new(corev1.Secret)
44+
objectKeySecret = new(corev1.Secret)
45+
objectKeyClusterProfile = new(inventoryv1alpha1.ClusterProfile)
3946
)
4047

4148
// fromFile provides an alternative to the deprecated ctrl.ConfigFile().AtPath(path).OfKind(&cfg)
@@ -201,3 +208,33 @@ func Load(scheme *runtime.Scheme, configFile string) (ctrl.Options, configapi.Co
201208
addTo(&options, &cfg)
202209
return options, cfg, err
203210
}
211+
212+
// ConfigureClusterProfileCache creates the CRD client from kubeConfig and delegates
213+
// to ConfigureClusterProfileCacheWithClient. Keeping this wrapper preserves the
214+
// original API while enabling dependency injection via the WithClient function.
215+
func ConfigureClusterProfileCache(ctx context.Context, log logr.Logger, options *ctrl.Options, kubeConfig *rest.Config, cfg configapi.Configuration) error {
216+
crdClient, err := apiextensionsclient.NewForConfig(kubeConfig)
217+
if err != nil {
218+
return fmt.Errorf("failed creating the CRD client: %w", err)
219+
}
220+
return configureClusterProfileCacheWithClient(ctx, log, options, crdClient, cfg)
221+
}
222+
223+
func configureClusterProfileCacheWithClient(ctx context.Context, log logr.Logger, options *ctrl.Options, crdClient apiextensionsclient.Interface, cfg configapi.Configuration) error {
224+
if _, err := crdClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, "clusterprofiles.multicluster.x-k8s.io", metav1.GetOptions{}); err != nil {
225+
if apierrors.IsNotFound(err) {
226+
log.Info("Skipping MultiKueue ClusterProfile setup as the ClusterProfile CRD is not installed")
227+
return nil
228+
}
229+
return fmt.Errorf("failed loading the ClusterProfile CRD: %w", err)
230+
}
231+
if options.Cache.ByObject == nil {
232+
options.Cache.ByObject = make(map[ctrlclient.Object]ctrlcache.ByObject)
233+
}
234+
options.Cache.ByObject[objectKeyClusterProfile] = ctrlcache.ByObject{
235+
Namespaces: map[string]ctrlcache.Config{
236+
*cfg.Namespace: {},
237+
},
238+
}
239+
return nil
240+
}

pkg/config/config_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,15 @@ import (
2929
"github.com/google/go-cmp/cmp"
3030
"github.com/google/go-cmp/cmp/cmpopts"
3131
corev1 "k8s.io/api/core/v1"
32+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
33+
apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
34+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3235
resourcev1 "k8s.io/apimachinery/pkg/api/resource"
3336
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3437
"k8s.io/apimachinery/pkg/runtime"
3538
"k8s.io/apimachinery/pkg/util/yaml"
39+
"k8s.io/client-go/rest"
40+
clienttesting "k8s.io/client-go/testing"
3641
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
3742
"k8s.io/client-go/tools/leaderelection/resourcelock"
3843
"k8s.io/utils/ptr"
@@ -45,6 +50,7 @@ import (
4550

4651
configapi "sigs.k8s.io/kueue/apis/config/v1beta2"
4752
"sigs.k8s.io/kueue/pkg/controller/jobs/job"
53+
utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
4854
"sigs.k8s.io/kueue/pkg/util/waitforpodsready"
4955

5056
_ "sigs.k8s.io/kueue/pkg/controller/jobs"
@@ -1054,3 +1060,116 @@ func TestWaitForPodsReadyIsEnabled(t *testing.T) {
10541060
})
10551061
}
10561062
}
1063+
1064+
func TestConfigureClusterProfileCacheWithClient(t *testing.T) {
1065+
multiclusterCRD := &apiextensionsv1.CustomResourceDefinition{
1066+
ObjectMeta: metav1.ObjectMeta{
1067+
Name: "clusterprofiles.multicluster.x-k8s.io",
1068+
},
1069+
}
1070+
1071+
testCases := map[string]struct {
1072+
crdPresent bool
1073+
failedGetCRD bool
1074+
wantError bool
1075+
wantOptionsCache func(string) ctrlcache.Options
1076+
}{
1077+
"clusterProfile CRD not present": {
1078+
wantOptionsCache: defaultControlCacheOptions,
1079+
},
1080+
"clusterProfile cache added to ByObject": {
1081+
crdPresent: true,
1082+
wantOptionsCache: func(namespace string) ctrlcache.Options {
1083+
cOpts := defaultControlOptions(namespace)
1084+
cOpts.Cache.ByObject[objectKeyClusterProfile] = ctrlcache.ByObject{
1085+
Namespaces: map[string]ctrlcache.Config{
1086+
namespace: {},
1087+
},
1088+
}
1089+
return cOpts.Cache
1090+
},
1091+
},
1092+
"error failed loading the ClusterProfile CRD": {
1093+
crdPresent: true,
1094+
failedGetCRD: true,
1095+
wantError: true,
1096+
},
1097+
}
1098+
1099+
for name, tc := range testCases {
1100+
t.Run(name, func(t *testing.T) {
1101+
ctx, log := utiltesting.ContextWithLog(t)
1102+
opts := &ctrl.Options{
1103+
Cache: defaultControlCacheOptions(configapi.DefaultNamespace),
1104+
}
1105+
cfg := &configapi.Configuration{
1106+
Namespace: ptr.To(configapi.DefaultNamespace),
1107+
}
1108+
1109+
var objects []runtime.Object
1110+
if tc.crdPresent {
1111+
objects = append(objects, multiclusterCRD)
1112+
}
1113+
1114+
fake := apiextensionsfake.NewClientset(objects...)
1115+
fake.PrependReactor("get", "customresourcedefinitions", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
1116+
getAction := action.(clienttesting.GetAction)
1117+
if getAction.GetName() == multiclusterCRD.Name && tc.failedGetCRD {
1118+
return true, nil, apierrors.NewBadRequest("testing error getting CRD")
1119+
}
1120+
return false, nil, nil
1121+
})
1122+
1123+
err := configureClusterProfileCacheWithClient(ctx, log, opts, fake, *cfg)
1124+
if tc.wantError {
1125+
if err == nil {
1126+
t.Error("Expected error but got none")
1127+
}
1128+
} else {
1129+
if err != nil {
1130+
t.Errorf("Unexpected error:%s", err)
1131+
}
1132+
if diff := cmp.Diff(tc.wantOptionsCache(configapi.DefaultNamespace), opts.Cache); diff != "" {
1133+
t.Errorf("Unexpected options cache (-want +got):\n%s", diff)
1134+
}
1135+
}
1136+
})
1137+
}
1138+
}
1139+
1140+
func TestConfigureClusterProfileCache(t *testing.T) {
1141+
testCases := map[string]struct {
1142+
kubeConfig *rest.Config
1143+
}{
1144+
"error creating CRD client with empty kubeConfig": {
1145+
kubeConfig: &rest.Config{},
1146+
},
1147+
"error creating CRD client with invalid kubeConfig": {
1148+
kubeConfig: &rest.Config{Host: "http://invalid-host"},
1149+
},
1150+
"valid kubeConfig but no clusterProfile CRD": {
1151+
kubeConfig: &rest.Config{
1152+
Host: "https://127.0.0.1:6443",
1153+
BearerToken: "fake-token",
1154+
TLSClientConfig: rest.TLSClientConfig{
1155+
Insecure: true,
1156+
},
1157+
},
1158+
},
1159+
}
1160+
1161+
for name, tc := range testCases {
1162+
t.Run(name, func(t *testing.T) {
1163+
ctx, log := utiltesting.ContextWithLog(t)
1164+
opts := &ctrl.Options{
1165+
Cache: ctrlcache.Options{},
1166+
}
1167+
cfg := configapi.Configuration{Namespace: ptr.To(configapi.DefaultNamespace)}
1168+
err := ConfigureClusterProfileCache(ctx, log, opts, tc.kubeConfig, cfg)
1169+
1170+
if err == nil {
1171+
t.Error("Expected error but got none")
1172+
}
1173+
})
1174+
}
1175+
}

0 commit comments

Comments
 (0)