Skip to content

Commit cc7265f

Browse files
ecordellecordell
committed
feat(olm): update deployment annotations as soon as the csv annotations
are available this avoids some unnecessary resyncing
1 parent fb90516 commit cc7265f

File tree

4 files changed

+194
-65
lines changed

4 files changed

+194
-65
lines changed

‎pkg/api/apis/operators/v1alpha2/operatorgroup_types.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,20 @@
11
package v1alpha2
22

33
import (
4+
"sort"
5+
"strings"
6+
47
corev1 "k8s.io/api/core/v1"
58
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
69
)
710

11+
const (
12+
OperatorGroupAnnotationKey = "olm.operatorGroup"
13+
OperatorGroupNamespaceAnnotationKey = "olm.operatorNamespace"
14+
OperatorGroupTargetsAnnotationKey = "olm.targetNamespaces"
15+
OperatorGroupProvidedAPIsAnnotationKey = "olm.providedAPIs"
16+
)
17+
818
type OperatorGroupSpec struct {
919
// Selector selects the OperatorGroup's target namespaces.
1020
// +optional
@@ -52,9 +62,7 @@ type OperatorGroupList struct {
5262
Items []OperatorGroup `json:"items"`
5363
}
5464

55-
const (
56-
OperatorGroupAnnotationKey = "olm.operatorGroup"
57-
OperatorGroupNamespaceAnnotationKey = "olm.operatorNamespace"
58-
OperatorGroupTargetsAnnotationKey = "olm.targetNamespaces"
59-
OperatorGroupProvidedAPIsAnnotationKey = "olm.providedAPIs"
60-
)
65+
func (o *OperatorGroup) BuildTargetNamespaces() string {
66+
sort.Strings(o.Status.Namespaces)
67+
return strings.Join(o.Status.Namespaces, ",")
68+
}

‎pkg/controller/operators/olm/operator.go‎

Lines changed: 116 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import (
77
"time"
88

99
"github.com/sirupsen/logrus"
10+
appsv1 "k8s.io/api/apps/v1"
1011
corev1 "k8s.io/api/core/v1"
1112
extv1beta1 "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
1213
k8serrors "k8s.io/apimachinery/pkg/api/errors"
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1415
"k8s.io/apimachinery/pkg/labels"
16+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1517
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1618
"k8s.io/client-go/informers"
1719
"k8s.io/client-go/tools/cache"
@@ -305,7 +307,7 @@ func (a *Operator) syncObject(obj interface{}) (syncError error) {
305307
logger := a.Log.WithFields(logrus.Fields{
306308
"name": metaObj.GetName(),
307309
"namespace": metaObj.GetNamespace(),
308-
"sel": metaObj.GetSelfLink(),
310+
"self": metaObj.GetSelfLink(),
309311
})
310312

311313
// Requeue all owner CSVs
@@ -572,7 +574,7 @@ func (a *Operator) syncCopyCSV(obj interface{}) (syncError error) {
572574
return
573575
}
574576

575-
operatorGroup := a.operatorGroupForActiveCSV(logger, clusterServiceVersion)
577+
operatorGroup := a.operatorGroupFromAnnotations(logger, clusterServiceVersion)
576578
if operatorGroup == nil {
577579
logger.WithField("reason", "no operatorgroup found for active CSV").Debug("skipping CSV resource copy to target namespaces")
578580
return
@@ -607,8 +609,8 @@ func (a *Operator) syncCopyCSV(obj interface{}) (syncError error) {
607609
return
608610
}
609611

610-
// operatorGroupForCSV returns the OperatorGroup for the CSV only if the CSV is active one in the group
611-
func (a *Operator) operatorGroupForActiveCSV(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) *v1alpha2.OperatorGroup {
612+
// operatorGroupFromAnnotations returns the OperatorGroup for the CSV only if the CSV is active one in the group
613+
func (a *Operator) operatorGroupFromAnnotations(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) *v1alpha2.OperatorGroup {
612614
annotations := csv.GetAnnotations()
613615

614616
// Not part of a group yet
@@ -640,23 +642,63 @@ func (a *Operator) operatorGroupForActiveCSV(logger *logrus.Entry, csv *v1alpha1
640642
return nil
641643
}
642644

643-
// targets, ok := annotations[v1alpha2.OperatorGroupTargetsAnnotationKey]
644-
//
645-
// // No target annotation
646-
// if !ok {
647-
// logger.Info("no olm.targetNamespaces annotation")
648-
// return nil
649-
// }
650-
//
651-
// // Target namespaces don't match
652-
// if targets != strings.Join(operatorGroup.Status.Namespaces, ",") {
653-
// logger.Info("olm.targetNamespaces annotation doesn't match operatorgroup status")
654-
// return nil
655-
// }
645+
targets, ok := annotations[v1alpha2.OperatorGroupTargetsAnnotationKey]
646+
647+
// No target annotation
648+
if !ok {
649+
logger.Info("no olm.targetNamespaces annotation")
650+
return nil
651+
}
652+
653+
// Target namespaces don't match
654+
if targets != strings.Join(operatorGroup.Status.Namespaces, ",") {
655+
logger.Info("olm.targetNamespaces annotation doesn't match operatorgroup status")
656+
return nil
657+
}
656658

657659
return operatorGroup
658660
}
659661

662+
func (a *Operator) operatorGroupForCSV(csv *v1alpha1.ClusterServiceVersion, logger *logrus.Entry) (*v1alpha2.OperatorGroup, error) {
663+
now := timeNow()
664+
665+
// Attempt to associate an OperatorGroup with the CSV.
666+
operatorGroups, err := a.client.OperatorsV1alpha2().OperatorGroups(csv.GetNamespace()).List(metav1.ListOptions{})
667+
if err != nil {
668+
logger.Errorf("error occurred while attempting to associate csv with operatorgroup")
669+
return nil, err
670+
}
671+
var operatorGroup *v1alpha2.OperatorGroup
672+
673+
switch len(operatorGroups.Items) {
674+
case 0:
675+
err = fmt.Errorf("csv in namespace with no operatorgroups")
676+
logger.Warn(err)
677+
csv.SetPhaseWithEvent(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonNoOperatorGroup, err.Error(), now, a.recorder)
678+
return nil, err
679+
case 1:
680+
operatorGroup = &operatorGroups.Items[0]
681+
logger = logger.WithField("opgroup", operatorGroup.GetName())
682+
if a.operatorGroupAnnotationsDiffer(&csv.ObjectMeta, operatorGroup) {
683+
a.setOperatorGroupAnnotations(&csv.ObjectMeta, operatorGroup, true)
684+
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(csv.GetNamespace()).Update(csv); err != nil {
685+
logger.WithError(err).Warn("error adding operatorgroup annotations")
686+
return nil, err
687+
}
688+
return nil, nil
689+
}
690+
logger.Info("csv in operatorgroup")
691+
return operatorGroup, nil
692+
default:
693+
err = fmt.Errorf("csv created in namespace with multiple operatorgroups, can't pick one automatically")
694+
logger.WithError(err).Warn("csv failed to become an operatorgroup member")
695+
if csv.Status.Reason != v1alpha1.CSVReasonTooManyOperatorGroups {
696+
csv.SetPhaseWithEvent(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonTooManyOperatorGroups, err.Error(), now, a.recorder)
697+
}
698+
return nil, err
699+
}
700+
}
701+
660702
// transitionCSVState moves the CSV status state machine along based on the current value and the current cluster state.
661703
func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v1alpha1.ClusterServiceVersion, syncError error) {
662704
logger := a.Log.WithFields(logrus.Fields{
@@ -681,44 +723,18 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
681723
return
682724
}
683725

684-
// Attempt to associate an OperatorGroup with the CSV.
685-
operatorGroups, err := a.lister.OperatorsV1alpha2().OperatorGroupLister().OperatorGroups(out.GetNamespace()).List(labels.Everything())
686-
if err != nil {
687-
logger.Errorf("error occurred while attempting to associate csv with operatorgroup")
726+
// Verify CSV operatorgroup (and update annotations if needed)
727+
operatorGroup, err := a.operatorGroupForCSV(out, logger)
728+
if operatorGroup == nil {
729+
// when err is nil, we still want to exit, but we don't want to re-add the csv ratelimited to the queue
688730
syncError = err
689-
}
690-
var operatorGroup *v1alpha2.OperatorGroup
691-
692-
switch len(operatorGroups) {
693-
case 0:
694-
syncError = fmt.Errorf("csv in namespace with no operatorgroups")
695-
logger.Warn(syncError.Error())
696-
out.SetPhaseWithEvent(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonNoOperatorGroup, syncError.Error(), now, a.recorder)
731+
logger.WithField("err", err).Info("operatorgroup incorrect")
697732
return
698-
case 1:
699-
operatorGroup = a.operatorGroupForActiveCSV(logger, out)
700-
if operatorGroup == nil {
701-
operatorGroup = operatorGroups[0]
702-
logger = logger.WithField("opgroup", operatorGroup.GetName())
703-
704-
if a.operatorGroupAnnotationsDiffer(&out.ObjectMeta, operatorGroup) {
705-
a.setOperatorGroupAnnotations(&out.ObjectMeta, operatorGroup, true)
706-
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(out.GetNamespace()).Update(out); err != nil {
707-
logger.WithError(err).Warn("error adding operatorgroup annotations")
708-
syncError = err
709-
}
710-
}
733+
}
711734

712-
return
713-
}
714-
logger.Info("csv in operatorgroup")
715-
default:
716-
syncError = fmt.Errorf("csv created in namespace with multiple operatorgroups, can't pick one automatically")
717-
logger.WithError(syncError).Warn("csv failed to become an operatorgroup member")
718-
if out.Status.Reason != v1alpha1.CSVReasonTooManyOperatorGroups {
719-
out.SetPhaseWithEvent(v1alpha1.CSVPhaseFailed, v1alpha1.CSVReasonTooManyOperatorGroups, syncError.Error(), now, a.recorder)
720-
}
721-
return
735+
logger.Info("updated annotations to match current operatorgroup")
736+
if err := a.ensureDeploymentAnnotations(logger, out); err != nil {
737+
return nil, err
722738
}
723739

724740
modeSet, err := v1alpha1.NewInstallModeSet(out.Spec.InstallModes)
@@ -1305,3 +1321,51 @@ func (a *Operator) cleanupCSVDeployments(logger *logrus.Entry, csv *v1alpha1.Clu
13051321
}
13061322
}
13071323
}
1324+
1325+
func (a *Operator) ensureDeploymentAnnotations(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error {
1326+
// Get csv operatorgroup annotations
1327+
annotations := a.copyOperatorGroupAnnotations(&csv.ObjectMeta)
1328+
1329+
// Extract the InstallStrategy for the deployment
1330+
strategy, err := a.resolver.UnmarshalStrategy(csv.Spec.InstallStrategy)
1331+
if err != nil {
1332+
logger.Warn("could not parse install strategy while cleaning up CSV deployment")
1333+
return nil
1334+
}
1335+
1336+
// Assume the strategy is for a deployment
1337+
strategyDetailsDeployment, ok := strategy.(*install.StrategyDetailsDeployment)
1338+
if !ok {
1339+
logger.Warnf("could not cast install strategy as type %T", strategyDetailsDeployment)
1340+
return nil
1341+
}
1342+
1343+
var depNames []string
1344+
for _, dep := range strategyDetailsDeployment.DeploymentSpecs {
1345+
depNames = append(depNames, dep.Name)
1346+
}
1347+
existingDeployments, err := a.lister.AppsV1().DeploymentLister().Deployments(csv.GetNamespace()).List(labels.Everything())
1348+
if err != nil {
1349+
return err
1350+
}
1351+
1352+
// compare deployments to see if any need to be created/updated
1353+
existingMap := map[string]*appsv1.Deployment{}
1354+
for _, d := range existingDeployments {
1355+
existingMap[d.GetName()] = d
1356+
}
1357+
1358+
updateErrs := []error{}
1359+
for _, dep := range existingMap {
1360+
if dep.Spec.Template.Annotations == nil {
1361+
dep.Spec.Template.Annotations = map[string]string{}
1362+
}
1363+
for key, value := range annotations {
1364+
dep.Spec.Template.Annotations[key] = value
1365+
}
1366+
if _, _, err := a.OpClient.UpdateDeployment(dep); err != nil {
1367+
updateErrs = append(updateErrs, err)
1368+
}
1369+
}
1370+
return utilerrors.NewAggregate(updateErrs)
1371+
}

‎pkg/controller/operators/olm/operator_test.go‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2085,6 +2085,51 @@ func TestTransitionCSV(t *testing.T) {
20852085
},
20862086
},
20872087
},
2088+
{
2089+
name: "SingleCSVSucceededToSucceeded/OperatorGroupChanged",
2090+
initial: initial{
2091+
csvs: []runtime.Object{
2092+
withConditionReason(csvWithAnnotations(csv("csv1",
2093+
namespace,
2094+
"0.0.0",
2095+
"",
2096+
installStrategy("a1", nil, nil),
2097+
[]*v1beta1.CustomResourceDefinition{crd("c1", "v1", "g1")},
2098+
[]*v1beta1.CustomResourceDefinition{},
2099+
v1alpha1.CSVPhaseSucceeded,
2100+
), defaultTemplateAnnotations), v1alpha1.CSVReasonInstallSuccessful),
2101+
},
2102+
clientObjs: []runtime.Object{
2103+
&v1alpha2.OperatorGroup{
2104+
TypeMeta: metav1.TypeMeta{
2105+
Kind: "OperatorGroup",
2106+
APIVersion: v1alpha2.SchemeGroupVersion.String(),
2107+
},
2108+
ObjectMeta: metav1.ObjectMeta{
2109+
Name: "default",
2110+
Namespace: namespace,
2111+
},
2112+
Spec: v1alpha2.OperatorGroupSpec{},
2113+
Status: v1alpha2.OperatorGroupStatus{
2114+
Namespaces: []string{namespace, "new-namespace"},
2115+
},
2116+
},
2117+
},
2118+
apis: []runtime.Object{},
2119+
objs: []runtime.Object{
2120+
deployment("a1", namespace, "sa", defaultTemplateAnnotations),
2121+
serviceAccount("sa", namespace),
2122+
},
2123+
crds: []runtime.Object{
2124+
crd("c1", "v1", "g1"),
2125+
},
2126+
},
2127+
expected: expected{
2128+
csvStates: map[string]csvState{
2129+
"csv1": {exists: true, phase: v1alpha1.CSVPhaseSucceeded, reason: v1alpha1.CSVReasonInstallSuccessful},
2130+
},
2131+
},
2132+
},
20882133
{
20892134
name: "SingleCSVInstallReadyToFailed/BadStrategy",
20902135
initial: initial{
@@ -3608,7 +3653,7 @@ func TestSyncOperatorGroups(t *testing.T) {
36083653
require.NoError(t, err)
36093654

36103655
// Sync csvs enough to get them back to succeeded state
3611-
for i := 0; i < 6; i++ {
3656+
for i := 0; i < 8; i++ {
36123657
opGroupCSVs, err := op.client.OperatorsV1alpha1().ClusterServiceVersions(operatorNamespace).List(metav1.ListOptions{})
36133658
require.NoError(t, err)
36143659

‎pkg/controller/operators/olm/operatorgroup.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package olm
33
import (
44
"fmt"
55
"reflect"
6-
"sort"
76
"strings"
87

98
"github.com/sirupsen/logrus"
@@ -72,14 +71,17 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
7271
logger.WithError(err).Warn("operatorgroup update failed")
7372
return err
7473
}
75-
74+
logger.Debug("namespace change detected and operatorgroup status updated")
7675
// CSV requeue is handled by the succeeding sync
76+
7777
return nil
7878
}
7979

80+
logger.Debug("check that operatorgroup has updated CSV anotations")
8081
err = a.annotateCSVs(op, targetNamespaces, logger)
8182
if err != nil {
8283
logger.WithError(err).Warn("failed to annotate CSVs in operatorgroup after group change")
84+
return err
8385
}
8486
logger.Debug("OperatorGroup CSV annotation completed")
8587

@@ -129,7 +131,7 @@ func (a *Operator) annotateCSVs(group *v1alpha2.OperatorGroup, targetNamespaces
129131
continue
130132
}
131133
}
132-
134+
logger.WithField("targets", targetNamespaces).Debug("requeueing copied csvs in target namespaces")
133135
for _, ns := range targetNamespaces {
134136
_, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(csv.GetName())
135137
if k8serrors.IsNotFound(err) {
@@ -588,23 +590,33 @@ func (a *Operator) setOperatorGroupAnnotations(obj *metav1.ObjectMeta, op *v1alp
588590
metav1.SetMetaDataAnnotation(obj, v1alpha2.OperatorGroupAnnotationKey, op.GetName())
589591

590592
if addTargets && op.Status.Namespaces != nil {
591-
sort.Strings(op.Status.Namespaces)
592-
metav1.SetMetaDataAnnotation(obj, v1alpha2.OperatorGroupTargetsAnnotationKey, strings.Join(op.Status.Namespaces, ","))
593+
metav1.SetMetaDataAnnotation(obj, v1alpha2.OperatorGroupTargetsAnnotationKey, op.BuildTargetNamespaces())
593594
}
594595
}
595596

596597
func (a *Operator) operatorGroupAnnotationsDiffer(obj *metav1.ObjectMeta, op *v1alpha2.OperatorGroup) bool {
597598
annotations := obj.GetAnnotations()
599+
if annotations == nil {
600+
return true
601+
}
598602
if operatorGroupNamespace, ok := annotations[v1alpha2.OperatorGroupNamespaceAnnotationKey]; !ok || operatorGroupNamespace != op.GetNamespace() {
599603
return true
600604
}
601605
if operatorGroup, ok := annotations[v1alpha2.OperatorGroupAnnotationKey]; !ok || operatorGroup != op.GetName() {
602606
return true
603607
}
604-
if targets, ok := annotations[v1alpha2.OperatorGroupTargetsAnnotationKey]; !ok || targets != strings.Join(op.Status.Namespaces, ",") {
608+
if targets, ok := annotations[v1alpha2.OperatorGroupTargetsAnnotationKey]; !ok || targets != op.BuildTargetNamespaces() {
609+
a.Log.WithFields(logrus.Fields{
610+
"annotationTargets": annotations[v1alpha2.OperatorGroupTargetsAnnotationKey],
611+
"opgroupTargets": op.BuildTargetNamespaces(),
612+
}).Debug("annotations different")
605613
return true
606614
}
607615

616+
a.Log.WithFields(logrus.Fields{
617+
"annotationTargets": annotations[v1alpha2.OperatorGroupTargetsAnnotationKey],
618+
"opgroupTargets": op.BuildTargetNamespaces(),
619+
}).Debug("annotations correct")
608620
return false
609621
}
610622

0 commit comments

Comments
 (0)