Skip to content

Commit eca0279

Browse files
ecordellopenshift-cherrypick-robot
authored andcommitted
fix(installplan): don't store the manifest contents on the installplan
the installplan status.plan field stores a list of all manifests that will be installed when the installplan is approved. if the installplan is resolving a large set of dependencies, it is possible for it to exceed various limits in kube (grpc request size limit, etcd value size limit, etc). this commit changes the way objects are tracked if they come from bundle images. in that case, the manifests are already unpacked onto the cluster into a configmap. the installplan now stores a reference to the source configmap for each step, rather than storing the manifest itself. this allows the installplan to scale to a large number of resolved objects.
1 parent 0bcd497 commit eca0279

File tree

10 files changed

+362
-38
lines changed

10 files changed

+362
-38
lines changed

pkg/controller/bundle/bundle_unpacker.go

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,27 @@ func (b *BundleUnpackResult) Bundle() *api.Bundle {
3434
return b.bundle
3535
}
3636

37+
func (b *BundleUnpackResult) Name() string {
38+
return b.name
39+
}
40+
41+
// SetCondition replaces the existing BundleLookupCondition of the same type, or adds it if it was not found.
42+
func (b *BundleUnpackResult) SetCondition(cond operatorsv1alpha1.BundleLookupCondition) operatorsv1alpha1.BundleLookupCondition {
43+
for i, existing := range b.Conditions {
44+
if existing.Type != cond.Type {
45+
continue
46+
}
47+
if existing.Status == cond.Status && existing.Reason == cond.Reason {
48+
cond.LastTransitionTime = existing.LastTransitionTime
49+
}
50+
b.Conditions[i] = cond
51+
return cond
52+
}
53+
b.Conditions = append(b.Conditions, cond)
54+
55+
return cond
56+
}
57+
3758
var catalogSourceGVK = operatorsv1alpha1.SchemeGroupVersion.WithKind(operatorsv1alpha1.CatalogSourceKind)
3859

