Skip to content

Conversation

@egegunes
Copy link
Contributor

@egegunes egegunes commented Jun 1, 2022

K8SPXC-1030 Powered by Pull Request Badge

We shouldn't set owner references to cert-manager objects if we don't want to delete secrets too. This way, after the PXC cluster is deleted issuers, certificates and their secrets will remain intact in the cluster.

If users want to cleanup objects created for SSL, we introduce a new finalizer: delete-ssl. If this finalizer is set, the operator will delete secrets, certificates and issuers. Unfortunately, cert-manager doesn't set owner reference to the secret it created and this behaviour can only configured by command line flag in the controller. Since we can't control how users deploy cert-manager to their clusters, we shouldn't rely on this feature and cleanup certificates and secrets altogether. Hopefully, cert-manager/cert-manager#5158 will merged and we can configure this behaviour on certificate level.

@egegunes egegunes requested review from hors, pooknull and tplavcic June 1, 2022 07:44
@pull-request-size pull-request-size bot added the size/M 30-99 lines label Jun 1, 2022
We shouldn't set owner references to cert-manager objects if we don't
want to delete secrets too. This way, after the PXC cluster is deleted
issuers, certificates and their secrets will remain intact in the
cluster.

If users want to cleanup objects created for SSL, we introduce a new
finalizer: `delete-ssl`. If this finalizer is set, the operator will
delete secrets, certificates and issuers. Unfortunately, cert-manager
doesn't set owner reference to the secret it created and this behaviour
can only configured by command line flag in the controller. Since we
can't control how users deploy cert-manager to their clusters, we
shouldn't rely on this feature and cleanup certificates and secrets
altogether. Hopefully,
cert-manager/cert-manager#5158 will merged and
we can configure this behaviour on certificate level.
@pull-request-size pull-request-size bot added size/L 100-499 lines and removed size/M 30-99 lines labels Jun 1, 2022
tplavcic
tplavcic previously approved these changes Jun 2, 2022
Copy link
Collaborator

@hors hors left a comment

Choose a reason for hiding this comment

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

@egegunes please fix the tests and add the test case for new finalizer

@JNKPercona
Copy link
Collaborator

Test name Status
recreate passed
big-data passed
restore-to-encrypted-cluster passed
scaling passed
scheduled-backup failed
cross-site passed
haproxy passed
upgrade-haproxy passed
demand-backup passed
scaling-proxysql passed
storage passed
init-deploy passed
pitr passed
security-context passed
limits passed
self-healing passed
demand-backup-encrypted-with-tls passed
monitoring-2-0 failed
self-healing-chaos passed
self-healing-advanced passed
self-healing-advanced-chaos passed
operator-self-healing passed
operator-self-healing-chaos passed
upgrade-proxysql passed
smart-update passed
upgrade-consistency passed
affinity passed
one-pod passed
auto-tuning passed
proxysql-sidecar-res-limits passed
users passed
tls-issue-self passed
tls-issue-cert-manager passed
tls-issue-cert-manager-ref passed
validation-hook passed
proxy-protocol passed
We run 36 out of 36

commit: 658e73d
image: perconalab/percona-xtradb-cluster-operator:PR-1171-658e73df

@nmarukovich
Copy link
Contributor

with 2.28.0 pmm-client version monitoring test passed.

@hors hors requested review from hors and inelpandzic July 19, 2022 07:44
@hors hors merged commit 8b35edd into main Jul 19, 2022
@hors hors deleted the K8SPXC-1030 branch July 19, 2022 13:19
egegunes added a commit that referenced this pull request Aug 1, 2022
* K8SPXC-1030: Don't delete cert-manager certs by default

We shouldn't set owner references to cert-manager objects if we don't
want to delete secrets too. This way, after the PXC cluster is deleted
issuers, certificates and their secrets will remain intact in the
cluster.

If users want to cleanup objects created for SSL, we introduce a new
finalizer: `delete-ssl`. If this finalizer is set, the operator will
delete secrets, certificates and issuers. Unfortunately, cert-manager
doesn't set owner reference to the secret it created and this behaviour
can only configured by command line flag in the controller. Since we
can't control how users deploy cert-manager to their clusters, we
shouldn't rely on this feature and cleanup certificates and secrets
altogether. Hopefully,
cert-manager/cert-manager#5158 will merged and
we can configure this behaviour on certificate level.

* fix tests

Co-authored-by: Viacheslav Sarzhan <[email protected]>
nmarukovich pushed a commit that referenced this pull request Aug 1, 2022
* K8SPXC-1030: Don't delete cert-manager certs by default

We shouldn't set owner references to cert-manager objects if we don't
want to delete secrets too. This way, after the PXC cluster is deleted
issuers, certificates and their secrets will remain intact in the
cluster.

If users want to cleanup objects created for SSL, we introduce a new
finalizer: `delete-ssl`. If this finalizer is set, the operator will
delete secrets, certificates and issuers. Unfortunately, cert-manager
doesn't set owner reference to the secret it created and this behaviour
can only configured by command line flag in the controller. Since we
can't control how users deploy cert-manager to their clusters, we
shouldn't rely on this feature and cleanup certificates and secrets
altogether. Hopefully,
cert-manager/cert-manager#5158 will merged and
we can configure this behaviour on certificate level.

* fix tests

Co-authored-by: Viacheslav Sarzhan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L 100-499 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants