Skip to content

Annotate CRDs that are installed alongside CSVs. #2114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/controller/operators/catalog/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
o.logger.Errorf("failed to get a client for plan execution- %v", err)
return err
}
b := newBuilder(builderKubeClient, builderDynamicClient, o.csvProvidedAPIsIndexer, r, o.logger)
b := newBuilder(plan, o.lister.OperatorsV1alpha1().ClusterServiceVersionLister(), builderKubeClient, builderDynamicClient, r, o.logger)

for i, step := range plan.Status.Plan {
doStep := true
Expand Down
103 changes: 73 additions & 30 deletions pkg/controller/operators/catalog/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,22 @@ import (
"context"
"fmt"

"k8s.io/client-go/dynamic"
"k8s.io/client-go/tools/cache"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is this supposed to be part of the following group of imports?


"github.com/operator-framework/api/pkg/operators/v1alpha1"
crdlib "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/crd"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
errorwrap "github.com/pkg/errors"
logger "github.com/sirupsen/logrus"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
apiextensionsv1beta1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"

k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic"

"github.com/operator-framework/api/pkg/operators/v1alpha1"
listersv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/alongside"
crdlib "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/crd"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
)

// Stepper manages cluster interactions based on the step.
Expand All @@ -36,21 +37,24 @@ func (s StepperFunc) Status() (v1alpha1.StepStatus, error) {
// Builder holds clients and data structures required for the StepBuilder to work
// Builder attributes are not to meant to be accessed outside the StepBuilder method
type builder struct {
opclient operatorclient.ClientInterface
dynamicClient dynamic.Interface
manifestResolver ManifestResolver
csvToProvidedAPIs map[string]cache.Indexer
logger logger.FieldLogger
plan *v1alpha1.InstallPlan
csvLister listersv1alpha1.ClusterServiceVersionLister
opclient operatorclient.ClientInterface
dynamicClient dynamic.Interface
manifestResolver ManifestResolver
logger logrus.FieldLogger

annotator alongside.Annotator
}

func newBuilder(opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, csvToProvidedAPIs map[string]cache.Indexer,
manifestResolver ManifestResolver, logger logger.FieldLogger) *builder {
func newBuilder(plan *v1alpha1.InstallPlan, csvLister listersv1alpha1.ClusterServiceVersionLister, opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, manifestResolver ManifestResolver, logger logrus.FieldLogger) *builder {
return &builder{
opclient: opclient,
dynamicClient: dynamicClient,
manifestResolver: manifestResolver,
csvToProvidedAPIs: csvToProvidedAPIs,
logger: logger,
plan: plan,
csvLister: csvLister,
opclient: opclient,
dynamicClient: dynamicClient,
manifestResolver: manifestResolver,
logger: logger,
}
}

Expand All @@ -75,6 +79,7 @@ func (b *builder) create(step v1alpha1.Step) (Stepper, error) {
if err != nil {
return nil, err
}

switch version {
case crdlib.V1Version:
return b.NewCRDV1Step(b.opclient.ApiextensionsInterface().ApiextensionsV1(), &step, manifest), nil
Expand All @@ -98,7 +103,7 @@ func (b *builder) NewCRDV1Step(client apiextensionsv1client.ApiextensionsV1Inter
if k8serrors.IsNotFound(err) {
return v1alpha1.StepStatusNotPresent, nil
} else {
return v1alpha1.StepStatusNotPresent, errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name)
return v1alpha1.StepStatusNotPresent, errors.Wrapf(err, "error finding the %s CRD", crd.Name)
}
}
established, namesAccepted := false, false
Expand All @@ -123,28 +128,31 @@ func (b *builder) NewCRDV1Step(client apiextensionsv1client.ApiextensionsV1Inter
return v1alpha1.StepStatusUnknown, err
}

setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd)

_, createError := client.CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
if k8serrors.IsAlreadyExists(createError) {
currentCRD, _ := client.CustomResourceDefinitions().Get(context.TODO(), crd.GetName(), metav1.GetOptions{})
crd.SetResourceVersion(currentCRD.GetResourceVersion())
if err = validateV1CRDCompatibility(b.dynamicClient, currentCRD, crd); err != nil {
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error validating existing CRs against new CRD's schema: %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error validating existing CRs against new CRD's schema: %s", step.Resource.Name)
}

// check to see if stored versions changed and whether the upgrade could cause potential data loss
safe, err := crdlib.SafeStorageVersionUpgrade(currentCRD, crd)
if !safe {
b.logger.Errorf("risk of data loss updating %s: %s", step.Resource.Name, err)
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
}
if err != nil {
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
}

// Update CRD to new version
setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd, currentCRD)
_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
if err != nil {
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
}
// If it already existed, mark the step as Present.
// they were equal - mark CRD as present
Expand Down Expand Up @@ -173,7 +181,7 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
if k8serrors.IsNotFound(err) {
return v1alpha1.StepStatusNotPresent, nil
} else {
return v1alpha1.StepStatusNotPresent, errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name)
return v1alpha1.StepStatusNotPresent, errors.Wrapf(err, "error finding the %s CRD", crd.Name)
}
}
established, namesAccepted := false, false
Expand All @@ -198,29 +206,32 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
return v1alpha1.StepStatusUnknown, err
}

setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd)

_, createError := client.CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
if k8serrors.IsAlreadyExists(createError) {
currentCRD, _ := client.CustomResourceDefinitions().Get(context.TODO(), crd.GetName(), metav1.GetOptions{})
crd.SetResourceVersion(currentCRD.GetResourceVersion())

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

// check to see if stored versions changed and whether the upgrade could cause potential data loss
safe, err := crdlib.SafeStorageVersionUpgrade(currentCRD, crd)
if !safe {
b.logger.Errorf("risk of data loss updating %s: %s", step.Resource.Name, err)
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "risk of data loss updating %s", step.Resource.Name)
}
if err != nil {
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "checking CRD for potential data loss updating %s", step.Resource.Name)
}

// Update CRD to new version
setInstalledAlongsideAnnotation(b.annotator, crd, b.plan.GetNamespace(), step.Resolving, b.csvLister, crd, currentCRD)
_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
if err != nil {
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
return v1alpha1.StepStatusUnknown, errors.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
}
// If it already existed, mark the step as Present.
// they were equal - mark CRD as present
Expand All @@ -235,3 +246,35 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
return v1alpha1.StepStatusUnknown, nil
}
}

func setInstalledAlongsideAnnotation(a alongside.Annotator, dst metav1.Object, namespace string, name string, lister listersv1alpha1.ClusterServiceVersionLister, srcs ...metav1.Object) {
var (
nns []alongside.NamespacedName
)

// Only keep references to existing and non-copied CSVs to
// avoid unbounded growth.
for _, src := range srcs {
for _, nn := range a.FromObject(src) {
if nn.Namespace == namespace && nn.Name == name {
continue
}

if csv, err := lister.ClusterServiceVersions(nn.Namespace).Get(nn.Name); k8serrors.IsNotFound(err) {
continue
} else if err == nil && csv.IsCopied() {
continue
}
// CSV exists and is not copied OR err is non-nil, but
// not 404. Safer to assume it exists in unhandled
// error cases and try again next time.
nns = append(nns, nn)
}
}

if namespace != "" && name != "" {
nns = append(nns, alongside.NamespacedName{Namespace: namespace, Name: name})
}

a.ToObject(dst, nns)
}
172 changes: 172 additions & 0 deletions pkg/controller/operators/catalog/step_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
package catalog

import (
"testing"

"github.com/stretchr/testify/assert"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/operator-framework/api/pkg/operators/v1alpha1"
v1alpha1listers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/alongside"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes"
)

