Skip to content

Commit 922f6ad

Browse files
Merge pull request #1594 from openshift-cherrypick-robot/cherry-pick-1589-to-release-4.5
[release-4.5] Bug 1850619: don't store full manifests in installplan status (for bundle images)
2 parents 825d12e + eca0279 commit 922f6ad

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)