search

artemiscloud / activemq-artemis-operator

Apache License 2.0
69 stars 63 forks source link

[#967] Operator fails to recover route/ingress when labels/host modified #972

Closed gaohoward closed 4 months ago

gaohoward commented 4 months ago

There are some issues to be decided:

  1. In route case when the host is auto generated, modifying the host during runtime won't get it recovered. Looking at https://github.com/RHsyseng/operator-utils/blob/4a2136b60a6e5bc379813515a79cb990cde235a3/pkg/resource/compare/defaults.go#L463 some other fields may also be ignored. In Ingress case as we requires IngressDomain to be defined in that case modifying host will get recovered.
  2. Annotations will be ignored as we don't defined or add any annotations by default
  3. Some other fields like those in ownerReferences are also not recovered on modification.

Question is : do we want to handle those cases and if we do, how to do it?

brusdev commented 4 months ago

@howardgao I like your approach: starting from the deployed route and setting fields that the operator cares to reconcile the ActiveMQArtemis CR spec. The operator should only restore fields that the operator cares to reconcile the ActiveMQArtemis CR spec, so no host if the field IngressHost is not defined and no annotations. The ownerReferences should be restored because the operator needs those to watch deployed resources.

gtully commented 4 months ago

The ownerReferences should be restored because the operator needs those to watch deployed resources.

the owner reference will be set on create, and we will only discover what we own, the comparison logic to determine if there is a change should ignore the ownerReference, in fact it ignores metadata, however we have had to respect metadata since we added the template to modify annotations. see calls to : https://github.com/artemiscloud/activemq-artemis-operator/blob/main/controllers/activemqartemis_reconciler.go#L1522

gaohoward commented 4 months ago

@howardgao I like your approach: starting from the deployed route and setting fields that the operator cares to reconcile the ActiveMQArtemis CR spec. The operator should only restore fields that the operator cares to reconcile the ActiveMQArtemis CR spec, so no host if the field IngressHost is not defined and no annotations. The ownerReferences should be restored because the operator needs those to watch deployed resources.

I could add updating the OwnerReference to the resources.Update() method.

gaohoward commented 4 months ago

@howardgao please ignore my previous comment, I changed my mind, I agree with @gtully

Agreed.