func TestSetInstalledAlongsideAnnotation(t *testing.T) {
for _, tc := range []struct {
Name string
NewNamespace string
NewName string
CSVs []v1alpha1.ClusterServiceVersion
Before []alongside.NamespacedName
After []alongside.NamespacedName
}{
{
Name: "object annotated with bundle name",
NewNamespace: "test-namespace",
NewName: "test-name",
After: []alongside.NamespacedName{
{Namespace: "test-namespace", Name: "test-name"},
},
},
{
Name: "annotations referencing missing bundles removed",
NewNamespace: "test-namespace",
NewName: "test-name",
Before: []alongside.NamespacedName{
{Namespace: "missing-namespace", Name: "missing-name"},
},
After: []alongside.NamespacedName{
{Namespace: "test-namespace", Name: "test-name"},
},
},
{
Name: "annotations referencing copied csv removed",
NewNamespace: "test-namespace",
NewName: "test-name",
CSVs: []v1alpha1.ClusterServiceVersion{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "copied-namespace",
Name: "copied-name",
},
Status: v1alpha1.ClusterServiceVersionStatus{
Reason: v1alpha1.CSVReasonCopied,
},
},
},
Before: []alongside.NamespacedName{
{Namespace: "copied-namespace", Name: "copied-name"},
},
After: []alongside.NamespacedName{
{Namespace: "test-namespace", Name: "test-name"},
},
},
{
Name: "annotations referencing found bundles preserved",
NewNamespace: "test-namespace",
NewName: "test-name",
CSVs: []v1alpha1.ClusterServiceVersion{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "found-namespace",
Name: "found-name",
},
},
},
Before: []alongside.NamespacedName{
{Namespace: "found-namespace", Name: "found-name"},
},
After: []alongside.NamespacedName{
{Namespace: "found-namespace", Name: "found-name"},
{Namespace: "test-namespace", Name: "test-name"},
},
},
{
Name: "nothing added if namespace empty",
NewNamespace: "",
NewName: "test-name",
CSVs: []v1alpha1.ClusterServiceVersion{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "found-namespace",
Name: "found-name",
},
},
},
Before: []alongside.NamespacedName{
{Namespace: "found-namespace", Name: "found-name"},
},
After: []alongside.NamespacedName{
{Namespace: "found-namespace", Name: "found-name"},
},
},
{
Name: "nothing added if name empty",
NewNamespace: "test-namespace",
NewName: "",
CSVs: []v1alpha1.ClusterServiceVersion{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "found-namespace",
Name: "found-name",
},
},
},
Before: []alongside.NamespacedName{
{Namespace: "found-namespace", Name: "found-name"},
},
After: []alongside.NamespacedName{
{Namespace: "found-namespace", Name: "found-name"},
},
},
} {
t.Run(tc.Name, func(t *testing.T) {
csvsByNamespace := make(map[string][]*v1alpha1.ClusterServiceVersion)
for _, csv := range tc.CSVs {
csvsByNamespace[csv.GetNamespace()] = append(csvsByNamespace[csv.GetNamespace()], csv.DeepCopy())
}

nsListers := make(map[string]v1alpha1listers.ClusterServiceVersionNamespaceLister)
for ns, csvs := range csvsByNamespace {
ns := ns
csvs := csvs
nslister := &operatorlisterfakes.FakeClusterServiceVersionNamespaceLister{}
nslister.GetCalls(func(name string) (*v1alpha1.ClusterServiceVersion, error) {
for _, csv := range csvs {
if csv.GetName() == name {
return csv, nil
}
}
return nil, errors.NewNotFound(schema.GroupResource{}, name)
})
nsListers[ns] = nslister
}

emptyLister := &operatorlisterfakes.FakeClusterServiceVersionNamespaceLister{}
emptyLister.GetCalls(func(name string) (*v1alpha1.ClusterServiceVersion, error) {
return nil, errors.NewNotFound(schema.GroupResource{}, name)
})

csvLister := &operatorlisterfakes.FakeClusterServiceVersionLister{}
csvLister.ClusterServiceVersionsCalls(func(namespace string) v1alpha1listers.ClusterServiceVersionNamespaceLister {
if lister, ok := nsListers[namespace]; ok {
return lister
}
return emptyLister
})

var (
dst, src metav1.ObjectMeta
a alongside.Annotator
)
a.ToObject(&src, tc.Before)
setInstalledAlongsideAnnotation(a, &dst, tc.NewNamespace, tc.NewName, csvLister, &src)
after := a.FromObject(&dst)
assert.ElementsMatch(t, tc.After, after)
})
}
}
Loading