From 2cd6cbdcb51a347b95b26122c92a200fd3dfd112 Mon Sep 17 00:00:00 2001 From: Hossein Pursultani <hpursultani@gitlab.com> Date: Tue, 15 Nov 2022 14:42:06 +1100 Subject: [PATCH 1/6] Provide access to Client resources from Context --- pkg/runtime/builder.go | 55 ++++++++++++++++++++++++++++++++++++++++++ pkg/runtime/context.go | 27 +++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 pkg/runtime/builder.go create mode 100644 pkg/runtime/context.go diff --git a/pkg/runtime/builder.go b/pkg/runtime/builder.go new file mode 100644 index 000000000..40d218643 --- /dev/null +++ b/pkg/runtime/builder.go @@ -0,0 +1,55 @@ +package runtime + +import ( + "context" + + "github.com/go-logr/logr" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type ContextBuilder struct { + client client.Client + logger logr.Logger + recorder record.EventRecorder +} + +func NewContext() *ContextBuilder { + return &ContextBuilder{} +} + +func (b *ContextBuilder) WithLogger(logger logr.Logger) *ContextBuilder { + b.logger = logger + + return b +} + +func (b *ContextBuilder) WithClient(client client.Client) *ContextBuilder { + b.client = client + + return b +} + +func (b *ContextBuilder) WithEventRecorder(recorder record.EventRecorder) *ContextBuilder { + b.recorder = recorder + + return b +} + +func (b *ContextBuilder) Build(parent context.Context) context.Context { + ctx := parent + + if b.logger != nil { + ctx = logr.NewContext(ctx, b.logger) + } + + if b.client != nil { + ctx = context.WithValue(ctx, clientContextKey{}, b.client) + } + + if b.recorder != nil { + ctx = context.WithValue(ctx, recorderContextKey{}, b.recorder) + } + + return ctx +} diff --git a/pkg/runtime/context.go b/pkg/runtime/context.go new file mode 100644 index 000000000..72373475f --- /dev/null +++ b/pkg/runtime/context.go @@ -0,0 +1,27 @@ +package runtime + +import ( + "context" + + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type clientContextKey struct{} +type recorderContextKey struct{} + +func ClientFromContext(ctx context.Context) client.Client { + if c, ok := ctx.Value(clientContextKey{}).(client.Client); ok { + return c + } + + return nil +} + +func RecorderFromContext(ctx context.Context) record.EventRecorder { + if r, ok := ctx.Value(clientContextKey{}).(record.EventRecorder); ok { + return r + } + + return nil +} -- GitLab From dc637f6a0daa30b48a0f0ba9d597da0f8407c501 Mon Sep 17 00:00:00 2001 From: Hossein Pursultani <hpursultani@gitlab.com> Date: Tue, 15 Nov 2022 16:04:38 +1100 Subject: [PATCH 2/6] Document runtime context types and functions --- pkg/runtime/builder.go | 12 ++++++++++++ pkg/runtime/context.go | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/pkg/runtime/builder.go b/pkg/runtime/builder.go index 40d218643..6ecd6801e 100644 --- a/pkg/runtime/builder.go +++ b/pkg/runtime/builder.go @@ -8,34 +8,46 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// ContextBuilder provides an interface for building a new runtime Context. type ContextBuilder struct { client client.Client logger logr.Logger recorder record.EventRecorder } +// NewContext returns a new ContextBuilder to build a new runtime Context. func NewContext() *ContextBuilder { return &ContextBuilder{} } +// WithLogger uses the specified Logger for the new runtime Context. +// +// This is compatible with logr Context. func (b *ContextBuilder) WithLogger(logger logr.Logger) *ContextBuilder { b.logger = logger return b } +// WithClient uses the specified Client for the new runtime Context. +// +// Generally this is the Client that the controller-runtime creates and +// is associated to the Controller. func (b *ContextBuilder) WithClient(client client.Client) *ContextBuilder { b.client = client return b } +// WithEventRecorder uses the specified EventRecorder for the new runtime +// Context. func (b *ContextBuilder) WithEventRecorder(recorder record.EventRecorder) *ContextBuilder { b.recorder = recorder return b } +// Build builds the new runtime Context and returns it. func (b *ContextBuilder) Build(parent context.Context) context.Context { ctx := parent diff --git a/pkg/runtime/context.go b/pkg/runtime/context.go index 72373475f..2833567e5 100644 --- a/pkg/runtime/context.go +++ b/pkg/runtime/context.go @@ -7,9 +7,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -type clientContextKey struct{} -type recorderContextKey struct{} - +// ClientFromContext returns a Client from the context or nil if client details +// are not found. +// +// Use runtime ContextBuilder func ClientFromContext(ctx context.Context) client.Client { if c, ok := ctx.Value(clientContextKey{}).(client.Client); ok { return c @@ -18,6 +19,8 @@ func ClientFromContext(ctx context.Context) client.Client { return nil } +// RecorderFromContext returns a Client from the context or nil if recorder +// details are not found. func RecorderFromContext(ctx context.Context) record.EventRecorder { if r, ok := ctx.Value(clientContextKey{}).(record.EventRecorder); ok { return r @@ -25,3 +28,9 @@ func RecorderFromContext(ctx context.Context) record.EventRecorder { return nil } + +/* Private */ + +type clientContextKey struct{} + +type recorderContextKey struct{} -- GitLab From 36bd89a8f5d8f5faa714507ca0559b6edb4da61e Mon Sep 17 00:00:00 2001 From: Hossein Pursultani <hpursultani@gitlab.com> Date: Tue, 15 Nov 2022 16:20:21 +1100 Subject: [PATCH 3/6] Use runtime context --- controllers/gitlab_controller.go | 10 +++++++++- pkg/runtime/context.go | 2 -- pkg/support/kube/apply/options.go | 26 +++++++++++++------------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/controllers/gitlab_controller.go b/controllers/gitlab_controller.go index 4eec7e7b6..0a0dbc769 100644 --- a/controllers/gitlab_controller.go +++ b/controllers/gitlab_controller.go @@ -53,6 +53,7 @@ import ( "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/gitlab/component" feature "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/gitlab/features" "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/gitlab/status" + rt "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/runtime" "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/support/kube" "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/support/kube/apply" ) @@ -92,12 +93,19 @@ type GitLabReconciler struct { // +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;patch;delete // Reconcile triggers when an event occurs on the watched resource. +// //nolint:gocognit,gocyclo,nestif // The complexity of this method will be addressed in #260. func (r *GitLabReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Log.WithValues("gitlab", req.NamespacedName) log.Info("Reconciling GitLab") + rtCtx := rt.NewContext(). + WithLogger(log). + WithClient(r.Client). + WithEventRecorder(r.Recorder). + Build(ctx) + gitlab := &apiv1beta1.GitLab{} if err := r.Get(ctx, req.NamespacedName, gitlab); err != nil { if errors.IsNotFound(err) { @@ -108,7 +116,7 @@ func (r *GitLabReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return requeue(err) } - adapter, err := adapter.NewV1Beta1(ctx, gitlab) + adapter, err := adapter.NewV1Beta1(rtCtx, gitlab) if err != nil { return requeue(err) } diff --git a/pkg/runtime/context.go b/pkg/runtime/context.go index 2833567e5..0b0dbdf59 100644 --- a/pkg/runtime/context.go +++ b/pkg/runtime/context.go @@ -9,8 +9,6 @@ import ( // ClientFromContext returns a Client from the context or nil if client details // are not found. -// -// Use runtime ContextBuilder func ClientFromContext(ctx context.Context) client.Client { if c, ok := ctx.Value(clientContextKey{}).(client.Client); ok { return c diff --git a/pkg/support/kube/apply/options.go b/pkg/support/kube/apply/options.go index 12ffe1c0a..933f4a091 100644 --- a/pkg/support/kube/apply/options.go +++ b/pkg/support/kube/apply/options.go @@ -9,6 +9,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" + rt "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/runtime" "gitlab.com/gitlab-org/cloud-native/gitlab-operator/pkg/support/kube" ) @@ -22,6 +23,7 @@ import ( func WithClient(client client.Client) kube.ApplyOption { return func(cfg *kube.ApplyConfig) { cfg.Client = client + cfg.Scheme = client.Scheme() } } @@ -35,16 +37,21 @@ func WithCodec(codec runtime.Codec) kube.ApplyOption { } } -// WithContext configures apply with the logger and the client from the context. +// WithContext configures apply with the specified context and its associated +// logger and client if it can locate them. +// +// When the context has a client it uses its scheme to configure apply. +// +// By default ApplyObject uses a Background context. func WithContext(ctx context.Context) kube.ApplyOption { return func(cfg *kube.ApplyConfig) { cfg.Context = ctx + cfg.Logger = logr.FromContextOrDiscard(ctx) - if cfg.Client == nil { - cfg.Client = getClientFromContext(cfg.Context) + cfg.Client = rt.ClientFromContext(ctx) + if cfg.Client != nil { + cfg.Scheme = cfg.Client.Scheme() } - - cfg.Logger = logr.FromContextOrDiscard(ctx) } } @@ -70,7 +77,7 @@ func WithScheme(scheme *runtime.Scheme) kube.ApplyOption { // WithContext configures apply with the client, scheme, and logger from the // manager. // -// This is a convenient way for configuring apply. +// This is a convenient way for configuring apply for test environment. func WithManager(manager manager.Manager) kube.ApplyOption { return func(cfg *kube.ApplyConfig) { cfg.Client = manager.GetClient() @@ -78,10 +85,3 @@ func WithManager(manager manager.Manager) kube.ApplyOption { cfg.Logger = manager.GetLogger() } } - -/* Private */ - -func getClientFromContext(ctx context.Context) client.Client { - /* This is a placeholder for future implementation */ - return nil -} -- GitLab From 1d820aadc09daf8c0a571f21307d52b449801422 Mon Sep 17 00:00:00 2001 From: Hossein Pursultani <hpursultani@gitlab.com> Date: Thu, 17 Nov 2022 10:01:04 +0000 Subject: [PATCH 4/6] Apply 1 suggestion(s) to 1 file(s) --- pkg/runtime/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/runtime/builder.go b/pkg/runtime/builder.go index 6ecd6801e..b067e2371 100644 --- a/pkg/runtime/builder.go +++ b/pkg/runtime/builder.go @@ -8,7 +8,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// ContextBuilder provides an interface for building a new runtime Context. +// ContextBuilder provides a data type for describing and building a new runtime Context. type ContextBuilder struct { client client.Client logger logr.Logger -- GitLab From c6589b89df0e45ed9f9ed4718818da3c42f0be22 Mon Sep 17 00:00:00 2001 From: Hossein Pursultani <hpursultani@gitlab.com> Date: Tue, 22 Nov 2022 13:59:44 +1100 Subject: [PATCH 5/6] Use optional arguments instead of Builder pattern --- controllers/gitlab_controller.go | 9 ++--- pkg/runtime/builder.go | 67 -------------------------------- pkg/runtime/context.go | 47 ++++++++++++++++++++++ pkg/runtime/options.go | 36 +++++++++++++++++ 4 files changed, 87 insertions(+), 72 deletions(-) delete mode 100644 pkg/runtime/builder.go create mode 100644 pkg/runtime/options.go diff --git a/controllers/gitlab_controller.go b/controllers/gitlab_controller.go index 0a0dbc769..3880d5700 100644 --- a/controllers/gitlab_controller.go +++ b/controllers/gitlab_controller.go @@ -100,11 +100,10 @@ func (r *GitLabReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr log.Info("Reconciling GitLab") - rtCtx := rt.NewContext(). - WithLogger(log). - WithClient(r.Client). - WithEventRecorder(r.Recorder). - Build(ctx) + rtCtx := rt.NewContext(ctx, + rt.WithLogger(log), + rt.WithClient(r.Client), + rt.WithEventRecorder(r.Recorder)) gitlab := &apiv1beta1.GitLab{} if err := r.Get(ctx, req.NamespacedName, gitlab); err != nil { diff --git a/pkg/runtime/builder.go b/pkg/runtime/builder.go deleted file mode 100644 index b067e2371..000000000 --- a/pkg/runtime/builder.go +++ /dev/null @@ -1,67 +0,0 @@ -package runtime - -import ( - "context" - - "github.com/go-logr/logr" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// ContextBuilder provides a data type for describing and building a new runtime Context. -type ContextBuilder struct { - client client.Client - logger logr.Logger - recorder record.EventRecorder -} - -// NewContext returns a new ContextBuilder to build a new runtime Context. -func NewContext() *ContextBuilder { - return &ContextBuilder{} -} - -// WithLogger uses the specified Logger for the new runtime Context. -// -// This is compatible with logr Context. -func (b *ContextBuilder) WithLogger(logger logr.Logger) *ContextBuilder { - b.logger = logger - - return b -} - -// WithClient uses the specified Client for the new runtime Context. -// -// Generally this is the Client that the controller-runtime creates and -// is associated to the Controller. -func (b *ContextBuilder) WithClient(client client.Client) *ContextBuilder { - b.client = client - - return b -} - -// WithEventRecorder uses the specified EventRecorder for the new runtime -// Context. -func (b *ContextBuilder) WithEventRecorder(recorder record.EventRecorder) *ContextBuilder { - b.recorder = recorder - - return b -} - -// Build builds the new runtime Context and returns it. -func (b *ContextBuilder) Build(parent context.Context) context.Context { - ctx := parent - - if b.logger != nil { - ctx = logr.NewContext(ctx, b.logger) - } - - if b.client != nil { - ctx = context.WithValue(ctx, clientContextKey{}, b.client) - } - - if b.recorder != nil { - ctx = context.WithValue(ctx, recorderContextKey{}, b.recorder) - } - - return ctx -} diff --git a/pkg/runtime/context.go b/pkg/runtime/context.go index 0b0dbdf59..90beb5156 100644 --- a/pkg/runtime/context.go +++ b/pkg/runtime/context.go @@ -3,10 +3,51 @@ package runtime import ( "context" + "github.com/go-logr/logr" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" ) +// ContextConfig is the configuration that is used for building a new runtime +// Context. +type ContextConfig struct { + Client client.Client + Logger logr.Logger + Recorder record.EventRecorder +} + +// ContextOption represents an individual option of NewContext. The available +// options are: +// +// - WithClient +// - WithEventRecorder +// - WithLogger +// +// See each option for further details. +type ContextOption = func(*ContextConfig) + +// NewContext returns a new ContextBuilder to build a new runtime Context. +func NewContext(parent context.Context, options ...ContextOption) context.Context { + ctx := parent + + cfg := &ContextConfig{} + cfg.applyOptions(options) + + if cfg.Logger != nil { + ctx = logr.NewContext(ctx, cfg.Logger) + } + + if cfg.Client != nil { + ctx = context.WithValue(ctx, clientContextKey{}, cfg.Client) + } + + if cfg.Recorder != nil { + ctx = context.WithValue(ctx, recorderContextKey{}, cfg.Recorder) + } + + return ctx +} + // ClientFromContext returns a Client from the context or nil if client details // are not found. func ClientFromContext(ctx context.Context) client.Client { @@ -32,3 +73,9 @@ func RecorderFromContext(ctx context.Context) record.EventRecorder { type clientContextKey struct{} type recorderContextKey struct{} + +func (c *ContextConfig) applyOptions(options []ContextOption) { + for _, option := range options { + option(c) + } +} diff --git a/pkg/runtime/options.go b/pkg/runtime/options.go new file mode 100644 index 000000000..70412812b --- /dev/null +++ b/pkg/runtime/options.go @@ -0,0 +1,36 @@ +package runtime + +import ( + "github.com/go-logr/logr" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// WithLogger uses the specified Logger for the new runtime Context. +// +// This is compatible with logr Context. +func WithLogger(logger logr.Logger) ContextOption { + return func(cfg *ContextConfig) { + cfg.Logger = logger + } +} + +// WithClient uses the specified Client for the new runtime Context. +// +// Generally this is the Client that the controller-runtime creates and +// is associated to the Controller. +func WithClient(client client.Client) ContextOption { + return func(cfg *ContextConfig) { + cfg.Client = client + } +} + +// WithEventRecorder uses the specified EventRecorder for the new runtime +// Context. +// +// You can obtain an EventRecorder from the controller-runtime Manager. +func WithEventRecorder(recorder record.EventRecorder) ContextOption { + return func(cfg *ContextConfig) { + cfg.Recorder = recorder + } +} -- GitLab From e95bc24079ebd1c51b59ce5a2f14a75e0a6f5375 Mon Sep 17 00:00:00 2001 From: Mitchell Nielsen <mnielsen@gitlab.com> Date: Tue, 22 Nov 2022 19:27:24 +0000 Subject: [PATCH 6/6] Fix and adjust comments --- pkg/runtime/context.go | 2 +- pkg/support/kube/apply/options.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/runtime/context.go b/pkg/runtime/context.go index 90beb5156..7e50de3cf 100644 --- a/pkg/runtime/context.go +++ b/pkg/runtime/context.go @@ -26,7 +26,7 @@ type ContextConfig struct { // See each option for further details. type ContextOption = func(*ContextConfig) -// NewContext returns a new ContextBuilder to build a new runtime Context. +// NewContext returns a new Builder to build a new runtime Context. func NewContext(parent context.Context, options ...ContextOption) context.Context { ctx := parent diff --git a/pkg/support/kube/apply/options.go b/pkg/support/kube/apply/options.go index 933f4a091..ae1ad8849 100644 --- a/pkg/support/kube/apply/options.go +++ b/pkg/support/kube/apply/options.go @@ -74,10 +74,10 @@ func WithScheme(scheme *runtime.Scheme) kube.ApplyOption { } } -// WithContext configures apply with the client, scheme, and logger from the +// WithManager configures ApplyOption with the client, scheme, and logger from the // manager. // -// This is a convenient way for configuring apply for test environment. +// This is a convenient way for configuring apply for test environments. func WithManager(manager manager.Manager) kube.ApplyOption { return func(cfg *kube.ApplyConfig) { cfg.Client = manager.GetClient() -- GitLab