Skip to content

Commit

Permalink
Remove deprecated manager, webhook and cluster options
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbueringer
sbueringer committed Jul 25, 2023
1 parent 52ce239 commit e92eadb
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 218 deletions.
42 changes: 4 additions & 38 deletions pkg/cluster/cluster.go
View file
Expand Up @@ -28,12 +28,11 @@ import (
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
logf "sigs.k8s.io/controller-runtime/pkg/internal/log"

"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
)

Expand Down Expand Up @@ -95,17 +94,9 @@ type Options struct {
// value only if you know what you are doing. Defaults to 10 hours if unset.
// there will a 10 percent jitter between the SyncPeriod of all controllers
// so that all controllers will not send list requests simultaneously.
SyncPeriod *time.Duration

// Namespace if specified restricts the manager's cache to watch objects in
// the desired namespace Defaults to all namespaces
//
// Note: If a namespace is specified, controllers can still Watch for a
// cluster-scoped resource (e.g Node). For namespaced resources the cache
// will only hold objects from the desired namespace.
//
// Deprecated: Use Cache.Namespaces instead.
Namespace string
// Deprecated: Use Cache.SyncPeriod instead.
SyncPeriod *time.Duration

// HTTPClient is the http client that will be used to create the default
// Cache and Client. If not set the rest.HTTPClientFor function will be used
Expand Down Expand Up @@ -141,18 +132,6 @@ type Options struct {
// Only use a custom NewClient if you know what you are doing.
NewClient client.NewClientFunc

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

// DryRunClient specifies whether the client should be configured to enforce
// dryRun mode.
//
// Deprecated: Use Client.DryRun instead.
DryRunClient bool

// EventBroadcaster records Events emitted by the manager and sends them to the Kubernetes API
// Use this to customize the event correlator and spam filter
//
Expand Down Expand Up @@ -211,9 +190,6 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
if cacheOpts.SyncPeriod == nil {
cacheOpts.SyncPeriod = options.SyncPeriod
}
if len(cacheOpts.Namespaces) == 0 && options.Namespace != "" {
cacheOpts.Namespaces = []string{options.Namespace}
}
}
cache, err := options.NewCache(config, cacheOpts)
if err != nil {
Expand All @@ -240,16 +216,6 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
if clientOpts.Cache.Reader == nil {
clientOpts.Cache.Reader = cache
}

// For backward compatibility, the ClientDisableCacheFor option should
// be appended to the DisableFor option in the client.
clientOpts.Cache.DisableFor = append(clientOpts.Cache.DisableFor, options.ClientDisableCacheFor...)

if clientOpts.DryRun == nil && options.DryRunClient {
// For backward compatibility, the DryRunClient (if set) option should override
// the DryRun option in the client (if unset).
clientOpts.DryRun = pointer.Bool(true)
}
}
clientWriter, err := options.NewClient(config, clientOpts)
if err != nil {
Expand Down
13 changes: 7 additions & 6 deletions pkg/envtest/webhook_test.go
View file
Expand Up @@ -29,6 +29,7 @@ import (
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook"
Expand All @@ -40,12 +41,12 @@ var _ = Describe("Test", func() {
Describe("Webhook", func() {
It("should reject create request for webhook that rejects all requests", func() {
m, err := manager.New(env.Config, manager.Options{
Port: env.WebhookInstallOptions.LocalServingPort,
Host: env.WebhookInstallOptions.LocalServingHost,
CertDir: env.WebhookInstallOptions.LocalServingCertDir,
TLSOpts: []func(*tls.Config){
func(config *tls.Config) {},
},
WebhookServer: webhook.NewServer(webhook.Options{
Port: env.WebhookInstallOptions.LocalServingPort,
Host: env.WebhookInstallOptions.LocalServingHost,
CertDir: env.WebhookInstallOptions.LocalServingCertDir,
TLSOpts: []func(*tls.Config){func(config *tls.Config) {}},
}),
}) // we need manager here just to leverage manager.SetFields
Expect(err).NotTo(HaveOccurred())
server := m.GetWebhookServer()
Expand Down
119 changes: 15 additions & 104 deletions pkg/manager/manager.go
View file
Expand Up @@ -18,7 +18,6 @@ package manager

import (
"context"
"crypto/tls"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -140,35 +139,6 @@ type Options struct {
// Only use a custom NewClient if you know what you are doing.
NewClient client.NewClientFunc

// SyncPeriod determines the minimum frequency at which watched resources are
// reconciled. A lower period will correct entropy more quickly, but reduce
// responsiveness to change if there are many watched resources. Change this
// value only if you know what you are doing. Defaults to 10 hours if unset.
// there will a 10 percent jitter between the SyncPeriod of all controllers
// so that all controllers will not send list requests simultaneously.
//
// This applies to all controllers.
//
// A period sync happens for two reasons:
// 1. To insure against a bug in the controller that causes an object to not
// be requeued, when it otherwise should be requeued.
// 2. To insure against an unknown bug in controller-runtime, or its dependencies,
// that causes an object to not be requeued, when it otherwise should be
// requeued, or to be removed from the queue, when it otherwise should not
// be removed.
//
// If you want
// 1. to insure against missed watch events, or
// 2. to poll services that cannot be watched,
// then we recommend that, instead of changing the default period, the
// controller requeue, with a constant duration `t`, whenever the controller
// is "done" with an object, and would otherwise not requeue it, i.e., we
// recommend the `Reconcile` function return `reconcile.Result{RequeueAfter: t}`,
// instead of `reconcile.Result{}`.
//
// Deprecated: Use Cache.SyncPeriod instead.
SyncPeriod *time.Duration

// Logger is the logger that should be used by this manager.
// If none is set, it defaults to log.Log global logger.
Logger logr.Logger
Expand Down Expand Up @@ -239,23 +209,15 @@ type Options struct {
// wait to force acquire leadership. This is measured against time of
// last observed ack. Default is 15 seconds.
LeaseDuration *time.Duration

// RenewDeadline is the duration that the acting controlplane will retry
// refreshing leadership before giving up. Default is 10 seconds.
RenewDeadline *time.Duration

// RetryPeriod is the duration the LeaderElector clients should wait
// between tries of actions. Default is 2 seconds.
RetryPeriod *time.Duration

// Namespace, if specified, restricts the manager's cache to watch objects in
// the desired namespace. Defaults to all namespaces.
//
// Note: If a namespace is specified, controllers can still Watch for a
// cluster-scoped resource (e.g Node). For namespaced resources, the cache
// will only hold objects from the desired namespace.
//
// Deprecated: Use Cache.Namespaces instead.
Namespace string

// MetricsBindAddress is the TCP address that the controller should bind to
// for serving prometheus metrics.
// It can be set to "0" to disable the metrics serving.
Expand All @@ -279,31 +241,6 @@ type Options struct {
// before exposing it to public.
PprofBindAddress string

// Port is the port that the webhook server serves at.
// It is used to set webhook.Server.Port if WebhookServer is not set.
//
// Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer.
Port int
// Host is the hostname that the webhook server binds to.
// It is used to set webhook.Server.Host if WebhookServer is not set.
//
// Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer.
Host string

// CertDir is the directory that contains the server key and certificate.
// If not set, webhook server would look up the server key and certificate in
// {TempDir}/k8s-webhook-server/serving-certs. The server key and certificate
// must be named tls.key and tls.crt, respectively.
// It is used to set webhook.Server.CertDir if WebhookServer is not set.
//
// Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer.
CertDir string

// TLSOpts is used to allow configuring the TLS config used for the webhook server.
//
// Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer.
TLSOpts []func(*tls.Config)

// WebhookServer is an externally configured webhook.Server. By default,
// a Manager will create a default server using Port, Host, and CertDir;
// if this is set, the Manager will use this server instead.
Expand All @@ -314,18 +251,6 @@ type Options struct {
// will receive a new Background Context instead.
BaseContext BaseContextFunc

// ClientDisableCacheFor tells the client that, if any cache is used, to bypass it
// for the given objects.
//
// Deprecated: Use Client.Cache.DisableCacheFor instead.
ClientDisableCacheFor []client.Object

// DryRunClient specifies whether the client should be configured to enforce
// dryRun mode.
//
// Deprecated: Use Client.DryRun instead.
DryRunClient bool

// EventBroadcaster records Events emitted by the manager and sends them to the Kubernetes API
// Use this to customize the event correlator and spam filter
//
Expand Down Expand Up @@ -401,15 +326,11 @@ func New(config *rest.Config, options Options) (Manager, error) {
clusterOptions.Scheme = options.Scheme
clusterOptions.MapperProvider = options.MapperProvider
clusterOptions.Logger = options.Logger
clusterOptions.SyncPeriod = options.SyncPeriod
clusterOptions.NewCache = options.NewCache
clusterOptions.NewClient = options.NewClient
clusterOptions.Cache = options.Cache
clusterOptions.Client = options.Client
clusterOptions.Namespace = options.Namespace //nolint:staticcheck
clusterOptions.ClientDisableCacheFor = options.ClientDisableCacheFor //nolint:staticcheck
clusterOptions.DryRunClient = options.DryRunClient //nolint:staticcheck
clusterOptions.EventBroadcaster = options.EventBroadcaster //nolint:staticcheck
clusterOptions.EventBroadcaster = options.EventBroadcaster //nolint:staticcheck
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -526,12 +447,12 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options,

o = o.setLeaderElectionConfig(newObj)

if o.SyncPeriod == nil && newObj.SyncPeriod != nil {
o.SyncPeriod = &newObj.SyncPeriod.Duration
if o.Cache.SyncPeriod == nil && newObj.SyncPeriod != nil {
o.Cache.SyncPeriod = &newObj.SyncPeriod.Duration
}

if o.Namespace == "" && newObj.CacheNamespace != "" {
o.Namespace = newObj.CacheNamespace
if len(o.Cache.Namespaces) == 0 && newObj.CacheNamespace != "" {
o.Cache.Namespaces = []string{newObj.CacheNamespace}
}

if o.MetricsBindAddress == "" && newObj.Metrics.BindAddress != "" {
Expand All @@ -550,20 +471,15 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options,
o.LivenessEndpointName = newObj.Health.LivenessEndpointName
}

if o.Port == 0 && newObj.Webhook.Port != nil {
o.Port = *newObj.Webhook.Port
}
if o.Host == "" && newObj.Webhook.Host != "" {
o.Host = newObj.Webhook.Host
}
if o.CertDir == "" && newObj.Webhook.CertDir != "" {
o.CertDir = newObj.Webhook.CertDir
}
if o.WebhookServer == nil {
port := 0
if newObj.Webhook.Port != nil {
port = *newObj.Webhook.Port
}
o.WebhookServer = webhook.NewServer(webhook.Options{
Port: o.Port,
Host: o.Host,
CertDir: o.CertDir,
Port: port,
Host: newObj.Webhook.Host,
CertDir: newObj.Webhook.CertDir,
})
}

Expand Down Expand Up @@ -737,12 +653,7 @@ func setOptionsDefaults(options Options) Options {
}

if options.WebhookServer == nil {
options.WebhookServer = webhook.NewServer(webhook.Options{
Host: options.Host,
Port: options.Port,
CertDir: options.CertDir,
TLSOpts: options.TLSOpts,
})
options.WebhookServer = webhook.NewServer(webhook.Options{})
}

return options
Expand Down
35 changes: 12 additions & 23 deletions pkg/manager/manager_test.go
View file
Expand Up @@ -156,22 +156,22 @@ var _ = Describe("manger.Manager", func() {
m, err := Options{}.AndFrom(&fakeDeferredLoader{ccfg})
Expect(err).ToNot(HaveOccurred())

Expect(*m.SyncPeriod).To(Equal(duration.Duration))
Expect(*m.Cache.SyncPeriod).To(Equal(duration.Duration))
Expect(m.LeaderElection).To(Equal(leaderElect))
Expect(m.LeaderElectionResourceLock).To(Equal("leases"))
Expect(m.LeaderElectionNamespace).To(Equal("default"))
Expect(m.LeaderElectionID).To(Equal("ctrl-lease"))
Expect(m.LeaseDuration.String()).To(Equal(duration.Duration.String()))
Expect(m.RenewDeadline.String()).To(Equal(duration.Duration.String()))
Expect(m.RetryPeriod.String()).To(Equal(duration.Duration.String()))
Expect(m.Namespace).To(Equal("default"))
Expect(m.Cache.Namespaces).To(Equal([]string{"default"}))
Expect(m.MetricsBindAddress).To(Equal(":6000"))
Expect(m.HealthProbeBindAddress).To(Equal("6060"))
Expect(m.ReadinessEndpointName).To(Equal("/readyz"))
Expect(m.LivenessEndpointName).To(Equal("/livez"))
Expect(m.Port).To(Equal(port))
Expect(m.Host).To(Equal("localhost"))
Expect(m.CertDir).To(Equal("/certs"))
Expect(m.WebhookServer.(*webhook.DefaultServer).Options.Port).To(Equal(port))
Expect(m.WebhookServer.(*webhook.DefaultServer).Options.Host).To(Equal("localhost"))
Expect(m.WebhookServer.(*webhook.DefaultServer).Options.CertDir).To(Equal("/certs"))
})

It("should be able to keep Options when cfg.ControllerManagerConfiguration set", func() {
Expand Down Expand Up @@ -213,15 +213,17 @@ var _ = Describe("manger.Manager", func() {
func(config *tls.Config) {},
}
m, err := Options{
SyncPeriod: &optDuration,
Cache: cache.Options{
SyncPeriod: &optDuration,
Namespaces: []string{"ctrl"},
},
LeaderElection: true,
LeaderElectionResourceLock: "configmaps",
LeaderElectionNamespace: "ctrl",
LeaderElectionID: "ctrl-configmap",
LeaseDuration: &optDuration,
RenewDeadline: &optDuration,
RetryPeriod: &optDuration,
Namespace: "ctrl",
MetricsBindAddress: ":7000",
HealthProbeBindAddress: "5000",
ReadinessEndpointName: "/readiness",
Expand All @@ -235,15 +237,15 @@ var _ = Describe("manger.Manager", func() {
}.AndFrom(&fakeDeferredLoader{ccfg})
Expect(err).ToNot(HaveOccurred())

Expect(m.SyncPeriod.String()).To(Equal(optDuration.String()))
Expect(m.Cache.SyncPeriod.String()).To(Equal(optDuration.String()))
Expect(m.LeaderElection).To(BeTrue())
Expect(m.LeaderElectionResourceLock).To(Equal("configmaps"))
Expect(m.LeaderElectionNamespace).To(Equal("ctrl"))
Expect(m.LeaderElectionID).To(Equal("ctrl-configmap"))
Expect(m.LeaseDuration.String()).To(Equal(optDuration.String()))
Expect(m.RenewDeadline.String()).To(Equal(optDuration.String()))
Expect(m.RetryPeriod.String()).To(Equal(optDuration.String()))
Expect(m.Namespace).To(Equal("ctrl"))
Expect(m.Cache.Namespaces).To(Equal([]string{"ctrl"}))
Expect(m.MetricsBindAddress).To(Equal(":7000"))
Expect(m.HealthProbeBindAddress).To(Equal("5000"))
Expect(m.ReadinessEndpointName).To(Equal("/readiness"))
Expand All @@ -267,23 +269,10 @@ var _ = Describe("manger.Manager", func() {
Expect(svr.(*webhook.DefaultServer).Options.Host).To(Equal("foo.com"))
})

It("should lazily initialize a webhook server if needed (deprecated)", func() {
By("creating a manager with options")
m, err := New(cfg, Options{Port: 9440, Host: "foo.com"})
Expect(err).NotTo(HaveOccurred())
Expect(m).NotTo(BeNil())

By("checking options are passed to the webhook server")
svr := m.GetWebhookServer()
Expect(svr).NotTo(BeNil())
Expect(svr.(*webhook.DefaultServer).Options.Port).To(Equal(9440))
Expect(svr.(*webhook.DefaultServer).Options.Host).To(Equal("foo.com"))
})

It("should not initialize a webhook server if Options.WebhookServer is set", func() {
By("creating a manager with options")
srv := webhook.NewServer(webhook.Options{Port: 9440})
m, err := New(cfg, Options{Port: 9441, WebhookServer: srv})
m, err := New(cfg, Options{WebhookServer: srv})
Expect(err).NotTo(HaveOccurred())
Expect(m).NotTo(BeNil())

Expand Down

0 comments on commit e92eadb

Please sign in to comment.