Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

K8SPG-594 delete custom extensions from installed #967

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
bdfa932
K8SPG-594 delete custom extensions from installed
nmarukovich Dec 2, 2024
d1c9434
update extensions check
nmarukovich Dec 2, 2024
a265b86
fix checks
nmarukovich Dec 2, 2024
d370c4a
fix checks
nmarukovich Dec 2, 2024
71c00fb
delete unused
nmarukovich Dec 2, 2024
afb1f0c
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 2, 2024
6f450e1
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 3, 2024
08695c6
fix PR coments
nmarukovich Dec 4, 2024
cf29171
Merge branch 'K8SPG-594_delete_installed_ext' of github.com:percona/p…
nmarukovich Dec 4, 2024
4c6bf22
fix PR comments
nmarukovich Dec 4, 2024
b6a2f72
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 6, 2024
f232f36
fix PR
nmarukovich Dec 9, 2024
f3999bd
Merge branch 'K8SPG-594_delete_installed_ext' of github.com:percona/p…
nmarukovich Dec 9, 2024
0a2524a
delete logs
nmarukovich Dec 9, 2024
e174f7b
update conditions
nmarukovich Dec 9, 2024
41e0fb4
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 9, 2024
08e178d
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 11, 2024
a4c54f2
update annotations adding
nmarukovich Dec 12, 2024
0dc2bbd
Merge branch 'main' into K8SPG-594_delete_installed_ext
hors Dec 12, 2024
fbe03d3
use status instead of annotations
nmarukovich Dec 16, 2024
bf0818c
Merge branch 'K8SPG-594_delete_installed_ext' of github.com:percona/p…
nmarukovich Dec 16, 2024
b6ce78d
fix PR
nmarukovich Dec 17, 2024
bbf10e0
delete unused logs
nmarukovich Dec 17, 2024
061f592
fix PR comments
nmarukovich Dec 18, 2024
a74161a
fix PR comments
nmarukovich Dec 18, 2024
bd04f98
fix PR comments
nmarukovich Dec 18, 2024
f9ea789
fix test
nmarukovich Dec 18, 2024
37b3e4e
fix the test
nmarukovich Dec 19, 2024
eb26df5
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 19, 2024
c688307
fix vulnarabilities
nmarukovich Dec 19, 2024
d94deba
fix vulnarabilities
nmarukovich Dec 19, 2024
ed21ea6
update test
nmarukovich Dec 19, 2024
75a3372
fix logging
nmarukovich Dec 19, 2024
4f86a07
fix test
nmarukovich Dec 19, 2024
4f1bac0
fix PR comments
nmarukovich Dec 21, 2024
7128445
Merge branch 'main' into K8SPG-594_delete_installed_ext
nmarukovich Dec 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17494,6 +17494,10 @@ spec:
properties:
host:
type: string
installedCustomExtensions:
items:
type: string
type: array
pgbouncer:
properties:
ready:
Expand Down
2 changes: 1 addition & 1 deletion build/postgres-operator/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ ARG GO_LDFLAGS
ARG GOOS=linux
ARG TARGETARCH
ARG OPERATOR_CGO_ENABLED=1
ARG EXTENSION_INSTALLER_CGO_ENABLED=0
ARG EXTENSION_INSTALLER_CGO_ENABLED=1
ARG TARGETPLATFORM
ARG BUILDPLATFORM

Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/pgv2.percona.com_perconapgclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17900,6 +17900,10 @@ spec:
properties:
host:
type: string
installedCustomExtensions:
items:
type: string
type: array
pgbouncer:
properties:
ready:
Expand Down
4 changes: 4 additions & 0 deletions deploy/bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18193,6 +18193,10 @@ spec:
properties:
host:
type: string
installedCustomExtensions:
items:
type: string
type: array
pgbouncer:
properties:
ready:
Expand Down
4 changes: 4 additions & 0 deletions deploy/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18193,6 +18193,10 @@ spec:
properties:
host:
type: string
installedCustomExtensions:
items:
type: string
type: array
pgbouncer:
properties:
ready:
Expand Down
4 changes: 4 additions & 0 deletions deploy/cw-bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18193,6 +18193,10 @@ spec:
properties:
host:
type: string
installedCustomExtensions:
items:
type: string
type: array
pgbouncer:
properties:
ready:
Expand Down
12 changes: 12 additions & 0 deletions e2e-tests/tests/custom-extensions/09-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: kuttl.dev/v1beta1
kind: TestAssert
timeout: 30
---
kind: ConfigMap
apiVersion: v1
metadata:
name: 11-check-extensions
data:
data: |2-
pg_stat_monitor
pgaudit
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
timeout: 30
commands:
- script: |-
set -o errexit
set -o xtrace

