Skip to content

Commit

Permalink
[operator] Clean up New functions (#538)
Browse files Browse the repository at this point in the history
Reduce number of New functions in the operator package to a singular New
for each type with an options struct. Future new functionality for any
type in the operator package can now be added to the options struct for
that type, without needing additional New methods, or breaking the
signature of an existing New method.

Relates to #454
  • Loading branch information
IfSentient authored Dec 11, 2024
1 parent 5f3f96a commit 389e0f7
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 26 deletions.
2 changes: 1 addition & 1 deletion examples/operator/opinionated/reconciler/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func main() {
informerController := operator.NewInformerController(operator.DefaultInformerControllerConfig())

// Create an informer for the schema to watch all namespaces.
informer, err := operator.NewKubernetesBasedInformer(kind, client, "")
informer, err := operator.NewKubernetesBasedInformer(kind, client, operator.KubernetesBasedInformerOptions{})
if err != nil {
panic(fmt.Errorf("unable to create controller: %w", err))
}
Expand Down
2 changes: 1 addition & 1 deletion examples/operator/opinionated/watcher/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func main() {
}

// Create an informer for the schema to watch all namespaces.
informer, err := operator.NewKubernetesBasedInformer(kind, client, "")
informer, err := operator.NewKubernetesBasedInformer(kind, client, operator.KubernetesBasedInformerOptions{})
if err != nil {
panic(fmt.Errorf("unable to create controller: %w", err))
}
Expand Down
22 changes: 21 additions & 1 deletion operator/informer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ type retryInfo struct {
// InformerControllerConfig contains configuration options for an InformerController
type InformerControllerConfig struct {
MetricsConfig metrics.Config
// ErrorHandler is a user-specified error handling function. This is typically for logging/metrics use,
// as retry logic is covered by the RetryPolicy. If left nil, DefaultErrorHandler will be used.
ErrorHandler func(context.Context, error)
// RetryPolicy is a user-specified retry logic function which will be used when ResourceWatcher function calls fail.
// If left nil, DefaultRetryPolicy will be used.
RetryPolicy RetryPolicy
// RetryDequeuePolicy is a user-specified retry dequeue logic function which will be used for new informer actions
// when one or more retries for the object are still pending. If not present, existing retries are always dequeued.
// If left nil, no RetryDequeuePolicy will be used, and retries will only be dequeued when RetryPolicy returns false.
RetryDequeuePolicy RetryDequeuePolicy
}

// DefaultInformerControllerConfig returns an InformerControllerConfig with default values
Expand All @@ -142,7 +152,7 @@ func DefaultInformerControllerConfig() InformerControllerConfig {

// NewInformerController creates a new controller
func NewInformerController(cfg InformerControllerConfig) *InformerController {
return &InformerController{
inf := &InformerController{
RetryPolicy: DefaultRetryPolicy,
ErrorHandler: DefaultErrorHandler,
informers: NewListMap[Informer](),
Expand Down Expand Up @@ -198,6 +208,16 @@ func NewInformerController(cfg InformerControllerConfig) *InformerController {
Help: "Current number of events which have active reconcile processes",
}, []string{"event_type", "kind"}),
}
if cfg.ErrorHandler != nil {
inf.ErrorHandler = cfg.ErrorHandler
}
if cfg.RetryPolicy != nil {
inf.RetryPolicy = cfg.RetryPolicy
}
if cfg.RetryDequeuePolicy != nil {
inf.RetryDequeuePolicy = cfg.RetryDequeuePolicy
}
return inf
}

// AddInformer adds an informer for a specific resourceKind.
Expand Down
19 changes: 10 additions & 9 deletions operator/informer_customcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,26 @@ type CustomCacheInformer struct {
runContext context.Context
}

// NewMemcachedInformer creates a new CustomCacheInformer which uses memcached as its custom cache.
// This is analogous to calling NewCustomCacheInformer with a MemcachedStore as the store.
func NewMemcachedInformer(kind resource.Kind, client ListWatchClient, namespace string, addrs ...string) (*CustomCacheInformer, error) {
return NewMemcachedInformerWithFilters(kind, client, ListWatchOptions{Namespace: namespace}, addrs...)
type MemcachedInformerOptions struct {
Addrs []string
ListWatchOptions ListWatchOptions
}

// NewMemcachedInformerWithFilters creates a new CustomCacheInformer which uses memcached as its custom cache.
// This is analogous to calling NewCustomCacheInformer with a MemcachedStore as the store.
func NewMemcachedInformerWithFilters(kind resource.Kind, client ListWatchClient, filterOptions ListWatchOptions, addrs ...string) (*CustomCacheInformer, error) {
// NewMemcachedInformer creates a new CustomCacheInformer which uses memcached as its custom cache.
// This is analogous to calling NewCustomCacheInformer with a MemcachedStore as the store, using the default memcached options.
// To set additional memcached options, use NewCustomCacheInformer and NewMemcachedStore.
func NewMemcachedInformer(kind resource.Kind, client ListWatchClient, opts MemcachedInformerOptions) (*CustomCacheInformer, error) {
c, err := NewMemcachedStore(kind, MemcachedStoreConfig{
Addrs: addrs,
Addrs: opts.Addrs,
})
if err != nil {
return nil, err
}
return NewCustomCacheInformer(c, NewListerWatcher(client, kind, filterOptions), kind), nil
return NewCustomCacheInformer(c, NewListerWatcher(client, kind, opts.ListWatchOptions), kind), nil
}

// NewCustomCacheInformer returns a new CustomCacheInformer using the provided cache.Store and cache.ListerWatcher.
// To use ListWatchOptions, use NewListerWatcher to get a cache.ListerWatcher.
func NewCustomCacheInformer(store cache.Store, lw cache.ListerWatcher, kind resource.Kind) *CustomCacheInformer {
return &CustomCacheInformer{
store: store,
Expand Down
14 changes: 3 additions & 11 deletions operator/informer_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,17 @@ type KubernetesBasedInformer struct {
}

type KubernetesBasedInformerOptions struct {
// ListWatchOptions are the options for filtering the watch based on namespace and other compatible filters.
ListWatchOptions ListWatchOptions
// CacheResyncInterval is the interval at which the informer will emit CacheResync events for all resources in the cache.
// This is distinct from a full resync, as no information is fetched from the API server.
// An empty value will disable cache resyncs.
CacheResyncInterval time.Duration
}

// NewKubernetesBasedInformer creates a new KubernetesBasedInformer for the provided schema and namespace,
// using the ListWatchClient provided to do its List and Watch requests.
func NewKubernetesBasedInformer(sch resource.Kind, client ListWatchClient, namespace string) (
*KubernetesBasedInformer, error) {
return NewKubernetesBasedInformerWithFilters(sch, client, KubernetesBasedInformerOptions{
ListWatchOptions: ListWatchOptions{Namespace: namespace},
})
}

// NewKubernetesBasedInformerWithFilters creates a new KubernetesBasedInformer for the provided schema and namespace,
// NewKubernetesBasedInformer creates a new KubernetesBasedInformer for the provided kind and options,
// using the ListWatchClient provided to do its List and Watch requests applying provided labelFilters if it is not empty.
func NewKubernetesBasedInformerWithFilters(sch resource.Kind, client ListWatchClient, options KubernetesBasedInformerOptions) (
func NewKubernetesBasedInformer(sch resource.Kind, client ListWatchClient, options KubernetesBasedInformerOptions) (
*KubernetesBasedInformer, error) {
if client == nil {
return nil, fmt.Errorf("client cannot be nil")
Expand Down
2 changes: 1 addition & 1 deletion simple/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (a *App) watchKind(kind AppUnmanagedKind) error {
if err != nil {
return err
}
inf, err := operator.NewKubernetesBasedInformerWithFilters(kind.Kind, client, operator.KubernetesBasedInformerOptions{
inf, err := operator.NewKubernetesBasedInformer(kind.Kind, client, operator.KubernetesBasedInformerOptions{
ListWatchOptions: operator.ListWatchOptions{
Namespace: kind.ReconcileOptions.Namespace,
LabelFilters: kind.ReconcileOptions.LabelFilters,
Expand Down
4 changes: 2 additions & 2 deletions simple/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (o *Operator) WatchKind(kind resource.Kind, watcher SyncWatcher, options op
if err != nil {
return err
}
inf, err := operator.NewKubernetesBasedInformerWithFilters(kind, client, operator.KubernetesBasedInformerOptions{
inf, err := operator.NewKubernetesBasedInformer(kind, client, operator.KubernetesBasedInformerOptions{
ListWatchOptions: operator.ListWatchOptions{Namespace: options.Namespace, LabelFilters: options.LabelFilters, FieldSelectors: options.FieldSelectors},
CacheResyncInterval: o.cacheResyncInterval,
})
Expand Down Expand Up @@ -242,7 +242,7 @@ func (o *Operator) ReconcileKind(kind resource.Kind, reconciler operator.Reconci
if err != nil {
return err
}
inf, err := operator.NewKubernetesBasedInformerWithFilters(kind, client, operator.KubernetesBasedInformerOptions{
inf, err := operator.NewKubernetesBasedInformer(kind, client, operator.KubernetesBasedInformerOptions{
ListWatchOptions: operator.ListWatchOptions{Namespace: options.Namespace, LabelFilters: options.LabelFilters, FieldSelectors: options.FieldSelectors},
CacheResyncInterval: o.cacheResyncInterval,
})
Expand Down

0 comments on commit 389e0f7

Please sign in to comment.