Skip to content

Commit 2c3434a

Browse files
stevekuznetsovstevekuznetsov
authored
OCPBUGS-17157: improve watching semantics around CSVs (#3003)
* operators/catalog: don't watch copied CSVs As far as I can tell, we never do anything with them. Signed-off-by: Steve Kuznetsov <[email protected]> * operators/olm: use a partial object metadata watch for copied CSVs All we ever ned to know about copied CSVs is their metadata. No need to prune objects in memory, it's better to never allocate the memory to deserilize them in the first place. Signed-off-by: Steve Kuznetsov <[email protected]> --------- Signed-off-by: Steve Kuznetsov <[email protected]>
1 parent 2c3928e commit 2c3434a

File tree

14 files changed

+1190
-214
lines changed

14 files changed

+1190
-214
lines changed

‎cmd/olm/main.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/sirupsen/logrus"
1515
"github.com/spf13/pflag"
1616
corev1 "k8s.io/api/core/v1"
17+
"k8s.io/client-go/metadata"
1718
"k8s.io/klog"
1819
ctrl "sigs.k8s.io/controller-runtime"
1920

@@ -154,6 +155,10 @@ func main() {
154155
if err != nil {
155156
logger.WithError(err).Fatal("error configuring custom resource client")
156157
}
158+
metadataClient, err := metadata.NewForConfig(config)
159+
if err != nil {
160+
logger.WithError(err).Fatal("error configuring metadata client")
161+
}
157162

158163
// Create a new instance of the operator.
159164
op, err := olm.NewOperator(
@@ -162,6 +167,7 @@ func main() {
162167
olm.WithWatchedNamespaces(namespaces...),
163168
olm.WithResyncPeriod(queueinformer.ResyncWithJitter(*wakeupInterval, 0.2)),
164169
olm.WithExternalClient(crClient),
170+
olm.WithMetadataClient(metadataClient),
165171
olm.WithOperatorClient(opClient),
166172
olm.WithRestConfig(config),
167173
olm.WithConfigClient(versionedConfigClient),

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

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -204,27 +204,31 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
204204
// Fields are pruned from local copies of the objects managed
205205
// by this informer in order to reduce cached size.
206206
prunedCSVInformer := cache.NewSharedIndexInformer(
207-
pruning.NewListerWatcher(op.client, metav1.NamespaceAll, func(*metav1.ListOptions) {}, pruning.PrunerFunc(func(csv *v1alpha1.ClusterServiceVersion) {
208-
*csv = v1alpha1.ClusterServiceVersion{
209-
TypeMeta: csv.TypeMeta,
210-
ObjectMeta: metav1.ObjectMeta{
211-
Name: csv.Name,
212-
Namespace: csv.Namespace,
213-
Labels: csv.Labels,
214-
Annotations: csv.Annotations,
215-
},
216-
Spec: v1alpha1.ClusterServiceVersionSpec{
217-
CustomResourceDefinitions: csv.Spec.CustomResourceDefinitions,
218-
APIServiceDefinitions: csv.Spec.APIServiceDefinitions,
219-
Replaces: csv.Spec.Replaces,
220-
Version: csv.Spec.Version,
221-
},
222-
Status: v1alpha1.ClusterServiceVersionStatus{
223-
Phase: csv.Status.Phase,
224-
Reason: csv.Status.Reason,
225-
},
226-
}
227-
})),
207+
pruning.NewListerWatcher(op.client, metav1.NamespaceAll,
208+
func(options *metav1.ListOptions) {
209+
options.LabelSelector = fmt.Sprintf("!%s", v1alpha1.CopiedLabelKey)
210+
},
211+
pruning.PrunerFunc(func(csv *v1alpha1.ClusterServiceVersion) {
212+
*csv = v1alpha1.ClusterServiceVersion{
213+
TypeMeta: csv.TypeMeta,
214+
ObjectMeta: metav1.ObjectMeta{
215+
Name: csv.Name,
216+
Namespace: csv.Namespace,
217+
Labels: csv.Labels,
218+
Annotations: csv.Annotations,
219+
},
220+
Spec: v1alpha1.ClusterServiceVersionSpec{
221+
CustomResourceDefinitions: csv.Spec.CustomResourceDefinitions,
222+
APIServiceDefinitions: csv.Spec.APIServiceDefinitions,
223+
Replaces: csv.Spec.Replaces,
224+
Version: csv.Spec.Version,
225+
},
226+
Status: v1alpha1.ClusterServiceVersionStatus{
227+
Phase: csv.Status.Phase,
228+
Reason: csv.Status.Reason,
229+
},
230+
}
231+
})),
228232
&v1alpha1.ClusterServiceVersion{},
229233
resyncPeriod(),
230234
cache.Indexers{

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"time"
66

77
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
8+
"k8s.io/client-go/metadata"
89

910
"github.com/pkg/errors"
1011
"github.com/sirupsen/logrus"
@@ -29,6 +30,7 @@ type operatorConfig struct {
2930
clock utilclock.Clock
3031
logger *logrus.Logger
3132
operatorClient operatorclient.ClientInterface
33+
metadataClient metadata.Interface
3234
externalClient versioned.Interface
3335
strategyResolver install.StrategyResolverInterface
3436
apiReconciler APIIntersectionReconciler
@@ -159,6 +161,12 @@ func WithOperatorClient(operatorClient operatorclient.ClientInterface) OperatorO
159161
}
160162
}
161163

164+
func WithMetadataClient(metadataClient metadata.Interface) OperatorOption {
165+
return func(config *operatorConfig) {
166+
config.metadataClient = metadataClient
167+
}
168+
}
169+
162170
func WithExternalClient(externalClient versioned.Interface) OperatorOption {
163171
return func(config *operatorConfig) {
164172
config.externalClient = externalClient

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

Lines changed: 23 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2525
"k8s.io/client-go/informers"
2626
k8sscheme "k8s.io/client-go/kubernetes/scheme"
27+
"k8s.io/client-go/metadata/metadatainformer"
28+
"k8s.io/client-go/metadata/metadatalister"
2729
"k8s.io/client-go/tools/cache"
2830
"k8s.io/client-go/tools/record"
2931
"k8s.io/client-go/util/workqueue"
@@ -35,12 +37,10 @@ import (
3537
"github.com/operator-framework/api/pkg/operators/v1alpha1"
3638
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
3739
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/informers/externalversions"
38-
operatorsv1alpha1listers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
3940
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/certs"
4041
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
41-
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/pruning"
4242
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/overrides"
43-
resolver "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
43+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
4444
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clients"
4545
csvutility "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/csv"
4646
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event"
@@ -75,7 +75,7 @@ type Operator struct {
7575
client versioned.Interface
7676
lister operatorlister.OperatorLister
7777
protectedCopiedCSVNamespaces map[string]struct{}
78-
copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister
78+
copiedCSVLister metadatalister.Lister
7979
ogQueueSet *queueinformer.ResourceQueueSet
8080
csvQueueSet *queueinformer.ResourceQueueSet
8181
olmConfigQueue workqueue.RateLimitingInterface
@@ -127,6 +127,9 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
127127
if err := k8sscheme.AddToScheme(scheme); err != nil {
128128
return nil, err
129129
}
130+
if err := metav1.AddMetaToScheme(scheme); err != nil {
131+
return nil, err
132+
}
130133

131134
op := &Operator{
132135
Operator: queueOperator,
@@ -208,44 +211,20 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
208211
return nil, err
209212
}
210213

211-
// A separate informer solely for CSV copies. Fields
212-
// are pruned from local copies of the objects managed
214+
// A separate informer solely for CSV copies. Object metadata requests are used
213215
// by this informer in order to reduce cached size.
214-
copiedCSVInformer := cache.NewSharedIndexInformer(
215-
pruning.NewListerWatcher(
216-
op.client,
217-
namespace,
218-
func(opts *metav1.ListOptions) {
219-
opts.LabelSelector = v1alpha1.CopiedLabelKey
220-
},
221-
pruning.PrunerFunc(func(csv *v1alpha1.ClusterServiceVersion) {
222-
nonstatus, status := copyableCSVHash(csv)
223-
*csv = v1alpha1.ClusterServiceVersion{
224-
TypeMeta: csv.TypeMeta,
225-
ObjectMeta: csv.ObjectMeta,
226-
Status: v1alpha1.ClusterServiceVersionStatus{
227-
Phase: csv.Status.Phase,
228-
Reason: csv.Status.Reason,
229-
},
230-
}
231-
if csv.Annotations == nil {
232-
csv.Annotations = make(map[string]string, 2)
233-
}
234-
// These annotation keys are
235-
// intentionally invalid -- all writes
236-
// to copied CSVs are regenerated from
237-
// the corresponding non-copied CSV,
238-
// so it should never be transmitted
239-
// back to the API server.
240-
csv.Annotations["$copyhash-nonstatus"] = nonstatus
241-
csv.Annotations["$copyhash-status"] = status
242-
}),
243-
),
244-
&v1alpha1.ClusterServiceVersion{},
216+
gvr := v1alpha1.SchemeGroupVersion.WithResource("clusterserviceversions")
217+
copiedCSVInformer := metadatainformer.NewFilteredMetadataInformer(
218+
config.metadataClient,
219+
gvr,
220+
namespace,
245221
config.resyncPeriod(),
246222
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
247-
)
248-
op.copiedCSVLister = operatorsv1alpha1listers.NewClusterServiceVersionLister(copiedCSVInformer.GetIndexer())
223+
func(options *metav1.ListOptions) {
224+
options.LabelSelector = v1alpha1.CopiedLabelKey
225+
},
226+
).Informer()
227+
op.copiedCSVLister = metadatalister.New(copiedCSVInformer.GetIndexer(), gvr)
249228

250229
// Register separate queue for gcing copied csvs
251230
copiedCSVGCQueue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), fmt.Sprintf("%s/csv-gc", namespace))
@@ -1195,17 +1174,16 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) {
11951174
}
11961175
}
11971176

1198-
func (a *Operator) removeDanglingChildCSVs(csv *v1alpha1.ClusterServiceVersion) error {
1177+
func (a *Operator) removeDanglingChildCSVs(csv *metav1.PartialObjectMetadata) error {
11991178
logger := a.logger.WithFields(logrus.Fields{
12001179
"id": queueinformer.NewLoopID(),
12011180
"csv": csv.GetName(),
12021181
"namespace": csv.GetNamespace(),
1203-
"phase": csv.Status.Phase,
12041182
"labels": csv.GetLabels(),
12051183
"annotations": csv.GetAnnotations(),
12061184
})
12071185

1208-
if !csv.IsCopied() {
1186+
if !v1alpha1.IsCopied(csv) {
12091187
logger.Warning("removeDanglingChild called on a parent. this is a no-op but should be avoided.")
12101188
return nil
12111189
}
@@ -1244,7 +1222,7 @@ func (a *Operator) removeDanglingChildCSVs(csv *v1alpha1.ClusterServiceVersion)
12441222
return nil
12451223
}
12461224

1247-
func (a *Operator) deleteChild(csv *v1alpha1.ClusterServiceVersion, logger *logrus.Entry) error {
1225+
func (a *Operator) deleteChild(csv *metav1.PartialObjectMetadata, logger *logrus.Entry) error {
12481226
logger.Debug("gcing csv")
12491227
return a.client.OperatorsV1alpha1().ClusterServiceVersions(csv.GetNamespace()).Delete(context.TODO(), csv.GetName(), metav1.DeleteOptions{})
12501228
}
@@ -1683,12 +1661,12 @@ func (a *Operator) createCSVCopyingDisabledEvent(csv *v1alpha1.ClusterServiceVer
16831661
}
16841662

16851663
func (a *Operator) syncGcCsv(obj interface{}) (syncError error) {
1686-
clusterServiceVersion, ok := obj.(*v1alpha1.ClusterServiceVersion)
1664+
clusterServiceVersion, ok := obj.(*metav1.PartialObjectMetadata)
16871665
if !ok {
16881666
a.logger.Debugf("wrong type: %#v", obj)
16891667
return fmt.Errorf("casting ClusterServiceVersion failed")
16901668
}
1691-
if clusterServiceVersion.IsCopied() {
1669+
if v1alpha1.IsCopied(clusterServiceVersion) {
16921670
syncError = a.removeDanglingChildCSVs(clusterServiceVersion)
16931671
return
16941672
}

0 commit comments

Comments
 (0)