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

apply finalizers to Secret for MariaDBAccount #210

Conversation

zzzeek
Copy link
Contributor

@zzzeek zzzeek commented Mar 12, 2024

when a controller is reconciling the delete of its own CR as well as its MariaDBAccount, the lookup for MariaDBDatabase/MariaDBAccount must succeed so that db.DeleteFinalizer can be called. This uses
GetDatabaseByNameAndAccount which is required to locate all three of MariaDBDatabase, MariaDBAccount, and Secret, else notfound is returned.

therefore, prevent the Secret from being prematurely removed from the cluster by adding a calling CR finalizer to it, the same as it's added to the MariaDBAccount.

Prior to this change, the GetDatabaseByNameAndAccount can return a not-found even though the MariaDBAccount exists, causing
the calling controller to skip it and delete its own CR, leaving the MariaDBAccount and MariaDBDatabase dangling.

the issue can be reproduced by building up an openstack env with keystone / galera / memcached / rabbitmq, then deleting the namespace. k8s will delete the secrets more quickly than it can remove the CRs since they are reconciling the delete.

alternate version is to return a database/account even when secret is not found, see #209

when a controller is reconciling the delete of its own CR as well as its
MariaDBAccount, the lookup for MariaDBDatabase/MariaDBAccount must
succeed so that db.DeleteFinalizer can be called.    This uses
GetDatabaseByNameAndAccount which is required to locate all three
of MariaDBDatabase, MariaDBAccount, and Secret, else notfound is
returned.

therefore, prevent the Secret from being prematurely removed from
the cluster by adding a calling CR finalizer to it, the same as
it's added to the MariaDBAccount.

Prior to this change, the GetDatabaseByNameAndAccount can return a not-found
even though the MariaDBAccount exists, causing
the calling controller to skip it and delete its own CR, leaving
the MariaDBAccount and MariaDBDatabase dangling.

the issue can be reproduced by building up an openstack env with
keystone / galera / memcached / rabbitmq, then deleting the namespace.
k8s will delete the secrets more quickly than it can remove the CRs
since they are reconciling the delete.
Copy link
Contributor

@gibizer gibizer left a comment

Choose a reason for hiding this comment

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

Looks good to me. I prefer this over #209

return err
}

accountSecret.Immutable = &trueVal
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: accountSecret.Immutable = ptr.To(true)

@dciabrin
Copy link
Contributor

/approve

Copy link
Contributor

openshift-ci bot commented Mar 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dciabrin, gibizer, zzzeek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit f152bee into openstack-k8s-operators:main Mar 13, 2024
6 checks passed
zzzeek added a commit to zzzeek/keystone-operator that referenced this pull request Mar 13, 2024
this PR updates mariadb-operator to include [1], which will
add / remove finalizers on the Secret objects that are associated with
MariaDBAccount objects.  This ensures that GetDatabaseByNameAndAccount
will successfully locate a MariaDBDatabase/MariaDBAccount/Secret trio,
rather than returning not found, allowing ``Database.DeleteFinalizer()``
to be called when a resource is in reconcile delete.

[1] openstack-k8s-operators/mariadb-operator#210
zzzeek added a commit to zzzeek/nova-operator that referenced this pull request Mar 13, 2024
this PR updates mariadb-operator to include [1], which will add / remove
finalizers on the Secret objects that are associated with MariaDBAccount
objects.  This ensures that GetDatabaseByNameAndAccount will
successfully locate a MariaDBDatabase/MariaDBAccount/Secret trio, rather
than returning not found, allowing ``Database.DeleteFinalizer()`` to be
called when a resource is in reconcile delete.

[1] openstack-k8s-operators/mariadb-operator#210
@zzzeek zzzeek deleted the no_really_add_finalizer_to_secret branch March 13, 2024 14:09
zzzeek added a commit to zzzeek/keystone-operator that referenced this pull request Apr 2, 2024
this PR updates mariadb-operator to include [1], which will
add / remove finalizers on the Secret objects that are associated with
MariaDBAccount objects.  This ensures that GetDatabaseByNameAndAccount
will successfully locate a MariaDBDatabase/MariaDBAccount/Secret trio,
rather than returning not found, allowing ``Database.DeleteFinalizer()``
to be called when a resource is in reconcile delete.

[1] openstack-k8s-operators/mariadb-operator#210
zzzeek added a commit to zzzeek/placement-operator that referenced this pull request Apr 2, 2024
this PR updates mariadb-operator to include [1], which will add / remove
finalizers on the Secret objects that are associated with MariaDBAccount
objects. This ensures that GetDatabaseByNameAndAccount will successfully
locate a MariaDBDatabase/MariaDBAccount/Secret trio, rather than
returning not found, allowing Database.DeleteFinalizer() to be called
when a resource is in reconcile delete.

[1] openstack-k8s-operators/mariadb-operator#210
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/placement-operator that referenced this pull request Apr 3, 2024
this PR updates mariadb-operator to include [1], which will add / remove
finalizers on the Secret objects that are associated with MariaDBAccount
objects. This ensures that GetDatabaseByNameAndAccount will successfully
locate a MariaDBDatabase/MariaDBAccount/Secret trio, rather than
returning not found, allowing Database.DeleteFinalizer() to be called
when a resource is in reconcile delete.

[1] openstack-k8s-operators/mariadb-operator#210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants