Skip to content

Commit 8118f7d

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #2114 from benluddy/alongside
Annotate CRDs that are installed alongside CSVs.
2 parents 1e0b2ba + c46877a commit 8118f7d

File tree

20 files changed

+2291
-44
lines changed

20 files changed

+2291
-44
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1642,7 +1642,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
16421642
o.logger.Errorf("failed to get a client for plan execution- %v", err)
16431643
return err
16441644
}
1645-
b := newBuilder(builderKubeClient, builderDynamicClient, o.csvProvidedAPIsIndexer, r, o.logger)
1645+
b := newBuilder(plan, o.lister.OperatorsV1alpha1().ClusterServiceVersionLister(), builderKubeClient, builderDynamicClient, r, o.logger)
16461646

16471647
for i, step := range plan.Status.Plan {
16481648
doStep := true

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

Lines changed: 73 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,22 @@ import (
44
"context"
55
"fmt"
66

7-
"k8s.io/client-go/dynamic"
8-
"k8s.io/client-go/tools/cache"
7+
"github.com/pkg/errors"
8+
"github.com/sirupsen/logrus"
99

10-
"github.com/operator-framework/api/pkg/operators/v1alpha1"
11-
crdlib "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/crd"
12-
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
13-
errorwrap "github.com/pkg/errors"
14-
logger "github.com/sirupsen/logrus"
1510
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1611
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
1712
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
1813
apiextensionsv1beta1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"
19-
2014
k8serrors "k8s.io/apimachinery/pkg/api/errors"
2115
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
"k8s.io/client-go/dynamic"
17+
18+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
19+
listersv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
20+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/alongside"
21+
crdlib "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/crd"
22+
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
2223
)
2324

2425
// Stepper manages cluster interactions based on the step.
@@ -36,21 +37,24 @@ func (s StepperFunc) Status() (v1alpha1.StepStatus, error) {
3637
// Builder holds clients and data structures required for the StepBuilder to work
3738
// Builder attributes are not to meant to be accessed outside the StepBuilder method
3839
type builder struct {
39-
opclient operatorclient.ClientInterface
40-
dynamicClient dynamic.Interface
41-
manifestResolver ManifestResolver
42-
csvToProvidedAPIs map[string]cache.Indexer
43-
logger logger.FieldLogger
40+
plan *v1alpha1.InstallPlan
41+
csvLister listersv1alpha1.ClusterServiceVersionLister
42+
opclient operatorclient.ClientInterface
43+
dynamicClient dynamic.Interface
44+
manifestResolver ManifestResolver
45+
logger logrus.FieldLogger
46+
47+
annotator alongside.Annotator
4448
}
4549

46-
func newBuilder(opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, csvToProvidedAPIs map[string]cache.Indexer,
47-
manifestResolver ManifestResolver, logger logger.FieldLogger) *builder {
50+
func newBuilder(plan *v1alpha1.InstallPlan, csvLister listersv1alpha1.ClusterServiceVersionLister, opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, manifestResolver ManifestResolver, logger logrus.FieldLogger) *builder {
4851
return &builder{
49-
opclient: opclient,
50-
dynamicClient: dynamicClient,
51-
manifestResolver: manifestResolver,
52-
csvToProvidedAPIs: csvToProvidedAPIs,
53-
logger: logger,
52+
plan: plan,
53+
csvLister: csvLister,
54+
opclient: opclient,
55+
dynamicClient: dynamicClient,
56+
manifestResolver: manifestResolver,
57+
logger: logger,
5458
}
5559
}
5660

@@ -75,6 +79,7 @@ func (b *builder) create(step v1alpha1.Step) (Stepper, error) {
7579
if err != nil {
7680
return nil, err
7781
}
82+
7883
switch version {
7984
case crdlib.V1Version:
8085
return b.NewCRDV1Step(b.opclient.ApiextensionsInterface().ApiextensionsV1(), &step, manifest), nil
@@ -98,7 +103,7 @@ func (b *builder) NewCRDV1Step(client apiextensionsv1client.ApiextensionsV1Inter
98103
if k8serrors.IsNotFound(err) {
99104
return v1alpha1.StepStatusNotPresent, nil
100105
} else {
101-
return v1alpha1.StepStatusNotPresent, errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name)
106+
return v1alpha1.StepStatusNotPresent, errors.Wrapf(err, "error finding the %s CRD", crd.Name)
102107
}
103108
}
104109
established, namesAccepted := false, false
@@ -123,28 +128,31 @@ func (b *builder) NewCRDV1Step(client apiextensionsv1client.ApiextensionsV1Inter
123128
return v1alpha1.StepStatusUnknown, err
124129
}
125130

131+
setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd)
132+
126133
_, createError := client.CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
127134
if k8serrors.IsAlreadyExists(createError) {
128135
currentCRD, _ := client.CustomResourceDefinitions().Get(context.TODO(), crd.GetName(), metav1.GetOptions{})
129136
crd.SetResourceVersion(currentCRD.GetResourceVersion())
130137
if err = validateV1CRDCompatibility(b.dynamicClient, currentCRD, crd); err != nil {
131-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error validating existing CRs against new CRD's schema: %s", step.Resource.Name)
138+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error validating existing CRs against new CRD's schema: %s", step.Resource.Name)
132139
}
133140

134141
// check to see if stored versions changed and whether the upgrade could cause potential data loss
135142
safe, err := crdlib.SafeStorageVersionUpgrade(currentCRD, crd)
136143
if !safe {
137144
b.logger.Errorf("risk of data loss updating %s: %s", step.Resource.Name, err)
138-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
145+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
139146
}
140147
if err != nil {
141-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
148+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
142149
}
143150

144151
// Update CRD to new version
152+
setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd, currentCRD)
145153
_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
146154
if err != nil {
147-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
155+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
148156
}
149157
// If it already existed, mark the step as Present.
150158
// they were equal - mark CRD as present
@@ -173,7 +181,7 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
173181
if k8serrors.IsNotFound(err) {
174182
return v1alpha1.StepStatusNotPresent, nil
175183
} else {
176-
return v1alpha1.StepStatusNotPresent, errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name)
184+
return v1alpha1.StepStatusNotPresent, errors.Wrapf(err, "error finding the %s CRD", crd.Name)
177185
}
178186
}
179187
established, namesAccepted := false, false
@@ -198,29 +206,32 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
198206
return v1alpha1.StepStatusUnknown, err
199207
}
200208

