Skip to content

Commit 09f53f5

Browse files
everettravenbentito
authored andcommitted
(bugfix): reduce frequency of update requests for CSVs
by adding annotations to copied CSVs that are populated with hashes of the non-status fields and the status fields. This seems to be how this was intended to work, but was not actually working this way because the annotations never actually existed on the copied CSV. This resulted in a hot loop of update requests being made on all copied CSVs. Signed-off-by: everettraven <[email protected]>
1 parent f33f7b3 commit 09f53f5

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export CONFIGMAP_SERVER_IMAGE ?= quay.io/operator-framework/configmap-operator-r
6363

6464
PKG := github.com/operator-framework/operator-lifecycle-manager
6565
IMAGE_REPO ?= quay.io/operator-framework/olm
66-
IMAGE_TAG ?= "dev"
66+
IMAGE_TAG ?= dev
6767

6868
# Go build settings #
6969

@@ -228,7 +228,7 @@ kind-create: kind-clean #HELP Create a new kind cluster $KIND_CLUSTER_NAME (defa
228228
$(KIND) export kubeconfig --name $(KIND_CLUSTER_NAME)
229229

230230
.PHONY: deploy
231-
OLM_IMAGE := quay.io/operator-framework/olm:local
231+
OLM_IMAGE := $(IMAGE_REPO):$(IMAGE_TAG)
232232
deploy: $(KIND) $(HELM) #HELP Deploy OLM to kind cluster $KIND_CLUSTER_NAME (default: kind-olmv0) using $OLM_IMAGE (default: quay.io/operator-framework/olm:local)
233233
$(KIND) load docker-image $(OLM_IMAGE) --name $(KIND_CLUSTER_NAME); \
234234
$(HELM) upgrade --install olm deploy/chart \

pkg/controller/operators/olm/operatorgroup.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func (a *Operator) pruneProvidedAPIs(group *operatorsv1.OperatorGroup, groupProv
360360
}
361361

362362
// Prune providedAPIs annotation if the cluster has fewer providedAPIs (handles CSV deletion)
363-
//if intersection := groupProvidedAPIs.Intersection(providedAPIsFromCSVs); len(intersection) < len(groupProvidedAPIs) {
363+
// if intersection := groupProvidedAPIs.Intersection(providedAPIsFromCSVs); len(intersection) < len(groupProvidedAPIs) {
364364
if len(intersection) < len(groupProvidedAPIs) {
365365
difference := groupProvidedAPIs.Difference(intersection)
366366
logger := logger.WithFields(logrus.Fields{
@@ -790,6 +790,11 @@ func copyableCSVHash(original *v1alpha1.ClusterServiceVersion) (string, string,
790790
return newHash, originalHash, nil
791791
}
792792

793+
const (
794+
nonStatusCopyHashAnnotation = "olm.operatorframework.io/nonStatusCopyHash"
795+
statusCopyHashAnnotation = "olm.operatorframework.io/statusCopyHash"
796+
)
797+
793798
// If returned error is not nil, the returned ClusterServiceVersion
794799
// has only the Name, Namespace, and UID fields set.
795800
func (a *Operator) copyToNamespace(prototype *v1alpha1.ClusterServiceVersion, nsFrom, nsTo, nonstatus, status string) (*v1alpha1.ClusterServiceVersion, error) {
@@ -825,11 +830,14 @@ func (a *Operator) copyToNamespace(prototype *v1alpha1.ClusterServiceVersion, ns
825830
prototype.Namespace = existing.Namespace
826831
prototype.ResourceVersion = existing.ResourceVersion
827832
prototype.UID = existing.UID
828-
existingNonStatus := existing.Annotations["$copyhash-nonstatus"]
829-
existingStatus := existing.Annotations["$copyhash-status"]
833+
// Get the non-status and status hash of the existing copied CSV
834+
existingNonStatus := existing.Annotations[nonStatusCopyHashAnnotation]
835+
existingStatus := existing.Annotations[statusCopyHashAnnotation]
830836

831837
var updated *v1alpha1.ClusterServiceVersion
832838
if existingNonStatus != nonstatus {
839+
// include updates to the non-status hash annotation if there is a mismatch
840+
prototype.Annotations[nonStatusCopyHashAnnotation] = nonstatus
833841
if updated, err = a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).Update(context.TODO(), prototype, metav1.UpdateOptions{}); err != nil {
834842
return nil, fmt.Errorf("failed to update: %w", err)
835843
}
@@ -843,6 +851,13 @@ func (a *Operator) copyToNamespace(prototype *v1alpha1.ClusterServiceVersion, ns
843851
if _, err = a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).UpdateStatus(context.TODO(), updated, metav1.UpdateOptions{}); err != nil {
844852
return nil, fmt.Errorf("failed to update status: %w", err)
845853
}
854+
// Update the status first if the existing copied CSV status hash doesn't match what we expect
855+
// to prevent a scenario where the hash annotations match but the contents do not.
856+
// We also need to update the CSV itself in this case to ensure we set the status hash annotation.
857+
prototype.Annotations[statusCopyHashAnnotation] = status
858+
if updated, err = a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).Update(context.TODO(), prototype, metav1.UpdateOptions{}); err != nil {
859+
return nil, fmt.Errorf("failed to update: %w", err)
860+
}
846861
}
847862
return &v1alpha1.ClusterServiceVersion{
848863
ObjectMeta: metav1.ObjectMeta{
@@ -939,7 +954,6 @@ func namespacesChanged(clusterNamespaces []string, statusNamespaces []string) bo
939954

940955
func (a *Operator) getOperatorGroupTargets(op *operatorsv1.OperatorGroup) (map[string]struct{}, error) {
941956
selector, err := metav1.LabelSelectorAsSelector(op.Spec.Selector)
942-
943957
if err != nil {
944958
return nil, err
945959
}

0 commit comments

Comments
 (0)