Skip to content

Commit f9d8abc

Browse files
authored
Merge pull request #2177 from vincepri/rework-cluster-options
✨ Expose Cache/Client options on Cluster
2 parents f127c11 + ed9c5ef commit f9d8abc

File tree

6 files changed

+146
-43
lines changed

6 files changed

+146
-43
lines changed

pkg/client/client.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ type Options struct {
5353
// WarningHandler is used to configure the warning handler responsible for
5454
// surfacing and handling warnings messages sent by the API server.
5555
WarningHandler WarningHandlerOptions
56+
57+
// DryRun instructs the client to only perform dry run requests.
58+
DryRun *bool
5659
}
5760

5861
// WarningHandlerOptions are options for configuring a
@@ -94,8 +97,12 @@ type NewClientFunc func(config *rest.Config, options Options) (Client, error)
9497
// corresponding group, version, and kind for the given type. In the
9598
// case of unstructured types, the group, version, and kind will be extracted
9699
// from the corresponding fields on the object.
97-
func New(config *rest.Config, options Options) (Client, error) {
98-
return newClient(config, options)
100+
func New(config *rest.Config, options Options) (c Client, err error) {
101+
c, err = newClient(config, options)
102+
if err == nil && options.DryRun != nil && *options.DryRun {
103+
c = NewDryRunClient(c)
104+
}
105+
return c, err
99106
}
100107

101108
func newClient(config *rest.Config, options Options) (*client, error) {

pkg/client/client_suite_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
"sigs.k8s.io/controller-runtime/pkg/log/zap"
3232
)
3333

34-
func TestSource(t *testing.T) {
34+
func TestClient(t *testing.T) {
3535
RegisterFailHandler(Fail)
3636
RunSpecs(t, "Client Suite")
3737
}

pkg/client/client_test.go

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ import (
3333
corev1 "k8s.io/api/core/v1"
3434
policyv1 "k8s.io/api/policy/v1"
3535
apierrors "k8s.io/apimachinery/pkg/api/errors"
36+
"k8s.io/apimachinery/pkg/api/meta"
3637
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3738
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3839
"k8s.io/apimachinery/pkg/runtime"
3940
"k8s.io/apimachinery/pkg/runtime/schema"
4041
"k8s.io/apimachinery/pkg/types"
4142
kscheme "k8s.io/client-go/kubernetes/scheme"
43+
"k8s.io/utils/pointer"
4244

4345
"sigs.k8s.io/controller-runtime/examples/crd/pkg"
4446
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -221,8 +223,6 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
221223
Expect(client.IgnoreNotFound(err)).NotTo(HaveOccurred())
222224
})
223225

224-
// TODO(seans): Cast "cl" as "client" struct from "Client" interface. Then validate the
225-
// instance values for the "client" struct.
226226
Describe("New", func() {
227227
It("should return a new Client", func() {
228228
cl, err := client.New(cfg, client.Options{})
@@ -236,29 +236,46 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
236236
Expect(cl).To(BeNil())
237237
})
238238

239-
// TODO(seans): cast as client struct and inspect Scheme
240239
It("should use the provided Scheme if provided", func() {
241240
cl, err := client.New(cfg, client.Options{Scheme: scheme})
242241
Expect(err).NotTo(HaveOccurred())
243242
Expect(cl).NotTo(BeNil())
243+
Expect(cl.Scheme()).ToNot(BeNil())
244+
Expect(cl.Scheme()).To(Equal(scheme))
244245
})
245246

246-
// TODO(seans): cast as client struct and inspect Scheme
247247
It("should default the Scheme if not provided", func() {
248248
cl, err := client.New(cfg, client.Options{})
249249
Expect(err).NotTo(HaveOccurred())
250250
Expect(cl).NotTo(BeNil())
251+
Expect(cl.Scheme()).ToNot(BeNil())
252+
Expect(cl.Scheme()).To(Equal(kscheme.Scheme))
251253
})
252254

253-
PIt("should use the provided Mapper if provided", func() {
254-
255+
It("should use the provided Mapper if provided", func() {
256+
mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{})
257+
cl, err := client.New(cfg, client.Options{Mapper: mapper})
258+
Expect(err).NotTo(HaveOccurred())
259+
Expect(cl).NotTo(BeNil())
260+
Expect(cl.RESTMapper()).ToNot(BeNil())
261+
Expect(cl.RESTMapper()).To(Equal(mapper))
255262
})
256263

257-
// TODO(seans): cast as client struct and inspect Mapper
258264
It("should create a Mapper if not provided", func() {
259265
cl, err := client.New(cfg, client.Options{})
260266
Expect(err).NotTo(HaveOccurred())
261267
Expect(cl).NotTo(BeNil())
268+
Expect(cl.RESTMapper()).ToNot(BeNil())
269+
})
270+
271+
It("should use the provided reader cache if provided, on get and list", func() {
272+
cache := &fakeReader{}
273+
cl, err := client.New(cfg, client.Options{Cache: &client.CacheOptions{Reader: cache}})
274+
Expect(err).NotTo(HaveOccurred())
275+
Expect(cl).NotTo(BeNil())
276+
Expect(cl.Get(ctx, client.ObjectKey{Name: "test"}, &appsv1.Deployment{})).To(Succeed())
277+
Expect(cl.List(ctx, &appsv1.DeploymentList{})).To(Succeed())
278+
Expect(cache.Called).To(Equal(2))
262279
})
263280
})
264281

@@ -350,7 +367,22 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
350367
})
351368

