fix(class): Don't reconcile when harbor-class changed

[thcdrt]

Hello,

When changing Harbor custom resource annotation goharbor.io/harbor-class from class-1 to class-2, operator managing class-1 still reconcile custom resource in same time than harbor-operator managing class-2, so operators are both modifying resources in same time breaking everything.
I don't see why operator should still reconcile resources not handled by its harbor class anymore.

Thomas

[MarcelMue]

Change makes a lot of sense to me. I will fix CI before merging here. Seems like a new issue which broke it now.

[thcdrt]

Thanks @MarcelMue, yeah it looks like cert-manager installation is broken, do you want me to take a look ?

[holyhope]

Hello 👋

Are you sure that the following use-case will be handled successfully?

1. Existing harbor resource with class harbor-1.
2. Patch the harbor resource (in a single request):
   • Change the class to harbor-2
   • Update the core spec

A while ago it did not.

Other question in mind, what about sub-resources, is operator managing the class harbor-2 will be able to take ownership of resources created by the other operator?

IMO the first operator needs to be triggered so it can cleanup resources. If I remember well, the original plan was that the first operator get the patched resource at the beginning of the reconciliation loop, and delete the sub-resources (core, registry, ...) because it can not find the harbor resource for its own class.

[thcdrt]

Hello @holyhope,

Thank you for your feedback, I'll do more tests and come back to you :)

Thomas

[thcdrt]

Hello,

Sorry for the delay, been quite busy.

I made some tests and it looks like your use case is working. Do you remember why it didn't work in the past ?

About the question about subresources ownership, I only tested with subresources CRs directly so I can't answer about what happens if we change harbor class on Harbor CR. BUT so that new operator modify labels on its managed resources (deploy, secrets, cm...) I needed to modify the checksum function to take into account harbor class and not only spec (see my last commit).

Finally, concerning your last point I agree that we should have a way to clean resources managed by old operator not needed anymore by new operator but it seems it's not implemented like that for now because when changing harbor class, the handler only tries to reconcile resources, not delete them. Moreover even if it tried to delete its resources it may have conflicts because the new operator will try to create the resources with the same name.

So should we merge it it as it is even if it's not perfect it's still better that previous situation, or may we cancel it and let's see it later ?

FYI: We don't need to change Harbor class anymore, we took another solution for our use case so we don't need this feature to work on our side.

Thomas