-
Notifications
You must be signed in to change notification settings - Fork 56
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
base: main
Are you sure you want to change the base?
Conversation
…ercona-postgresql-operator into K8SPG-594_delete_installed_ext
…ercona-postgresql-operator into K8SPG-594_delete_installed_ext
We discussed it with @egegunes one more time and decided to move custom extensions list to status not to annotations.
|
…ercona-postgresql-operator into K8SPG-594_delete_installed_ext
|
||
if len(removedExtension) > 0 { | ||
|
||
disabled := func(ctx context.Context, exec postgres.Executor) error { |
There was a problem hiding this comment.
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
} | ||
|
||
if len(removedExtension) > 0 { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary empty line
"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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
?
commit: 7128445 |
CHANGE DESCRIPTION
Problem:
We need to delete custom extensions not only from available but from installed too. If we deleted a custom extension from available extensions it doesn't work correctly so anyway we should delete it.
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
As solution, custom annotations with the list of installed custom extensions were added.
Under each reconciliation extension either added to annotations (if it's new one and were deleted in custom extensions in CR or if it was deleted from custom extensions we should delete in from DB and from annotations too )
CHECKLIST
Jira
Needs Doc
) and QA (Needs QA
)?Tests
Config/Logging/Testability