352369
Context("with the DryRun option", func() {
353-
It("should not create a new object", func() {
370+
It("should not create a new object, global option", func() {
371+
cl, err := client.New(cfg, client.Options{DryRun: pointer.Bool(true)})
372+
Expect(err).NotTo(HaveOccurred())
373+
Expect(cl).NotTo(BeNil())
374+
375+
By("creating the object (with DryRun)")
376+
err = cl.Create(context.TODO(), dep)
377+
Expect(err).NotTo(HaveOccurred())
378+
379+
actual, err := clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{})
380+
Expect(err).To(HaveOccurred())
381+
Expect(apierrors.IsNotFound(err)).To(BeTrue())
382+
Expect(actual).To(Equal(&appsv1.Deployment{}))
383+
})
384+
385+
It("should not create a new object, inline option", func() {
354386
cl, err := client.New(cfg, client.Options{})
355387
Expect(err).NotTo(HaveOccurred())
356388
Expect(cl).NotTo(BeNil())

pkg/client/dryrun_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
apierrors "k8s.io/apimachinery/pkg/api/errors"
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3030
"k8s.io/apimachinery/pkg/types"
31+
"k8s.io/utils/pointer"
3132

3233
"sigs.k8s.io/controller-runtime/pkg/client"
3334
)
@@ -40,10 +41,10 @@ var _ = Describe("DryRunClient", func() {
4041
ctx := context.Background()
4142

4243
getClient := func() client.Client {
43-
nonDryRunClient, err := client.New(cfg, client.Options{})
44+
cl, err := client.New(cfg, client.Options{DryRun: pointer.Bool(true)})
4445
Expect(err).NotTo(HaveOccurred())
45-
Expect(nonDryRunClient).NotTo(BeNil())
46-
return client.NewDryRunClient(nonDryRunClient)
46+
Expect(cl).NotTo(BeNil())
47+
return cl
4748
}
4849

4950
BeforeEach(func() {

pkg/cluster/cluster.go

Lines changed: 87 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"k8s.io/client-go/kubernetes/scheme"
2929
"k8s.io/client-go/rest"
3030
"k8s.io/client-go/tools/record"
31+
"k8s.io/utils/pointer"
3132
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
3233
logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
3334

@@ -82,6 +83,8 @@ type Options struct {
8283
Scheme *runtime.Scheme
8384

8485
// MapperProvider provides the rest mapper used to map go types to Kubernetes APIs
86+
//
87+
// Deprecated: Set Cache.Mapper and Client.Mapper directly instead.
8588
MapperProvider func(c *rest.Config, httpClient *http.Client) (meta.RESTMapper, error)
8689

8790
// Logger is the logger that should be used by this Cluster.
@@ -102,29 +105,54 @@ type Options struct {
102105
// Note: If a namespace is specified, controllers can still Watch for a
103106
// cluster-scoped resource (e.g Node). For namespaced resources the cache
104107
// will only hold objects from the desired namespace.
108+
//
109+
// Deprecated: Use Cache.Namespaces instead.
105110
Namespace string
106111

107112
// HTTPClient is the http client that will be used to create the default
108113
// Cache and Client. If not set the rest.HTTPClientFor function will be used
109114
// to create the http client.
110115
HTTPClient *http.Client
111116

117+
// Cache is the cache.Options that will be used to create the default Cache.
118+
// By default, the cache will watch and list requested objects in all namespaces.
119+
Cache cache.Options
120+
112121
// NewCache is the function that will create the cache to be used
113122
// by the manager. If not set this will use the default new cache function.
123+
//
124+
// When using a custom NewCache, the Cache options will be passed to the
125+
// NewCache function.
126+
//
127+
// NOTE: LOW LEVEL PRIMITIVE!
128+
// Only use a custom NewCache if you know what you are doing.
114129
NewCache cache.NewCacheFunc
115130

131+
// Client is the client.Options that will be used to create the default Client.
132+
// By default, the client will use the cache for reads and direct calls for writes.
133+
Client client.Options
134+
116135
// NewClient is the func that creates the client to be used by the manager.
117136
// If not set this will create a Client backed by a Cache for read operations
118137
// and a direct Client for write operations.
119-
// NOTE: The default client will not cache Unstructured.
138+
//
139+
// When using a custom NewClient, the Client options will be passed to the
140+
// NewClient function.
141+
//
142+
// NOTE: LOW LEVEL PRIMITIVE!
143+
// Only use a custom NewClient if you know what you are doing.
120144
NewClient client.NewClientFunc
121145

122146
// ClientDisableCacheFor tells the client that, if any cache is used, to bypass it
123147
// for the given objects.
148+
//
149+
// Deprecated: Use Client.Cache.DisableFor instead.
124150
ClientDisableCacheFor []client.Object
125151

126152
// DryRunClient specifies whether the client should be configured to enforce
127153
// dryRun mode.
154+
//
155+
// Deprecated: Use Client.DryRun instead.
128156
DryRunClient bool
129157

130158
// EventBroadcaster records Events emitted by the manager and sends them to the Kubernetes API
@@ -171,36 +199,71 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
171199
}
172200

173201
// Create the cache for the cached read client and registering informers
174-
cache, err := options.NewCache(config, cache.Options{
175-
HTTPClient: options.HTTPClient,
176-
Scheme: options.Scheme,
177-
Mapper: mapper,
178-
ResyncEvery: options.SyncPeriod,
179-
Namespaces: []string{options.Namespace},
180-
})
202+
cacheOpts := options.Cache
203+
{
204+
if cacheOpts.Scheme == nil {
205+
cacheOpts.Scheme = options.Scheme
206+
}
207+
if cacheOpts.Mapper == nil {
208+
cacheOpts.Mapper = mapper
209+
}
210+
if cacheOpts.HTTPClient == nil {
211+
cacheOpts.HTTPClient = options.HTTPClient
212+
}
213+
if cacheOpts.ResyncEvery == nil {
214+
cacheOpts.ResyncEvery = options.SyncPeriod
215+
}
216+
if len(cacheOpts.Namespaces) == 0 && options.Namespace != "" {
217+
cacheOpts.Namespaces = []string{options.Namespace}
218+
}
219+
}
220+
cache, err := options.NewCache(config, cacheOpts)
181221
if err != nil {
182222
return nil, err
183223
}
184224

185-
writeObj, err := options.NewClient(config, client.Options{
186-
HTTPClient: options.HTTPClient,
187-
Scheme: options.Scheme,
188-
Mapper: mapper,
189-
Cache: &client.CacheOptions{
190-
Reader: cache,
191-
DisableFor: options.ClientDisableCacheFor,
192-
},
193-
})
225+
// Create the client, and default its options.
226+
clientOpts := options.Client
227+
{
228+
if clientOpts.Scheme == nil {
229+
clientOpts.Scheme = options.Scheme
230+
}
231+
if clientOpts.Mapper == nil {
232+
clientOpts.Mapper = mapper
233+
}
234+
if clientOpts.HTTPClient == nil {
235+
clientOpts.HTTPClient = options.HTTPClient
236+
}
237+
if clientOpts.Cache == nil {
238+
clientOpts.Cache = &client.CacheOptions{
239+
Unstructured: false,
240+
}
241+
}
242+
if clientOpts.Cache.Reader == nil {
243+
clientOpts.Cache.Reader = cache
244+
}
245+
246+
// For backward compatibility, the ClientDisableCacheFor option should
247+
// be appended to the DisableFor option in the client.
248+
clientOpts.Cache.DisableFor = append(clientOpts.Cache.DisableFor, options.ClientDisableCacheFor...)
249+
250+
if clientOpts.DryRun == nil && options.DryRunClient {
251+
// For backward compatibility, the DryRunClient (if set) option should override
252+
// the DryRun option in the client (if unset).
253+
clientOpts.DryRun = pointer.Bool(true)
254+
}
255+
}
256+
clientWriter, err := options.NewClient(config, clientOpts)
194257
if err != nil {
195258
return nil, err
196259
}
197260

198-
if options.DryRunClient {
199-
writeObj = client.NewDryRunClient(writeObj)
200-
}
201-
202261
// Create the API Reader, a client with no cache.
203-
apiReader, err := client.New(config, client.Options{HTTPClient: options.HTTPClient, Scheme: options.Scheme, Mapper: mapper})
262+
clientReader, err := client.New(config, client.Options{
263+
HTTPClient: options.HTTPClient,
264+
Scheme: options.Scheme,
265+
Mapper: mapper,
266+
})
204267
if err != nil {
205268
return nil, err
206269
}
@@ -219,8 +282,8 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
219282
scheme: options.Scheme,
220283
cache: cache,
221284
fieldIndexes: cache,
222-
client: writeObj,
223-
apiReader: apiReader,
285+
client: clientWriter,
286+
apiReader: clientReader,
224287
recorderProvider: recorderProvider,
225288
mapper: mapper,
226289
logger: options.Logger,

pkg/manager/manager.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -348,15 +348,15 @@ func New(config *rest.Config, options Options) (Manager, error) {
348348

349349
cluster, err := cluster.New(config, func(clusterOptions *cluster.Options) {
350350
clusterOptions.Scheme = options.Scheme
351-
clusterOptions.MapperProvider = options.MapperProvider
351+
clusterOptions.MapperProvider = options.MapperProvider //nolint:staticcheck
352352
clusterOptions.Logger = options.Logger
353353
clusterOptions.SyncPeriod = options.SyncPeriod
354-
clusterOptions.Namespace = options.Namespace
354+
clusterOptions.Namespace = options.Namespace //nolint:staticcheck
355355
clusterOptions.NewCache = options.NewCache
356356
clusterOptions.NewClient = options.NewClient
357-
clusterOptions.ClientDisableCacheFor = options.ClientDisableCacheFor
358-
clusterOptions.DryRunClient = options.DryRunClient
359-
clusterOptions.EventBroadcaster = options.EventBroadcaster //nolint:staticcheck
357+
clusterOptions.ClientDisableCacheFor = options.ClientDisableCacheFor //nolint:staticcheck
358+
clusterOptions.DryRunClient = options.DryRunClient //nolint:staticcheck
359+
clusterOptions.EventBroadcaster = options.EventBroadcaster //nolint:staticcheck
360360
})
361361
if err != nil {
362362
return nil, err

0 commit comments

Comments
 (0)