3scale-ops / saas-operator

3scale SaaS Operator - www.3scale.net
Apache License 2.0
8 stars 2 forks source link

fix/canary-deployment-deletion #239

Closed roivaz closed 1 year ago

roivaz commented 1 year ago

This PR adds a generic "prune" mechanism to the basereconciler/v2 that will fix the problem of the canary resources never being deleted. The basic idea is that any resource owned by the controller that has not be explicitely passed to the ReconcileOwnedResources() function needs pruning. This behaviour can be deactivated on custom resource using the saas.3scale.net/prune=no annotation.

I also did some standardization to the logging so all controllers log in the same way, using the same fields in the structured logging.

/kind feature /kind bug /priority important-soon /assign

NOTES:

3scale-robot commented 1 year ago

@roivaz: The label(s) priority/important-sonn cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/3scale-ops/saas-operator/pull/239): >This PR adds a generic "prune" mechanism to the basereconciler/v2 that will fix the problem of the canary resources never being deleted. The basic idea is that any resource owned by the controller that has not be explicitely passed to the ReconcileOwnedResources() function needs pruning. This behaviour can be deactivated on custom resource using the `saas.3scale.net/prune=no` annotation. > >I also did some standardization to the logging so all controllers log in the same way, using the same fields in the structured logging. > >/kind feature >/kind bug >/priority important-sonn >/assign > >NOTES: > >* Due to the changes to ReconcileOwnedResources() function I had to do some changes to the TwemproxyConfig controller, which has some custom logic that the other controllers don't have. >* I did a generic GetItems() function that can return the items contained in any k8s List type. We were using a custom ExtendedObjectList for our own custom resources List types that is not necessary anymore with this function, so this code has been removed. >* There was still some code related to the operator-utils locked resources generic controller which I have removed also. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
roivaz commented 1 year ago

/hold until #238 is merged

roivaz commented 1 year ago

/unhold

3scale-robot commented 1 year ago

LGTM label has been added.

Git tree hash: 8c4253310e3e219b8f79132e9666e9260aea70ee

slopezz commented 1 year ago

/lgtm

roivaz commented 1 year ago

/approve

3scale-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: roivaz

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: - ~~[OWNERS](https://github.com/3scale-ops/saas-operator/blob/main/OWNERS)~~ [roivaz] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment