From d1690c7f6f351512d845af30055bf18e4af544f7 Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Mon, 10 Jun 2024 10:22:08 -0400 Subject: [PATCH 1/2] Limit overlap detection to the same cluster Otherwise, hosted mode runs into problems where it believes there will be an overlap, but actually the policies are for different clusters. Refs: - https://issues.redhat.com/browse/ACM-12032 Signed-off-by: Justin Kulikauskas (cherry picked from commit 04184bc9aee106d6af3b02d9a86acdd03d7936c3) --- controllers/operatorpolicy_controller.go | 11 ++++++++++- .../operator-policy-all-defaults.yaml | 3 +++ .../operator-policy-argocd.yaml | 3 +++ .../operator-policy-authorino.yaml | 3 +++ .../operator-policy-defaults-invalid-source.yaml | 3 +++ .../operator-policy-manual-upgrades.yaml | 3 +++ .../operator-policy-mustnothave-any-version.yaml | 3 +++ .../operator-policy-mustnothave.yaml | 3 +++ .../operator-policy-no-exist-enforce.yaml | 3 +++ .../operator-policy-no-group-csv-fail.yaml | 3 +++ .../operator-policy-no-group-enforce-one-version.yaml | 3 +++ .../operator-policy-no-group-enforce.yaml | 3 +++ .../operator-policy-no-group-one-version.yaml | 3 +++ .../operator-policy-no-group.yaml | 3 +++ .../operator-policy-strimzi-kafka-operator.yaml | 3 +++ .../operator-policy-validity-test.yaml | 3 +++ .../operator-policy-with-group.yaml | 3 +++ .../operator-policy-with-templates.yaml | 3 +++ 18 files changed, 61 insertions(+), 1 deletion(-) diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index f649b419..ff75c13a 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -53,6 +53,8 @@ const ( olmGracePeriod = 30 * time.Second ManagedByLabel string = "operatorpolicy.policy.open-cluster-management.io/managed" ManagedByAnnotation string = ManagedByLabel + ClusterNameLabel string = "policy.open-cluster-management.io/cluster-name" + ClusterNamespaceLabel string = "policy.open-cluster-management.io/cluster-namespace" ) var ( @@ -476,8 +478,15 @@ func (r *OperatorPolicyReconciler) checkSubOverlap( return statusChanged, nil, nil } + // limit to OperatorPolicies for the same cluster as the policy + // (this cluster might be hosting OperatorPolicies for other clusters) + sel := labels.SelectorFromSet(map[string]string{ + ClusterNameLabel: policy.GetLabels()[ClusterNameLabel], + ClusterNamespaceLabel: policy.GetLabels()[ClusterNamespaceLabel], + }) + opList := &policyv1beta1.OperatorPolicyList{} - if err := r.List(ctx, opList); err != nil { + if err := r.List(ctx, opList, &client.ListOptions{LabelSelector: sel}); err != nil { return statusChanged, nil, err } diff --git a/test/resources/case38_operator_install/operator-policy-all-defaults.yaml b/test/resources/case38_operator_install/operator-policy-all-defaults.yaml index 69f59cb6..a677e9c0 100644 --- a/test/resources/case38_operator_install/operator-policy-all-defaults.yaml +++ b/test/resources/case38_operator_install/operator-policy-all-defaults.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-argocd.yaml b/test/resources/case38_operator_install/operator-policy-argocd.yaml index b836c66d..96fa4ceb 100644 --- a/test/resources/case38_operator_install/operator-policy-argocd.yaml +++ b/test/resources/case38_operator_install/operator-policy-argocd.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-authorino.yaml b/test/resources/case38_operator_install/operator-policy-authorino.yaml index 7aa8b9b5..f7a103a2 100644 --- a/test/resources/case38_operator_install/operator-policy-authorino.yaml +++ b/test/resources/case38_operator_install/operator-policy-authorino.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-defaults-invalid-source.yaml b/test/resources/case38_operator_install/operator-policy-defaults-invalid-source.yaml index adb12926..a536aaa3 100644 --- a/test/resources/case38_operator_install/operator-policy-defaults-invalid-source.yaml +++ b/test/resources/case38_operator_install/operator-policy-defaults-invalid-source.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml b/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml index 3d013c31..62723d31 100644 --- a/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml +++ b/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-mustnothave-any-version.yaml b/test/resources/case38_operator_install/operator-policy-mustnothave-any-version.yaml index 0515fe86..9754ef05 100644 --- a/test/resources/case38_operator_install/operator-policy-mustnothave-any-version.yaml +++ b/test/resources/case38_operator_install/operator-policy-mustnothave-any-version.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-mustnothave.yaml b/test/resources/case38_operator_install/operator-policy-mustnothave.yaml index 43454055..9e2aa203 100644 --- a/test/resources/case38_operator_install/operator-policy-mustnothave.yaml +++ b/test/resources/case38_operator_install/operator-policy-mustnothave.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-no-exist-enforce.yaml b/test/resources/case38_operator_install/operator-policy-no-exist-enforce.yaml index 829c0b4d..b4e6794d 100644 --- a/test/resources/case38_operator_install/operator-policy-no-exist-enforce.yaml +++ b/test/resources/case38_operator_install/operator-policy-no-exist-enforce.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-no-group-csv-fail.yaml b/test/resources/case38_operator_install/operator-policy-no-group-csv-fail.yaml index 2b0e87d3..cca79775 100644 --- a/test/resources/case38_operator_install/operator-policy-no-group-csv-fail.yaml +++ b/test/resources/case38_operator_install/operator-policy-no-group-csv-fail.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-no-group-enforce-one-version.yaml b/test/resources/case38_operator_install/operator-policy-no-group-enforce-one-version.yaml index fce66f19..b5d9fae6 100644 --- a/test/resources/case38_operator_install/operator-policy-no-group-enforce-one-version.yaml +++ b/test/resources/case38_operator_install/operator-policy-no-group-enforce-one-version.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-no-group-enforce.yaml b/test/resources/case38_operator_install/operator-policy-no-group-enforce.yaml index 400da462..cfc6969b 100644 --- a/test/resources/case38_operator_install/operator-policy-no-group-enforce.yaml +++ b/test/resources/case38_operator_install/operator-policy-no-group-enforce.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-no-group-one-version.yaml b/test/resources/case38_operator_install/operator-policy-no-group-one-version.yaml index be9bf4eb..21ab38d2 100644 --- a/test/resources/case38_operator_install/operator-policy-no-group-one-version.yaml +++ b/test/resources/case38_operator_install/operator-policy-no-group-one-version.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-no-group.yaml b/test/resources/case38_operator_install/operator-policy-no-group.yaml index 80a9c473..98820709 100644 --- a/test/resources/case38_operator_install/operator-policy-no-group.yaml +++ b/test/resources/case38_operator_install/operator-policy-no-group.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-strimzi-kafka-operator.yaml b/test/resources/case38_operator_install/operator-policy-strimzi-kafka-operator.yaml index 55eebbb9..dee95da4 100644 --- a/test/resources/case38_operator_install/operator-policy-strimzi-kafka-operator.yaml +++ b/test/resources/case38_operator_install/operator-policy-strimzi-kafka-operator.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-validity-test.yaml b/test/resources/case38_operator_install/operator-policy-validity-test.yaml index b491ad42..41dc1842 100644 --- a/test/resources/case38_operator_install/operator-policy-validity-test.yaml +++ b/test/resources/case38_operator_install/operator-policy-validity-test.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-with-group.yaml b/test/resources/case38_operator_install/operator-policy-with-group.yaml index 870e3983..1c85f233 100644 --- a/test/resources/case38_operator_install/operator-policy-with-group.yaml +++ b/test/resources/case38_operator_install/operator-policy-with-group.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy diff --git a/test/resources/case38_operator_install/operator-policy-with-templates.yaml b/test/resources/case38_operator_install/operator-policy-with-templates.yaml index 202d0a43..d5b313f9 100644 --- a/test/resources/case38_operator_install/operator-policy-with-templates.yaml +++ b/test/resources/case38_operator_install/operator-policy-with-templates.yaml @@ -5,6 +5,9 @@ metadata: annotations: policy.open-cluster-management.io/parent-policy-compliance-db-id: "124" policy.open-cluster-management.io/policy-compliance-db-id: "64" + labels: + policy.open-cluster-management.io/cluster-name: "managed" + policy.open-cluster-management.io/cluster-namespace: "managed" ownerReferences: - apiVersion: policy.open-cluster-management.io/v1 kind: Policy From 3e92860837add8595c553562acdc33c4bc86e014 Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Mon, 10 Jun 2024 10:46:51 -0400 Subject: [PATCH 2/2] Stop subscription intervention if CSV already set Especially in the case of an active upgrade, the previous intervention logic could fight with OLM while setting the CurrentCSV in the sub status. Now, if CurrentCSV or InstalledCSV are set, no intervention will be done. Signed-off-by: Justin Kulikauskas (cherry picked from commit 40d47e06b718be422a5297899ae2183e4e57ef84) --- controllers/operatorpolicy_controller.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index ff75c13a..3bb94c80 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -1319,6 +1319,12 @@ func (r *OperatorPolicyReconciler) considerResolutionFailed( // Do the "intervention" + if mergedSub.Status.InstalledCSV != "" || mergedSub.Status.CurrentCSV != "" { + opLog.Info("The subscription already points to a CSV, not intervening", "currentSub", mergedSub) + + return mergedSub, nil, changed, nil + } + watcher := opPolIdentifier(policy.Namespace, policy.Name) existingCSV, err := r.DynamicWatcher.Get(watcher, clusterServiceVersionGVK, mergedSub.Namespace, unrefCSVMatches[1]) @@ -1344,14 +1350,15 @@ func (r *OperatorPolicyReconciler) considerResolutionFailed( return mergedSub, nil, changed, nil } + opLog.Info("Updating Subscription status to point to CSV", + "csvName", existingCSV.GetName(), "currentSub", mergedSub) + if mergedSub.Status.LastUpdated.IsZero() { mergedSub.Status.LastUpdated = metav1.Now() } mergedSub.Status.CurrentCSV = existingCSV.GetName() - opLog.Info("Updating Subscription status to point to CSV", "csvName", existingCSV.GetName()) - if err := r.TargetClient.Status().Update(ctx, mergedSub); err != nil { return mergedSub, nil, changed, fmt.Errorf("error updating the Subscription status to point to the CSV: %w", err)