source ../../functions

data=$(kubectl -n ${NAMESPACE} exec $(get_client_pod) -- psql -v ON_ERROR_STOP=1 -t -q postgres://postgres:$(get_psql_user_pass custom-extensions-pguser-postgres)@$(get_psql_user_host custom-extensions-pguser-postgres) -c "\c postgres" -c "select extname from pg_extension order by extname")

kubectl create configmap -n "${NAMESPACE}" 11-check-extensions --from-literal=data="${data}"
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ require (
go.opentelemetry.io/proto/otlp v1.4.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
golang.org/x/net v0.32.0 // indirect
golang.org/x/net v0.33.0 // indirect
golang.org/x/oauth2 v0.24.0 // indirect
golang.org/x/sync v0.10.0 // indirect
golang.org/x/sys v0.28.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.32.0 h1:ZqPmj8Kzc+Y6e0+skZsuACbx+wzMgo5MQsJh9Qd6aYI=
golang.org/x/net v0.32.0/go.mod h1:CwU0IoeOlnQQWJ6ioyFrfRuomB8GKF6KbYXZVyeXNfs=
golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.24.0 h1:KTBBxWqUa0ykRPLtV69rRto9TLXcqYkeswu48x/gvNE=
golang.org/x/oauth2 v0.24.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
Expand Down
91 changes: 86 additions & 5 deletions percona/controller/pgcluster/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"crypto/md5"
"fmt"
"io"
"reflect"
"strings"
"time"
Expand All @@ -29,13 +30,16 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/percona/percona-postgresql-operator/internal/controller/runtime"
"github.com/percona/percona-postgresql-operator/internal/logging"
"github.com/percona/percona-postgresql-operator/internal/naming"
"github.com/percona/percona-postgresql-operator/internal/postgres"
perconaController "github.com/percona/percona-postgresql-operator/percona/controller"
"github.com/percona/percona-postgresql-operator/percona/extensions"
"github.com/percona/percona-postgresql-operator/percona/k8s"
pNaming "github.com/percona/percona-postgresql-operator/percona/naming"
"github.com/percona/percona-postgresql-operator/percona/pmm"
common "github.com/percona/percona-postgresql-operator/percona/postgres"
"github.com/percona/percona-postgresql-operator/percona/utils/registry"
"github.com/percona/percona-postgresql-operator/percona/watcher"
v2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
Expand All @@ -49,8 +53,12 @@ const (

// Reconciler holds resources for the PerconaPGCluster reconciler
type PGClusterReconciler struct {
Client client.Client
Owner client.FieldOwner
Client client.Client
Owner client.FieldOwner
PodExec func(
ctx context.Context, namespace, pod, container string,
stdin io.Reader, stdout, stderr io.Writer, command ...string,
) error
Recorder record.EventRecorder
Tracer trace.Tracer
Platform string
Expand All @@ -65,6 +73,13 @@ type PGClusterReconciler struct {

// SetupWithManager adds the PerconaPGCluster controller to the provided runtime manager
func (r *PGClusterReconciler) SetupWithManager(mgr manager.Manager) error {
if r.PodExec == nil {
var err error
r.PodExec, err = runtime.NewPodExecutor(mgr.GetConfig())
if err != nil {
return err
}
}
if err := r.CrunchyController.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}, r.watchSecrets())); err != nil {
return errors.Wrap(err, "unable to watch secrets")
}
Expand Down Expand Up @@ -241,7 +256,9 @@ func (r *PGClusterReconciler) Reconcile(ctx context.Context, request reconcile.R
return reconcile.Result{}, errors.Wrap(err, "failed to handle monitor user password change")
}

r.reconcileCustomExtensions(cr)
if err := r.reconcileCustomExtensions(ctx, cr); err != nil {
return reconcile.Result{}, errors.Wrap(err, "reconcile custom extensions")
}

if err := r.reconcileScheduledBackups(ctx, cr); err != nil {
return reconcile.Result{}, errors.Wrap(err, "reconcile scheduled backups")
Expand Down Expand Up @@ -524,15 +541,54 @@ func (r *PGClusterReconciler) handleMonitorUserPassChange(ctx context.Context, c
return nil
}

func (r *PGClusterReconciler) reconcileCustomExtensions(cr *v2.PerconaPGCluster) {
func (r *PGClusterReconciler) reconcileCustomExtensions(ctx context.Context, cr *v2.PerconaPGCluster) error {
if cr.Spec.Extensions.Storage.Secret == nil {
return
return nil
}

extensionKeys := make([]string, 0)
extensionNames := make([]string, 0)

for _, extension := range cr.Spec.Extensions.Custom {
key := extensions.GetExtensionKey(cr.Spec.PostgresVersion, extension.Name, extension.Version)
extensionKeys = append(extensionKeys, key)
extensionNames = append(extensionNames, extension.Name)
}

if cr.CompareVersion("2.6.0") >= 0 {
nmarukovich marked this conversation as resolved.
Show resolved Hide resolved
var removedExtension []string
installedExtensions := cr.Status.InstalledCustomExtensions
crExtensions := make(map[string]struct{})
for _, ext := range extensionNames {
crExtensions[ext] = struct{}{}
}

// Check for missing entries in crExtensions
for _, ext := range installedExtensions {
// If an object exists in installedExtensions but not in crExtensions, the extension should be deleted.
if _, ok := crExtensions[ext]; !ok {
removedExtension = append(removedExtension, ext)
}
}

if len(removedExtension) > 0 {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary empty line

disabled := func(ctx context.Context, exec postgres.Executor) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disable would be a better name for this function

return errors.WithStack(disableCustomExtensionsInDB(ctx, exec, removedExtension))
}

primary, err := common.GetPrimaryPod(ctx, r.Client, cr)
if err != nil {
return errors.New("primary pod not found")
}

err = disabled(ctx, func(ctx context.Context, stdin io.Reader, stdout, stderr io.Writer, command ...string) error {
return r.PodExec(ctx, primary.Namespace, primary.Name, naming.ContainerDatabase, stdin, stdout, stderr, command...)
})
if err != nil {
return errors.Wrap(err, "deletion extension from installed")
}
}
}

for i := 0; i < len(cr.Spec.InstanceSets); i++ {
Expand All @@ -549,6 +605,31 @@ func (r *PGClusterReconciler) reconcileCustomExtensions(cr *v2.PerconaPGCluster)
))
set.VolumeMounts = append(set.VolumeMounts, extensions.ExtensionVolumeMounts(cr.Spec.PostgresVersion)...)
}
return nil
}

func disableCustomExtensionsInDB(ctx context.Context, exec postgres.Executor, customExtensionsForDeletion []string) error {
log := logging.FromContext(ctx)

for _, extensionName := range customExtensionsForDeletion {
sqlCommand := fmt.Sprintf(
`SET client_min_messages = WARNING; DROP EXTENSION IF EXISTS %s;`,
extensionName,
)
_, _, err := exec.ExecInAllDatabases(ctx,
sqlCommand,
map[string]string{
"ON_ERROR_STOP": "on", // Abort when any one command fails.
"QUIET": "on", // Do not print successful commands to stdout.
},
)

log.V(1).Info("extension was disabled ", "extensionName", extensionName)

return errors.Wrap(err, "custom extension deletion")
}

return nil
}

func isBackupRunning(ctx context.Context, cl client.Reader, cr *v2.PerconaPGCluster) (bool, error) {
Expand Down
9 changes: 7 additions & 2 deletions percona/controller/pgcluster/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package pgcluster

import (
"context"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -78,6 +77,11 @@ func (r *PGClusterReconciler) updateStatus(ctx context.Context, cr *v2.PerconaPG
return errors.Wrap(err, "get app host")
}

installedCustomExtensions := make([]string, 0)
for _, extension := range cr.Spec.Extensions.Custom {
installedCustomExtensions = append(installedCustomExtensions, extension.Name)
}

var size, ready int32
ss := make([]v2.PostgresInstanceSetStatus, 0, len(status.InstanceSets))
for _, is := range status.InstanceSets {
Expand Down Expand Up @@ -110,7 +114,8 @@ func (r *PGClusterReconciler) updateStatus(ctx context.Context, cr *v2.PerconaPG
Size: status.Proxy.PGBouncer.Replicas,
Ready: status.Proxy.PGBouncer.ReadyReplicas,
},
Host: host,
Host: host,
InstalledCustomExtensions: installedCustomExtensions,
}

cluster.Status.State = r.getState(cr, &cluster.Status, status)
Expand Down
36 changes: 36 additions & 0 deletions percona/postgres/common.go
inelpandzic marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package postgres

import (
"context"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/controller-runtime/pkg/client"

v2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
)

func GetPrimaryPod(ctx context.Context, cli client.Client, cr *v2.PerconaPGCluster) (*corev1.Pod, error) {
podList := &corev1.PodList{}
err := cli.List(ctx, podList, &client.ListOptions{
Namespace: cr.Namespace,
LabelSelector: labels.SelectorFromSet(map[string]string{
"app.kubernetes.io/instance": cr.Name,
"postgres-operator.crunchydata.com/role": "master",
}),
})
if err != nil {
return nil, err
}

if len(podList.Items) == 0 {
return nil, errors.New("no primary pod found")
}

if len(podList.Items) > 1 {
return nil, errors.New("multiple primary pods found")
}

return &podList.Items[0], nil
}
31 changes: 3 additions & 28 deletions percona/watcher/wal.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,15 @@ import (
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"

"github.com/percona/percona-postgresql-operator/internal/logging"
"github.com/percona/percona-postgresql-operator/percona/clientcmd"
"github.com/percona/percona-postgresql-operator/percona/pgbackrest"
common "github.com/percona/percona-postgresql-operator/percona/postgres"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't like this common, why it was necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need to use both "github.com/percona/percona-postgresql-operator/internal/postgres"
and "github.com/percona/percona-postgresql-operator/percona/postgres".
To keep both packages and avoid using variable, I can suggest to rename postgres package in percona directory to perconapostgres. What do you think?
@egegunes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we generally use util rather than common, maybe you could have alias here like pgUtil or something like that. Or maybe even perconaPostgres (probably too long) or maybe perconaPG?

pgv2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
)

Expand Down Expand Up @@ -153,7 +152,7 @@ func getLatestBackup(ctx context.Context, cli client.Client, cr *pgv2.PerconaPGC
func GetLatestCommitTimestamp(ctx context.Context, cli client.Client, execCli *clientcmd.Client, cr *pgv2.PerconaPGCluster, backup *pgv2.PerconaPGBackup) (*metav1.Time, error) {
log := logging.FromContext(ctx)

primary, err := getPrimaryPod(ctx, cli, cr)
primary, err := common.GetPrimaryPod(ctx, cli, cr)
if err != nil {
return nil, PrimaryPodNotFound
}
Expand Down Expand Up @@ -188,32 +187,8 @@ func GetLatestCommitTimestamp(ctx context.Context, cli client.Client, execCli *c
return &commitTsMeta, nil
}

func getPrimaryPod(ctx context.Context, cli client.Client, cr *pgv2.PerconaPGCluster) (*corev1.Pod, error) {
podList := &corev1.PodList{}
err := cli.List(ctx, podList, &client.ListOptions{
Namespace: cr.Namespace,
LabelSelector: labels.SelectorFromSet(map[string]string{
"app.kubernetes.io/instance": cr.Name,
"postgres-operator.crunchydata.com/role": "master",
}),
})
if err != nil {
return nil, err
}

if len(podList.Items) == 0 {
return nil, errors.New("no primary pod found")
}

if len(podList.Items) > 1 {
return nil, errors.New("multiple primary pods found")
}

return &podList.Items[0], nil
}

func getBackupStartTimestamp(ctx context.Context, cli client.Client, cr *pgv2.PerconaPGCluster, backup *pgv2.PerconaPGBackup) (time.Time, error) {
primary, err := getPrimaryPod(ctx, cli, cr)
primary, err := common.GetPrimaryPod(ctx, cli, cr)
if err != nil {
return time.Time{}, PrimaryPodNotFound
}
Expand Down
Loading
Loading