EnMasseProject / enmasse

EnMasse - Self-service messaging on Kubernetes and OpenShift
https://enmasseproject.github.io
Apache License 2.0
190 stars 87 forks source link

Uninstalling using bundle don't delete address-space-controller #3474

Closed famarting closed 4 years ago

famarting commented 4 years ago

Describe the bug Now that address-space-controller is deployed from enmasse-operator uninstalling enmasse doing oc delete -f ... over the bundle leaves the address-space-controller deployment. This uninstalling process it's being used in systemtests and it worked before, leaving the namespace clean. To Reproduce Steps to reproduce the behavior:

  1. Install enmasse applying the bundles
  2. Run oc delete -f install/bundles/enmasse
  3. See address-space-controller remains deployed

Expected behavior Address-space-controller uninstalled

lulf commented 4 years ago

The documented way of uninstalling is: https://enmasse.io/documentation/master/kubernetes/#assembly-uninstalling-messaging

Should we update the tests to uninstall that way?

k-wall commented 4 years ago

@lulf I agree the documented process is good. However, for symmetry's sake, as the operator installs the address-space-controller, would it not be correct for the operator to organise its removal? I was wondering if this should be done using an owner reference from the operator deployment itself to the address-space-contrioller?

famarting commented 4 years ago

IMO @k-wall is right, also given that we have this issue too #2863 related to resources not deleted probably due to lack of owner references I think it's worth giving this a look. Apart from that @lulf , It's maybe correct to adapt systemtests to use the documented uninstalling process, I'm thinking of a reason not to do it but nothing comes to my mind, maybe @kornys have something to say

lulf commented 4 years ago

@k-wall @famartinrh Tying the address-space-controller to the enmasse-operator deployment via an ownerreference, requires the enmasse-operator to know about its own deployment. This also means that the enmasse-operator is required to run on the cluster itself for it to work. This circular self-reference feels like an anti-pattern to me.

I think ideally that the address-space-controller deployment would be bound to the lifetime of some CR instance, similar to consoleservice and authenticationservice. Maybe this is a 'super-CRD' as previously discussed or something else I don't know.

k-wall commented 4 years ago

I think ideally that the address-space-controller deployment would be bound to the lifetime of some CR instance, similar to consoleservice and authenticationservice. Maybe this is a 'super-CRD' as previously discussed or something else I don't know.

The idea of the relation to the super CR seems the best fit, although probably not something for now

famarting commented 4 years ago

I was thinking of the super CRD too

lulf commented 4 years ago

I suggest we close this and let this be properly addresses in 1.0 where we don't have address-space-controller.