feat: provide option to control reconciliation when creating resources

[cam-garrison]

https://issues.redhat.com/browse/RHOAIENG-5483

Description

Previously, every manifest/template (which is not a patch) was applied only once and left without reconciliation when Manifests are created/applied by our Features.

This PR aims to give users more control over when if resources are reconciled, allowing for them to mark resources as those that should be updated on operator reconcile by marking them with the annotation:

"opendatahub.io/managed": "true" for those that should be reconciled. If the annotation is not present or is set to "false" we do not reconcile, maintaining previous behavior.

The PR also provides another more convenient way to mark that all Manifests part of a Feature should be annotated and reconciled with the new Managed() part of the builder. Simply add Managed(). to the call chain of a builder to enforce that all resources created by the Feature will be updated on reconcile. Like so:

createServiceErr := feature.CreateFeature("create-local-gw-svc").
				For(handler).
				UsingConfig(envTest.Config).
				ManifestSource(fixtures.TestEmbeddedFiles).
				Manifests(path.Join(fixtures.BaseDir, "local-gateway-svc.tmpl.yaml")).
				Managed().
				Load()

How Has This Been Tested?

See the tests added both testing the behavior of CreateResource() as well as Managed().

Also tested manually with an operator build that contained some dummy configmap manifests, one with the annotation as true and one without the annotation, and checked that after editing each and triggering a reconcile, the managed configmap was reconciled to its previous state and the non-managed configmap still contained my changes.

Merge criteria:

• The commits are squashed in a cohesive manner and have meaningful messages.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[bartoszmajsak]

Thanks for this work! That was the big missing piece to make Features more suitable for operator use cases.

A few things in this review:

• important calling Update vs Patch (server-side apply) (open for discussion, that's why I tagged @aslakknutsen). The kustomize deploy.go logic does the latter.
• some minor simplifications in the manifest logic
• remark about the focus of the tests (🏅 for writing them, my suggestion is about what is the subject of the test, but the foundation is solid!)

[bartoszmajsak]

I wonder if we don't want to make server-side apply here, WDYT @aslakknutsen ?

opendatahub-operator/pkg/deploy/deploy.go

Lines 316 to 322 in 6711ab3

	// Perform server-side apply
	data, err := json.Marshal(obj)
	if err != nil {
	return err
	}
	return cli.Patch(ctx, found, client.RawPatch(types.ApplyPatchType, data), client.ForceOwnership, client.FieldOwner(owner.GetName()))

[bartoszmajsak]

Not sure we need such a test scenario. It focuses on the "how" part (implementation detail) whereas we should be testing the behavior (if it's actually managed).

I was thinking - how about converting the entire resources_int_test.go test cases to be feature-driven with Managed being invoked or not? This is what we need to test here - and also scenario when we do not call .Managed() explicitly.

[cam-garrison]

96c7980 reworked!

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aslakknutsen, zdtsw

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 [zdtsw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cam-garrison]

/retest-required