From 4784e18cebfecc92db2d260c6bf4b160da401e79 Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Fri, 18 Apr 2025 15:13:10 +0300 Subject: [PATCH 1/4] K8SPG-761: add support for the `PGO_WORKERS` env var https://perconadev.atlassian.net/browse/K8SPG-761 --- cmd/postgres-operator/main.go | 11 ++---- cmd/postgres-operator/main_test.go | 16 ++++---- deploy/bundle.yaml | 2 + deploy/cw-operator.yaml | 2 + deploy/operator.yaml | 2 + percona/runtime/runtime.go | 62 +++++++++++------------------- 6 files changed, 41 insertions(+), 54 deletions(-) diff --git a/cmd/postgres-operator/main.go b/cmd/postgres-operator/main.go index ad7ffadb88..b44a5219b5 100644 --- a/cmd/postgres-operator/main.go +++ b/cmd/postgres-operator/main.go @@ -41,7 +41,6 @@ import ( "github.com/percona/percona-postgresql-operator/percona/controller/pgcluster" "github.com/percona/percona-postgresql-operator/percona/controller/pgrestore" perconaPGUpgrade "github.com/percona/percona-postgresql-operator/percona/controller/pgupgrade" - "github.com/percona/percona-postgresql-operator/percona/k8s" perconaRuntime "github.com/percona/percona-postgresql-operator/percona/runtime" "github.com/percona/percona-postgresql-operator/percona/utils/registry" v2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2" @@ -111,15 +110,13 @@ func main() { // deprecation warnings when using an older version of a resource for backwards compatibility). rest.SetDefaultWarningHandler(rest.NoWarnings{}) - namespaces, err := k8s.GetWatchNamespace() + options, err := initManager(ctx) assertNoError(err) mgr, err := perconaRuntime.CreateRuntimeManager( - namespaces, cfg, - false, - false, features, + options, ) assertNoError(err) @@ -260,8 +257,8 @@ func addControllersToManager(ctx context.Context, mgr manager.Manager) error { //+kubebuilder:rbac:groups="coordination.k8s.io",resources="leases",verbs={get,create,update} -func initManager() (runtime.Options, error) { - log := logging.FromContext(context.Background()) +func initManager(ctx context.Context) (runtime.Options, error) { + log := logging.FromContext(ctx) options := runtime.Options{} options.Cache.SyncPeriod = initialize.Pointer(time.Hour) diff --git a/cmd/postgres-operator/main_test.go b/cmd/postgres-operator/main_test.go index f369ce6bd3..ff83f30dd7 100644 --- a/cmd/postgres-operator/main_test.go +++ b/cmd/postgres-operator/main_test.go @@ -5,6 +5,7 @@ package main import ( + "context" "reflect" "testing" "time" @@ -14,8 +15,9 @@ import ( ) func TestInitManager(t *testing.T) { + ctx := context.Background() t.Run("Defaults", func(t *testing.T) { - options, err := initManager() + options, err := initManager(ctx) assert.NilError(t, err) if assert.Check(t, options.Cache.SyncPeriod != nil) { @@ -48,7 +50,7 @@ func TestInitManager(t *testing.T) { t.Run("Invalid", func(t *testing.T) { t.Setenv("PGO_CONTROLLER_LEASE_NAME", "INVALID_NAME") - options, err := initManager() + options, err := initManager(ctx) assert.ErrorContains(t, err, "PGO_CONTROLLER_LEASE_NAME") assert.ErrorContains(t, err, "invalid") @@ -59,7 +61,7 @@ func TestInitManager(t *testing.T) { t.Run("Valid", func(t *testing.T) { t.Setenv("PGO_CONTROLLER_LEASE_NAME", "valid-name") - options, err := initManager() + options, err := initManager(ctx) assert.NilError(t, err) assert.Assert(t, options.LeaderElection == true) assert.Equal(t, options.LeaderElectionNamespace, "test-namespace") @@ -70,7 +72,7 @@ func TestInitManager(t *testing.T) { t.Run("PGO_TARGET_NAMESPACE", func(t *testing.T) { t.Setenv("PGO_TARGET_NAMESPACE", "some-such") - options, err := initManager() + options, err := initManager(ctx) assert.NilError(t, err) assert.Assert(t, cmp.Len(options.Cache.DefaultNamespaces, 1), "expected only one configured namespace") @@ -81,7 +83,7 @@ func TestInitManager(t *testing.T) { t.Run("PGO_TARGET_NAMESPACES", func(t *testing.T) { t.Setenv("PGO_TARGET_NAMESPACES", "some-such,another-one") - options, err := initManager() + options, err := initManager(ctx) assert.NilError(t, err) assert.Assert(t, cmp.Len(options.Cache.DefaultNamespaces, 2), "expect two configured namespaces") @@ -95,7 +97,7 @@ func TestInitManager(t *testing.T) { for _, v := range []string{"-3", "0", "3.14"} { t.Setenv("PGO_WORKERS", v) - options, err := initManager() + options, err := initManager(ctx) assert.NilError(t, err) assert.DeepEqual(t, options.Controller.GroupKindConcurrency, map[string]int{ @@ -107,7 +109,7 @@ func TestInitManager(t *testing.T) { t.Run("Valid", func(t *testing.T) { t.Setenv("PGO_WORKERS", "19") - options, err := initManager() + options, err := initManager(ctx) assert.NilError(t, err) assert.DeepEqual(t, options.Controller.GroupKindConcurrency, map[string]int{ diff --git a/deploy/bundle.yaml b/deploy/bundle.yaml index 64cdb850d0..0768919028 100644 --- a/deploy/bundle.yaml +++ b/deploy/bundle.yaml @@ -47675,6 +47675,8 @@ spec: value: INFO - name: DISABLE_TELEMETRY value: "false" + - name: PGO_WORKERS + value: "1" image: perconalab/percona-postgresql-operator:main imagePullPolicy: Always livenessProbe: diff --git a/deploy/cw-operator.yaml b/deploy/cw-operator.yaml index b7fc3785ea..90232d8f7d 100644 --- a/deploy/cw-operator.yaml +++ b/deploy/cw-operator.yaml @@ -42,6 +42,8 @@ spec: value: INFO - name: DISABLE_TELEMETRY value: "false" + - name: PGO_WORKERS + value: "1" image: perconalab/percona-postgresql-operator:main imagePullPolicy: Always livenessProbe: diff --git a/deploy/operator.yaml b/deploy/operator.yaml index 37959623a3..24e1f7081e 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -45,6 +45,8 @@ spec: value: INFO - name: DISABLE_TELEMETRY value: "false" + - name: PGO_WORKERS + value: "1" image: perconalab/percona-postgresql-operator:main imagePullPolicy: Always livenessProbe: diff --git a/percona/runtime/runtime.go b/percona/runtime/runtime.go index a6e8efe39d..089c0d33f7 100644 --- a/percona/runtime/runtime.go +++ b/percona/runtime/runtime.go @@ -6,43 +6,36 @@ import ( "time" "k8s.io/client-go/rest" - "k8s.io/client-go/util/flowcontrol" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/manager" - metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server" r "github.com/percona/percona-postgresql-operator/internal/controller/runtime" "github.com/percona/percona-postgresql-operator/internal/feature" + "github.com/percona/percona-postgresql-operator/internal/initialize" + "github.com/percona/percona-postgresql-operator/percona/k8s" ) // default refresh interval in minutes -var refreshInterval = 60 * time.Minute +const refreshInterval time.Duration = 60 * time.Minute const electionID string = "08db3feb.percona.com" -// CreateRuntimeManager does the same thing as `internal/controller/runtime.CreateRuntimeManager`, -// excet it configures the manager to watch multiple namespaces. -func CreateRuntimeManager(namespaces string, config *rest.Config, disableMetrics, disableLeaderElection bool, features feature.MutableGate) (manager.Manager, error) { - - var leaderElectionID string - if !disableLeaderElection { - leaderElectionID = electionID +// CreateRuntimeManager wraps internal/controller/runtime.NewManager and modifies the given options: +// - Fully overwrites the Cache field +// - Sets Cache.SyncPeriod to refreshInterval const +// - Sets Cache.DefaultNamespaces by using k8s.GetWatchNamespace() split by "," +// - Sets LeaderElection to true +// - Sets LeaderElectionID to the electionID const +// - Sets BaseContext to include the provided feature gates +func CreateRuntimeManager(config *rest.Config, features feature.MutableGate, options manager.Options) (manager.Manager, error) { + namespaces, err := k8s.GetWatchNamespace() + if err != nil { + return nil, err } - options := manager.Options{ - Cache: cache.Options{ - SyncPeriod: &refreshInterval, - }, - Scheme: r.Scheme, - LeaderElection: !disableLeaderElection, - LeaderElectionID: leaderElectionID, + options.Cache = cache.Options{ + SyncPeriod: initialize.Pointer(refreshInterval), } - - options.BaseContext = func() context.Context { - ctx := context.Background() - return feature.NewContext(ctx, features) - } - nn := strings.Split(namespaces, ",") if len(nn) > 0 && nn[0] != "" { namespaces := make(map[string]cache.Config) @@ -52,24 +45,13 @@ func CreateRuntimeManager(namespaces string, config *rest.Config, disableMetrics options.Cache.DefaultNamespaces = namespaces } - if disableMetrics { - options.HealthProbeBindAddress = "0" - options.Metrics = metricsServer.Options{ - BindAddress: "0", - } - } - - // Create a copy of the config to avoid modifying the original - configCopy := rest.CopyConfig(config) + options.LeaderElection = true + options.LeaderElectionID = electionID - // Ensure throttling is disabled by setting a fake rate limiter - configCopy.RateLimiter = flowcontrol.NewFakeAlwaysRateLimiter() - - // create controller runtime manager - mgr, err := manager.New(configCopy, options) - if err != nil { - return nil, err + options.BaseContext = func() context.Context { + ctx := context.Background() + return feature.NewContext(ctx, features) } - return mgr, nil + return r.NewManager(config, options) } From 84fdf44c3c1a7bf72fd131e66beaf116daab963c Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Fri, 18 Apr 2025 17:14:20 +0300 Subject: [PATCH 2/4] fix unit-tests --- cmd/postgres-operator/main.go | 3 +++ config/manager/default/manager.yaml | 2 ++ deploy/cw-bundle.yaml | 2 ++ percona/controller/pgcluster/controller_test.go | 13 ++++++++++++- percona/runtime/runtime.go | 7 +------ 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/cmd/postgres-operator/main.go b/cmd/postgres-operator/main.go index b44a5219b5..0a64be57c6 100644 --- a/cmd/postgres-operator/main.go +++ b/cmd/postgres-operator/main.go @@ -113,6 +113,9 @@ func main() { options, err := initManager(ctx) assertNoError(err) + options.LeaderElection = true + options.LeaderElectionID = perconaRuntime.ElectionID + mgr, err := perconaRuntime.CreateRuntimeManager( cfg, features, diff --git a/config/manager/default/manager.yaml b/config/manager/default/manager.yaml index 21e536517e..bc132a43cd 100644 --- a/config/manager/default/manager.yaml +++ b/config/manager/default/manager.yaml @@ -35,6 +35,8 @@ spec: value: INFO - name: DISABLE_TELEMETRY value: "false" + - name: PGO_WORKERS + value: "1" ports: - containerPort: 8080 name: metrics diff --git a/deploy/cw-bundle.yaml b/deploy/cw-bundle.yaml index 3fbb77be04..0c16506f0d 100644 --- a/deploy/cw-bundle.yaml +++ b/deploy/cw-bundle.yaml @@ -47673,6 +47673,8 @@ spec: value: INFO - name: DISABLE_TELEMETRY value: "false" + - name: PGO_WORKERS + value: "1" image: perconalab/percona-postgresql-operator:main imagePullPolicy: Always livenessProbe: diff --git a/percona/controller/pgcluster/controller_test.go b/percona/controller/pgcluster/controller_test.go index 74689b22cb..c015406287 100644 --- a/percona/controller/pgcluster/controller_test.go +++ b/percona/controller/pgcluster/controller_test.go @@ -7,6 +7,7 @@ import ( "context" "crypto/md5" //nolint:gosec "fmt" + "os" "strconv" "sync" "time" @@ -24,8 +25,10 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server" "github.com/percona/percona-postgresql-operator/internal/controller/postgrescluster" + internalRuntime "github.com/percona/percona-postgresql-operator/internal/controller/runtime" "github.com/percona/percona-postgresql-operator/internal/feature" "github.com/percona/percona-postgresql-operator/internal/naming" perconaController "github.com/percona/percona-postgresql-operator/percona/controller" @@ -539,7 +542,15 @@ var _ = Describe("Watching secrets", Ordered, func() { Expect(err).NotTo(HaveOccurred()) Expect(err).To(Not(HaveOccurred())) - mgr, err := runtime.CreateRuntimeManager(namespace.Name, cfg, true, true, gate) + + os.Setenv("PGO_TARGET_NAMESPACE", "") + mgr, err := runtime.CreateRuntimeManager(cfg, gate, internalRuntime.Options{ + LeaderElection: false, + HealthProbeBindAddress: "0", + Metrics: metricsServer.Options{ + BindAddress: "0", + }, + }) Expect(err).To(Succeed()) Expect(v2.AddToScheme(mgr.GetScheme())).To(Succeed()) diff --git a/percona/runtime/runtime.go b/percona/runtime/runtime.go index 089c0d33f7..cc2df3e008 100644 --- a/percona/runtime/runtime.go +++ b/percona/runtime/runtime.go @@ -18,14 +18,12 @@ import ( // default refresh interval in minutes const refreshInterval time.Duration = 60 * time.Minute -const electionID string = "08db3feb.percona.com" +const ElectionID string = "08db3feb.percona.com" // CreateRuntimeManager wraps internal/controller/runtime.NewManager and modifies the given options: // - Fully overwrites the Cache field // - Sets Cache.SyncPeriod to refreshInterval const // - Sets Cache.DefaultNamespaces by using k8s.GetWatchNamespace() split by "," -// - Sets LeaderElection to true -// - Sets LeaderElectionID to the electionID const // - Sets BaseContext to include the provided feature gates func CreateRuntimeManager(config *rest.Config, features feature.MutableGate, options manager.Options) (manager.Manager, error) { namespaces, err := k8s.GetWatchNamespace() @@ -45,9 +43,6 @@ func CreateRuntimeManager(config *rest.Config, features feature.MutableGate, opt options.Cache.DefaultNamespaces = namespaces } - options.LeaderElection = true - options.LeaderElectionID = electionID - options.BaseContext = func() context.Context { ctx := context.Background() return feature.NewContext(ctx, features) From 0434c3997413cf07a63d163fbb371f4ca1a33714 Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Mon, 21 Apr 2025 12:19:26 +0300 Subject: [PATCH 3/4] fix --- cmd/postgres-operator/main.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/postgres-operator/main.go b/cmd/postgres-operator/main.go index 0a64be57c6..a682680dfa 100644 --- a/cmd/postgres-operator/main.go +++ b/cmd/postgres-operator/main.go @@ -113,9 +113,6 @@ func main() { options, err := initManager(ctx) assertNoError(err) - options.LeaderElection = true - options.LeaderElectionID = perconaRuntime.ElectionID - mgr, err := perconaRuntime.CreateRuntimeManager( cfg, features, @@ -318,6 +315,10 @@ func initManager(ctx context.Context) (runtime.Options, error) { } } + // K8SPG-761 + options.LeaderElection = true + options.LeaderElectionID = perconaRuntime.ElectionID + return options, nil } From 30a70f5b2d1f9c28b7b70b10e4fe70fc86af07de Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Mon, 21 Apr 2025 13:08:02 +0300 Subject: [PATCH 4/4] fix unit-test --- cmd/postgres-operator/main.go | 8 ++++---- cmd/postgres-operator/main_test.go | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/postgres-operator/main.go b/cmd/postgres-operator/main.go index a682680dfa..44bbd39fc6 100644 --- a/cmd/postgres-operator/main.go +++ b/cmd/postgres-operator/main.go @@ -276,6 +276,10 @@ func initManager(ctx context.Context) (runtime.Options, error) { options.LeaderElection = true options.LeaderElectionID = lease options.LeaderElectionNamespace = os.Getenv("PGO_NAMESPACE") + } else { + // K8SPG-761 + options.LeaderElection = true + options.LeaderElectionID = perconaRuntime.ElectionID } // Check PGO_TARGET_NAMESPACE for backwards compatibility with @@ -315,10 +319,6 @@ func initManager(ctx context.Context) (runtime.Options, error) { } } - // K8SPG-761 - options.LeaderElection = true - options.LeaderElectionID = perconaRuntime.ElectionID - return options, nil } diff --git a/cmd/postgres-operator/main_test.go b/cmd/postgres-operator/main_test.go index ff83f30dd7..9235b7d2c2 100644 --- a/cmd/postgres-operator/main_test.go +++ b/cmd/postgres-operator/main_test.go @@ -32,12 +32,14 @@ func TestInitManager(t *testing.T) { }) assert.Assert(t, options.Cache.DefaultNamespaces == nil) - assert.Assert(t, options.LeaderElection == false) + assert.Assert(t, options.LeaderElection == true) { options.Cache.SyncPeriod = nil options.Controller.GroupKindConcurrency = nil options.HealthProbeBindAddress = "" + options.LeaderElection = false + options.LeaderElectionID = "" assert.Assert(t, reflect.ValueOf(options).IsZero(), "expected remaining fields to be unset:\n%+v", options)