3960
func newBundleUnpackResult(lookup *operatorsv1alpha1.BundleLookup) *BundleUnpackResult {
@@ -251,16 +272,26 @@ const (
251272
CatalogSourceMissingMessage = "referenced catalogsource not found"
252273
JobIncompleteReason = "JobIncomplete"
253274
JobIncompleteMessage = "unpack job not completed"
275+
JobNotStartedReason = "JobNotStarted"
276+
JobNotStartedMessage = "unpack job not yet started"
277+
NotUnpackedReason = "BundleNotUnpacked"
278+
NotUnpackedMessage = "bundle contents have not yet been persisted to installplan status"
254279
)
255280

256281
func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup) (result *BundleUnpackResult, err error) {
257282
result = newBundleUnpackResult(lookup)
283+
284+
// if pending condition is missing, bundle has already been unpacked
258285
cond := result.GetCondition(operatorsv1alpha1.BundleLookupPending)
286+
if cond.Status == corev1.ConditionUnknown {
287+
return result, nil
288+
}
289+
259290
now := c.now()
260291

261292
var cs *operatorsv1alpha1.CatalogSource
262293
if cs, err = c.csLister.CatalogSources(result.CatalogSourceRef.Namespace).Get(result.CatalogSourceRef.Name); err != nil {
263-
if apierrors.IsNotFound(err) && cond.Status != corev1.ConditionTrue && cond.Reason != CatalogSourceMissingReason {
294+
if apierrors.IsNotFound(err) && cond.Reason != CatalogSourceMissingReason {
264295
cond.Status = corev1.ConditionTrue
265296
cond.Reason = CatalogSourceMissingReason
266297
cond.Message = CatalogSourceMissingMessage
@@ -304,7 +335,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
304335
return
305336
}
306337

307-
if !jobConditionTrue(job, batchv1.JobComplete) && cond.Status != corev1.ConditionTrue && cond.Reason != JobIncompleteReason {
338+
if !jobConditionTrue(job, batchv1.JobComplete) && (cond.Status != corev1.ConditionTrue || cond.Reason != JobIncompleteReason) {
308339
cond.Status = corev1.ConditionTrue
309340
cond.Reason = JobIncompleteReason
310341
cond.Message = JobIncompleteMessage
@@ -319,6 +350,10 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
319350
return
320351
}
321352

353+
if result.Bundle() == nil || len(result.Bundle().GetObject()) == 0 {
354+
return
355+
}
356+
322357
// A successful load should remove the pending condition
323358
result.RemoveCondition(operatorsv1alpha1.BundleLookupPending)
324359

pkg/controller/bundle/bundle_unpacker_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ func TestConfigMapUnpacker(t *testing.T) {
7171
Namespace: "ns-a",
7272
Name: "src-a",
7373
},
74+
Conditions: []operatorsv1alpha1.BundleLookupCondition{
75+
{
76+
Type: operatorsv1alpha1.BundleLookupPending,
77+
Status: corev1.ConditionTrue,
78+
Reason: JobNotStartedReason,
79+
Message: JobNotStartedMessage,
80+
},
81+
},
7482
},
7583
},
7684
expected: expected{
@@ -116,6 +124,14 @@ func TestConfigMapUnpacker(t *testing.T) {
116124
Namespace: "ns-a",
117125
Name: "src-a",
118126
},
127+
Conditions: []operatorsv1alpha1.BundleLookupCondition{
128+
{
129+
Type: operatorsv1alpha1.BundleLookupPending,
130+
Status: corev1.ConditionTrue,
131+
Reason: JobNotStartedReason,
132+
Message: JobNotStartedMessage,
133+
},
134+
},
119135
},
120136
},
121137
expected: expected{
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package catalog
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
7+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
8+
"github.com/operator-framework/operator-registry/pkg/configmap"
9+
errorwrap "github.com/pkg/errors"
10+
"github.com/sirupsen/logrus"
11+
v1 "k8s.io/client-go/listers/core/v1"
12+
13+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
14+
)
15+
16+
// ManifestResolver can dereference a manifest for a step. Steps may embed manifests directly or reference content
17+
// in configmaps
18+
type ManifestResolver interface {
19+
ManifestForStep(step *v1alpha1.Step) (string, error)
20+
}
21+
22+
// manifestResolver caches manifest from unpacked bundles (via configmaps)
23+
type manifestResolver struct {
24+
configMapLister v1.ConfigMapLister
25+
unpackedSteps map[string][]v1alpha1.StepResource
26+
namespace string
27+
logger logrus.FieldLogger
28+
}
29+
30+
func newManifestResolver(namespace string, configMapLister v1.ConfigMapLister, logger logrus.FieldLogger) *manifestResolver {
31+
return &manifestResolver{
32+
namespace: namespace,
33+
configMapLister: configMapLister,
34+
unpackedSteps: map[string][]v1alpha1.StepResource{},
35+
logger: logger,
36+
}
37+
}
38+
39+
// ManifestForStep always returns the manifest that should be applied to the cluster for a given step
40+
// the manifest field in the installplan status can contain a reference to a configmap instead
41+
func (r *manifestResolver) ManifestForStep(step *v1alpha1.Step) (string, error) {
42+
manifest := step.Resource.Manifest
43+
ref := refForStep(step, r.logger)
44+
if ref == nil {
45+
return manifest, nil
46+
}
47+
48+
log := r.logger.WithFields(logrus.Fields{"resolving": step.Resolving, "step": step.Resource.Name})
49+
log.WithField("ref", ref).Debug("step is a reference to configmap")
50+
51+
usteps, err := r.unpackedStepsForBundle(step.Resolving, ref)
52+
if err != nil {
53+
return "", err
54+
}
55+
56+
log.Debugf("checking cache for unpacked step")
57+
// need to find the real manifest from the unpacked steps
58+
for _, u := range usteps {
59+
if u.Name == step.Resource.Name &&
60+
u.Kind == step.Resource.Kind &&
61+
u.Version == step.Resource.Version &&
62+
u.Group == step.Resource.Group {
63+
manifest = u.Manifest
64+
log.WithField("manifest", manifest).Debug("step replaced with unpacked value")
65+
break
66+
}
67+
}
68+
if manifest == step.Resource.Manifest {
69+
return "", fmt.Errorf("couldn't find unpacked step for %v", step)
70+
}
71+
return manifest, nil
72+
}
73+
74+
func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *UnpackedBundleReference) ([]v1alpha1.StepResource, error) {
75+
usteps, ok := r.unpackedSteps[bundleName]
76+
if ok {
77+
return usteps, nil
78+
}
79+
cm, err := r.configMapLister.ConfigMaps(ref.Namespace).Get(ref.Name)
80+
if err != nil {
81+
return nil, errorwrap.Wrapf(err, "error finding unpacked bundle configmap for ref %v", *ref)
82+
}
83+
loader := configmap.NewBundleLoader()
84+
bundle, err := loader.Load(cm)
85+
if err != nil {
86+
return nil, errorwrap.Wrapf(err, "error loading unpacked bundle configmap for ref %v", *ref)
87+
}
88+
steps, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
89+
if err != nil {
90+
return nil, errorwrap.Wrapf(err, "error calculating steps for ref %v", *ref)
91+
}
92+
r.unpackedSteps[bundleName] = steps
93+
return steps, nil
94+
}
95+
96+
func refForStep(step *v1alpha1.Step, log logrus.FieldLogger) *UnpackedBundleReference {
97+
log = log.WithFields(logrus.Fields{"resolving": step.Resolving, "step": step.Resource.Name})
98+
var ref UnpackedBundleReference
99+
if err := json.Unmarshal([]byte(step.Resource.Manifest), &ref); err != nil {
100+
log.Debug("step is not a reference to an unpacked bundle (this is not an error if the step is a manifest)")
101+
return nil
102+
}
103+
log = log.WithField("ref", ref)
104+
if ref.Kind != "ConfigMap" || ref.Name == "" || ref.Namespace == "" || ref.CatalogSourceName == "" || ref.CatalogSourceNamespace == "" {
105+
log.Debug("step is not a reference to an unpacked bundle (this is not an error if the step is a manifest)")
106+
return nil
107+
}
108+
return &ref
109+
}

0 commit comments

Comments
 (0)