Skip to content

Commit 9477a55

Browse files
committed
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 759ff45 commit 9477a55

File tree

9 files changed

+348
-32
lines changed

9 files changed

+348
-32
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ setup-bare: clean e2e.namespace
119119
e2e:
120120
go test -v $(MOD_FLAGS) -failfast -timeout 70m ./test/e2e/... -namespace=openshift-operators -kubeconfig=${KUBECONFIG} -olmNamespace=openshift-operator-lifecycle-manager -dummyImage=bitnami/nginx:latest
121121

122-
e2e-local: build-linux build-wait
122+
e2e-local: build-linux build-wait build-util-linux
123123
. ./scripts/build_local.sh
124124
. ./scripts/run_e2e_local.sh $(TEST)
125125

pkg/controller/bundle/bundle_unpacker.go

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

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

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

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

260291
var cs *operatorsv1alpha1.CatalogSource
261292
if cs, err = c.csLister.CatalogSources(result.CatalogSourceRef.Namespace).Get(result.CatalogSourceRef.Name); err != nil {
262-
if apierrors.IsNotFound(err) && cond.Status != corev1.ConditionTrue && cond.Reason != CatalogSourceMissingReason {
293+
if apierrors.IsNotFound(err) && cond.Reason != CatalogSourceMissingReason {
263294
cond.Status = corev1.ConditionTrue
264295
cond.Reason = CatalogSourceMissingReason
265296
cond.Message = CatalogSourceMissingMessage
@@ -303,7 +334,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
303334
return
304335
}
305336

306-
if !jobConditionTrue(job, batchv1.JobComplete) && cond.Status != corev1.ConditionTrue && cond.Reason != JobIncompleteReason {
337+
if !jobConditionTrue(job, batchv1.JobComplete) && (cond.Status != corev1.ConditionTrue || cond.Reason != JobIncompleteReason) {
307338
cond.Status = corev1.ConditionTrue
308339
cond.Reason = JobIncompleteReason
309340
cond.Message = JobIncompleteMessage
@@ -318,6 +349,10 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
318349
return
319350
}
320351

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

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/operator-registry/pkg/configmap"
8+
errorwrap "github.com/pkg/errors"
9+
"github.com/sirupsen/logrus"
10+
v1 "k8s.io/client-go/listers/core/v1"
11+
12+
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
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)