Skip to content

Commit afe2d61

Browse files
committed
(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 461018e commit afe2d61

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

Makefile

+2-2
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

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

213213
.PHONY: deploy
214-
OLM_IMAGE := quay.io/operator-framework/olm:local
214+
OLM_IMAGE := $(IMAGE_REPO):$(IMAGE_TAG)
215215
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)
216216
$(KIND) load docker-image $(OLM_IMAGE) --name $(KIND_CLUSTER_NAME); \
217217
$(HELM) upgrade --install olm deploy/chart \

pkg/controller/operators/olm/operatorgroup.go

+18-4
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ func (a *Operator) pruneProvidedAPIs(group *operatorsv1.OperatorGroup, groupProv
361361
}
362362

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

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

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

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

0 commit comments

Comments
 (0)