209+
setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd)
210+
201211
_, createError := client.CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
202212
if k8serrors.IsAlreadyExists(createError) {
203213
currentCRD, _ := client.CustomResourceDefinitions().Get(context.TODO(), crd.GetName(), metav1.GetOptions{})
204214
crd.SetResourceVersion(currentCRD.GetResourceVersion())
205215

206216
if err = validateV1Beta1CRDCompatibility(b.dynamicClient, currentCRD, crd); err != nil {
207-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error validating existing CRs against new CRD's schema: %s", step.Resource.Name)
217+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error validating existing CRs against new CRD's schema: %s", step.Resource.Name)
208218
}
209219

210220
// check to see if stored versions changed and whether the upgrade could cause potential data loss
211221
safe, err := crdlib.SafeStorageVersionUpgrade(currentCRD, crd)
212222
if !safe {
213223
b.logger.Errorf("risk of data loss updating %s: %s", step.Resource.Name, err)
214-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
224+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
215225
}
216226
if err != nil {
217-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
227+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
218228
}
219229

220230
// Update CRD to new version
231+
setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd, currentCRD)
221232
_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
222233
if err != nil {
223-
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
234+
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
224235
}
225236
// If it already existed, mark the step as Present.
226237
// they were equal - mark CRD as present
@@ -235,3 +246,35 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
235246
return v1alpha1.StepStatusUnknown, nil
236247
}
237248
}
249+
250+
func setInstalledAlongsideAnnotation(a alongside.Annotator, dst metav1.Object, namespace string, name string, lister listersv1alpha1.ClusterServiceVersionLister, srcs ...metav1.Object) {
251+
var (
252+
nns []alongside.NamespacedName
253+
)
254+
255+
// Only keep references to existing and non-copied CSVs to
256+
// avoid unbounded growth.
257+
for _, src := range srcs {
258+
for _, nn := range a.FromObject(src) {
259+
if nn.Namespace == namespace && nn.Name == name {
260+
continue
261+
}
262+
263+
if csv, err := lister.ClusterServiceVersions(nn.Namespace).Get(nn.Name); k8serrors.IsNotFound(err) {
264+
continue
265+
} else if err == nil && csv.IsCopied() {
266+
continue
267+
}
268+
// CSV exists and is not copied OR err is non-nil, but
269+
// not 404. Safer to assume it exists in unhandled
270+
// error cases and try again next time.
271+
nns = append(nns, nn)
272+
}
273+
}
274+
275+
if namespace != "" && name != "" {
276+
nns = append(nns, alongside.NamespacedName{Namespace: namespace, Name: name})
277+
}
278+
279+
a.ToObject(dst, nns)
280+
}

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

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
package catalog
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
"k8s.io/apimachinery/pkg/api/errors"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
"k8s.io/apimachinery/pkg/runtime/schema"
11+
12+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
13+
v1alpha1listers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
14+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/alongside"
15+
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes"
16+
)
17+
18+
func TestSetInstalledAlongsideAnnotation(t *testing.T) {
19+
for _, tc := range []struct {
20+
Name string
21+
NewNamespace string
22+
NewName string
23+
CSVs []v1alpha1.ClusterServiceVersion
24+
Before []alongside.NamespacedName
25+
After []alongside.NamespacedName
26+
}{
27+
{
28+
Name: "object annotated with bundle name",
29+
NewNamespace: "test-namespace",
30+
NewName: "test-name",
31+
After: []alongside.NamespacedName{
32+
{Namespace: "test-namespace", Name: "test-name"},
33+
},
34+
},
35+
{
36+
Name: "annotations referencing missing bundles removed",
37+
NewNamespace: "test-namespace",
38+
NewName: "test-name",
39+
Before: []alongside.NamespacedName{
40+
{Namespace: "missing-namespace", Name: "missing-name"},
41+
},
42+
After: []alongside.NamespacedName{
43+
{Namespace: "test-namespace", Name: "test-name"},
44+
},
45+
},
46+
{
47+
Name: "annotations referencing copied csv removed",
48+
NewNamespace: "test-namespace",
49+
NewName: "test-name",
50+
CSVs: []v1alpha1.ClusterServiceVersion{
51+
{
52+
ObjectMeta: metav1.ObjectMeta{
53+
Namespace: "copied-namespace",
54+
Name: "copied-name",
55+
},
56+
Status: v1alpha1.ClusterServiceVersionStatus{
57+
Reason: v1alpha1.CSVReasonCopied,
58+
},
59+
},
60+
},
61+
Before: []alongside.NamespacedName{
62+
{Namespace: "copied-namespace", Name: "copied-name"},
63+
},
64+
After: []alongside.NamespacedName{
65+
{Namespace: "test-namespace", Name: "test-name"},
66+
},
67+
},
68+
{
69+
Name: "annotations referencing found bundles preserved",
70+
NewNamespace: "test-namespace",
71+
NewName: "test-name",
72+
CSVs: []v1alpha1.ClusterServiceVersion{
73+
{
74+
ObjectMeta: metav1.ObjectMeta{
75+
Namespace: "found-namespace",
76+
Name: "found-name",
77+
},
78+
},
79+
},
80+
Before: []alongside.NamespacedName{
81+
{Namespace: "found-namespace", Name: "found-name"},
82+
},
83+
After: []alongside.NamespacedName{
84+
{Namespace: "found-namespace", Name: "found-name"},
85+
{Namespace: "test-namespace", Name: "test-name"},
86+
},
87+
},
88+
{
89+
Name: "nothing added if namespace empty",
90+
NewNamespace: "",
91+
NewName: "test-name",
92+
CSVs: []v1alpha1.ClusterServiceVersion{
93+
{
94+
ObjectMeta: metav1.ObjectMeta{
95+
Namespace: "found-namespace",
96+
Name: "found-name",
97+
},
98+
},
99+
},
100+
Before: []alongside.NamespacedName{
101+
{Namespace: "found-namespace", Name: "found-name"},
102+
},
103+
After: []alongside.NamespacedName{
104+
{Namespace: "found-namespace", Name: "found-name"},
105+
},
106+
},
107+
{
108+
Name: "nothing added if name empty",
109+
NewNamespace: "test-namespace",
110+
NewName: "",
111+
CSVs: []v1alpha1.ClusterServiceVersion{
112+
{
113+
ObjectMeta: metav1.ObjectMeta{
114+
Namespace: "found-namespace",
115+
Name: "found-name",
116+
},
117+
},
118+
},
119+
Before: []alongside.NamespacedName{
120+
{Namespace: "found-namespace", Name: "found-name"},
121+
},
122+
After: []alongside.NamespacedName{
123+
{Namespace: "found-namespace", Name: "found-name"},
124+
},
125+
},
126+
} {
127+
t.Run(tc.Name, func(t *testing.T) {
128+
csvsByNamespace := make(map[string][]*v1alpha1.ClusterServiceVersion)
129+
for _, csv := range tc.CSVs {
130+
csvsByNamespace[csv.GetNamespace()] = append(csvsByNamespace[csv.GetNamespace()], csv.DeepCopy())
131+
}
132+
133+
nsListers := make(map[string]v1alpha1listers.ClusterServiceVersionNamespaceLister)
134+
for ns, csvs := range csvsByNamespace {
135+
ns := ns
136+
csvs := csvs
137+
nslister := &operatorlisterfakes.FakeClusterServiceVersionNamespaceLister{}
138+
nslister.GetCalls(func(name string) (*v1alpha1.ClusterServiceVersion, error) {
139+
for _, csv := range csvs {
140+
if csv.GetName() == name {
141+
return csv, nil
142+
}
143+
}
144+
return nil, errors.NewNotFound(schema.GroupResource{}, name)
145+
})
146+
nsListers[ns] = nslister
147+
}
148+
149+
emptyLister := &operatorlisterfakes.FakeClusterServiceVersionNamespaceLister{}
150+
emptyLister.GetCalls(func(name string) (*v1alpha1.ClusterServiceVersion, error) {
151+
return nil, errors.NewNotFound(schema.GroupResource{}, name)
152+
})
153+
154+
csvLister := &operatorlisterfakes.FakeClusterServiceVersionLister{}
155+
csvLister.ClusterServiceVersionsCalls(func(namespace string) v1alpha1listers.ClusterServiceVersionNamespaceLister {
156+
if lister, ok := nsListers[namespace]; ok {
157+
return lister
158+
}
159+
return emptyLister
160+
})
161+
162+
var (
163+
dst, src metav1.ObjectMeta
164+
a alongside.Annotator
165+
)
166+
a.ToObject(&src, tc.Before)
167+
setInstalledAlongsideAnnotation(a, &dst, tc.NewNamespace, tc.NewName, csvLister, &src)
168+
after := a.FromObject(&dst)
169+
assert.ElementsMatch(t, tc.After, after)
170+
})
171+
}
172+
}

0 commit comments

Comments
 (0)