Update owner references in reconcile resource util function

[pebrc]

We previously set owner references only on creation as they usually don't change. With the association model in ECK ownership can change though. The referenced resource can change and the controller e.g. the Elasticsearch controller for a Kibana -> Elasticsearch relationship relies on the owner referrence in its watch on secrets.

This PR takes this into account by resetting the owner reference on updates.

Fixes #3470

[barkbay]

I was wondering why we are not reusing the logic of the ReconcileResource function:

1. NeedsUpdate() detects if owners need to be updated
2. UpdateReconciled() updates the owners

I guess this is because we want to do that for all the resources ? But is it not a bit inconsistent with the fact that NeedsUpdate() may prevent the ownership to be fixed, because it does not detect that owners needs to be updated ?

cloud-on-k8s/pkg/controller/common/reconciler/secret.go

Lines 25 to 31 in 869870f

	NeedsUpdate: func() bool {
	// update if expected labels and annotations are not there
	return !maps.IsSubset(expected.Labels, reconciled.Labels) ||
	!maps.IsSubset(expected.Annotations, reconciled.Annotations) ||
	// or if secret data is not strictly equal
	!reflect.DeepEqual(expected.Data, reconciled.Data)
	},

[barkbay]

nit: maybe move those lines a bit closer to where expectedMeta is used

[pebrc]

I was wondering why we are not reusing the logic of the ReconcileResource function:

I think that is a fair point. I guess the only problem with that approach is that we would have to remember at each use of the ReconcileResource function to include the OwnerReference in both the check whether an update needs to happen and in the actual update. My thinking was that thats somewhat inconsistent with the current signature of the function as it takes an owner as an argument which sort of indicates that it will handle the OwnerReference for the caller.

Regarding the question of whether we would miss the owner upgrade if it is not included in the NeedsUpdate function: I was kind of relying on the fact that if the owner changes this typically has an effect on other attributes as well (labels for example) which would make sure that the update happens.

I don't feel super strongly about both points and am happy to reconsider if we think an explicit inclusion of the owner reference in NeedsUpdate and UpdateReconciled is preferable.

[barkbay]

My thinking was that thats somewhat inconsistent with the current signature of the function as it takes an owner as an argument which sort of indicates that it will handle the OwnerReference for the caller.

👍 I'm fine with keeping the PR as is