OperatorPolicy Templates #235
Conversation
JustinKuli
force-pushed
the
10858-oppol-templates
branch
from
8cc4bf1
to
8a5c568
Compare
JustinKuli
force-pushed
the
10858-oppol-templates
branch
from
8a5c568
to
45275dd
Compare
| sub, subErr := buildSubscription(policy, r.DefaultNamespace) | ||
| disableTemplates := false | ||
|
|
||
| if disableAnnotation, ok := policy.GetAnnotations()["policy.open-cluster-management.io/disable-templates"]; ok { |
There was a problem hiding this comment.
I don't like that this is an annotation instead of a spec field but I guess consistency with configuration policy is probably better than changing this at this point.
|
/hold @JustinKuli this looks great but let's block on this based on this PR sharing the dynamic watcher cache: |
| @@ -461,44 +461,6 @@ var _ = Describe("Test templatization", Ordered, func() { | |||
| return configmap | |||
| }, defaultTimeoutSeconds, 1).ShouldNot(BeNil()) | |||
|
|
|||
| By("Patch with invalid hub template") | |||
There was a problem hiding this comment.
Why is this removed?
There was a problem hiding this comment.
This is now handled in the governance-policy-framework:
open-cluster-management-io/governance-policy-framework-addon#136
|
/hold for Matt |
|
/hold for some updates to template-utils, then I will bump the version this uses to get the DeepCopy improvements (and remove some manual DeepCopy calls we were doing here) |
|
/unhold |
The framework-addon now handles emitting compliance events for those errors for all template policy types. Refs: - https://issues.redhat.com/browse/ACM-10858 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
The Subscription and OperatorGroup fields can now utilize templates. The resolver uses the DynamicWatcher already used by the controller, so no additional plumbing was needed for new watches or in order to share a cache. Refs: - https://issues.redhat.com/browse/ACM-10858 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
The library now does the DeepCopy for us, to prevent accidental mutations to objects in the cache. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
JustinKuli
force-pushed
the
10858-oppol-templates
branch
from
21f066d
to
dfb236a
Compare
|
I seriously feel like someone is playing a prank on me 😭 I half understand the "ConstraintsNotSatisfiable" failures now - the catalog was not responding (maybe because the previous test broke it on purpose), but I have no clue why it was suddenly happening very frequently... I'd believe that the quay operator version might have inconsistently removed the deprecated quayecosystems CRD, but it's still surprising that we only recently noticed it some of the time... and now the previous run had a disappearing CRD (or maybe the OLM label was removed?)... Cursed, I tell you. |
JustinKuli
force-pushed
the
10858-oppol-templates
branch
2 times, most recently
from
6055b85
to
30923ea
Compare
From some test runs, it seems like some of the 3.8.x releases do not always have the quayecosystems CRD, which makes the tests inconsistent. From what I can tell, *none* of the 3.10.x releases will have that CRD, so it should be consistent. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
This test started failing relatively often. Debugging revealed that the Subscription was getting into a ResolutionFailed=True condition, but that it did not have the expected ConstraintsNotSatisfiable reason; it looked like it was failing to resolve because the connection was refused to the catalog. Refs: - https://issues.redhat.com/browse/ACM-11414 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
The full policy status is often helpful when understanding what might be unexpectedly happening in some of the operatorpolicy tests. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
JustinKuli
force-pushed
the
10858-oppol-templates
branch
from
30923ea
to
0b74448
Compare
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli 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:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli 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:
Approvers can indicate their approval by writing |
This should help with some consistency. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
openshift-merge-bot
bot
merged commit 188daa6
into
open-cluster-management-io:main
Adds the ability to use templates in Subscription and OperatorGroup fields in an OperatorPolicy. The template resolver uses the DynamicWatcher already used by the controller, so no additional plumbing was needed for new watches or in order to share a cache.
Also removes the special handling of hub-template errors from ConfigurationPolicy - that is now handled by the framework-addon (open-cluster-management-io/governance-policy-framework-addon#136), and so that logic didn't need to be added to the OperatorPolicy controller.
For https://issues.redhat.com/browse/ACM-10858