From c2674279c862b3b8264efddc931eee250c52beb5 Mon Sep 17 00:00:00 2001 From: Daniel Sover Date: Tue, 29 Jun 2021 12:57:21 -0400 Subject: [PATCH] feat: add GVK existence check for OLM supported resources by querying discovery in case of 404 not found errors when applying installplan steps. --- pkg/controller/operators/catalog/not_found.go | 69 ++++++++++++++ pkg/controller/operators/catalog/operator.go | 12 +++ test/e2e/deprecated_e2e_test.go | 89 ++++++++++++++++++- test/e2e/installplan_e2e_test.go | 7 -- test/e2e/util_test.go | 14 +++ 5 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 pkg/controller/operators/catalog/not_found.go diff --git a/pkg/controller/operators/catalog/not_found.go b/pkg/controller/operators/catalog/not_found.go new file mode 100644 index 0000000000..8cf4a3b991 --- /dev/null +++ b/pkg/controller/operators/catalog/not_found.go @@ -0,0 +1,69 @@ +package catalog + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" +) + +// gvkNotFoundError is returned from installplan execution when a step contains a GVK that is not found on cluster. +type gvkNotFoundError struct { + gvk schema.GroupVersionKind + name string +} + +func (g gvkNotFoundError) Error() string { + return fmt.Sprintf("api-server resource not found installing %s %s: GroupVersionKind %s not found on the cluster. %s", g.gvk.Kind, g.name, g.gvk, + "This API may have been deprecated and removed, see https://kubernetes.io/docs/reference/using-api/deprecation-guide/ for more information.") +} + +type DiscoveryQuerier interface { + QueryForGVK() error +} + +type DiscoveryQuerierFunc func() error + +func (d DiscoveryQuerierFunc) QueryForGVK() error { + return d() +} + +type discoveryQuerier struct { + client discovery.DiscoveryInterface +} + +func newDiscoveryQuerier(client discovery.DiscoveryInterface) *discoveryQuerier { + return &discoveryQuerier{client: client} +} + +// WithStepResource returns a DiscoveryQuerier which uses discovery to query for supported APIs on the server based on the provided step's GVK. +func (d *discoveryQuerier) WithStepResource(stepResource operatorsv1alpha1.StepResource) DiscoveryQuerier { + var f DiscoveryQuerierFunc = func() error { + gvk := schema.GroupVersionKind{Group: stepResource.Group, Version: stepResource.Version, Kind: stepResource.Kind} + + resourceList, err := d.client.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) + if err != nil { + if errors.IsNotFound(err) { + return gvkNotFoundError{gvk: gvk, name: stepResource.Name} + } + return err + } + + if resourceList == nil { + return gvkNotFoundError{gvk: gvk, name: stepResource.Name} + } + + for _, resource := range resourceList.APIResources { + if resource.Kind == stepResource.Kind { + // this kind is supported for this particular GroupVersion + return nil + } + } + + return gvkNotFoundError{gvk: gvk, name: stepResource.Name} + } + return f +} diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go index 6aec1d1ffe..6d057af5f0 100644 --- a/pkg/controller/operators/catalog/operator.go +++ b/pkg/controller/operators/catalog/operator.go @@ -11,6 +11,8 @@ import ( "sync" "time" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + errorwrap "github.com/pkg/errors" "github.com/sirupsen/logrus" "google.golang.org/grpc/connectivity" @@ -1774,6 +1776,8 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error { ensurer := newStepEnsurer(kubeclient, crclient, dynamicClient) r := newManifestResolver(plan.GetNamespace(), o.lister.CoreV1().ConfigMapLister(), o.logger) + discoveryQuerier := newDiscoveryQuerier(o.opClient.KubernetesInterface().Discovery()) + // CRDs should be installed via the default OLM (cluster-admin) client and not the scoped client specified by the AttenuatedServiceAccount // the StepBuilder is currently only implemented for CRD types // TODO give the StepBuilder both OLM and scoped clients when it supports new scoped types @@ -2173,6 +2177,14 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error { } return nil }(i, step); err != nil { + if k8serrors.IsNotFound(err) { + // Check for APIVersions present in the installplan steps that are not available on the server. + // The check is made via discovery per step in the plan. Transient communication failures to the api-server are handled by the plan retry logic. + notFoundErr := discoveryQuerier.WithStepResource(step.Resource).QueryForGVK() + if notFoundErr != nil { + return notFoundErr + } + } return err } } diff --git a/test/e2e/deprecated_e2e_test.go b/test/e2e/deprecated_e2e_test.go index f411d7a174..f9d66cbeb1 100644 --- a/test/e2e/deprecated_e2e_test.go +++ b/test/e2e/deprecated_e2e_test.go @@ -1,6 +1,89 @@ package e2e -// TODO +import ( + "context" + "time" -// v1beta1 CRD in installplan fails -// v1beta1 RBAC in an installplan fails + "github.com/blang/semver/v4" + . "github.com/onsi/ginkgo" + "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx" +) + +var missingAPI = `{"apiVersion":"verticalpodautoscalers.autoscaling.k8s.io/v1","kind":"VerticalPodAutoscaler","metadata":{"name":"my.thing","namespace":"foo"}}` + +var _ = Describe("Not found APIs", func() { + BeforeEach(func() { + csv := newCSV("test-csv", testNamespace, "", semver.Version{}, nil, nil, nil) + Expect(ctx.Ctx().Client().Create(context.TODO(), &csv)).To(Succeed()) + }) + AfterEach(func() { + TearDown(testNamespace) + }) + + When("objects with APIs that are not on-cluster are created in the installplan", func() { + // each entry is an installplan with a deprecated resource + type payload struct { + name string + ip *operatorsv1alpha1.InstallPlan + errMessage string + } + + var tableEntries []table.TableEntry + tableEntries = []table.TableEntry{ + table.Entry("contains an entry with a missing API not found on cluster ", payload{ + name: "installplan contains a missing API", + ip: &operatorsv1alpha1.InstallPlan{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: *namespace, // this is necessary due to ginkgo table semantics, see https://github.com/onsi/ginkgo/issues/378 + Name: "test-plan-api", + }, + Spec: operatorsv1alpha1.InstallPlanSpec{ + Approval: operatorsv1alpha1.ApprovalAutomatic, + Approved: true, + ClusterServiceVersionNames: []string{}, + }, + Status: operatorsv1alpha1.InstallPlanStatus{ + Phase: operatorsv1alpha1.InstallPlanPhaseInstalling, + CatalogSources: []string{}, + Plan: []*operatorsv1alpha1.Step{ + { + Resolving: "test-csv", + Status: operatorsv1alpha1.StepStatusUnknown, + Resource: operatorsv1alpha1.StepResource{ + Name: "my.thing", + Group: "verticalpodautoscalers.autoscaling.k8s.io", + Version: "v1", + Kind: "VerticalPodAutoscaler", + Manifest: missingAPI, + }, + }, + }, + }, + }, + errMessage: "api-server resource not found installing VerticalPodAutoscaler my.thing: GroupVersionKind " + + "verticalpodautoscalers.autoscaling.k8s.io/v1, Kind=VerticalPodAutoscaler not found on the cluster", + }), + } + + table.DescribeTable("the ip enters a failed state with a helpful error message", func(tt payload) { + Expect(ctx.Ctx().Client().Create(context.Background(), tt.ip)).To(Succeed()) + Expect(ctx.Ctx().Client().Status().Update(context.Background(), tt.ip)).To(Succeed()) + + // The IP sits in the Installing phase with the GVK missing error + Eventually(func() (*operatorsv1alpha1.InstallPlan, error) { + return tt.ip, ctx.Ctx().Client().Get(context.Background(), client.ObjectKeyFromObject(tt.ip), tt.ip) + }).Should(And(HavePhase(operatorsv1alpha1.InstallPlanPhaseInstalling)), HaveMessage(tt.errMessage)) + + // Eventually the IP fails with the GVK missing error, after installplan retries, which is by default 1 minute. + Eventually(func() (*operatorsv1alpha1.InstallPlan, error) { + return tt.ip, ctx.Ctx().Client().Get(context.Background(), client.ObjectKeyFromObject(tt.ip), tt.ip) + }, 2*time.Minute).Should(And(HavePhase(operatorsv1alpha1.InstallPlanPhaseFailed)), HaveMessage(tt.errMessage)) + }, tableEntries...) + }) +}) diff --git a/test/e2e/installplan_e2e_test.go b/test/e2e/installplan_e2e_test.go index 9cb87e23d9..dea25b5b2c 100644 --- a/test/e2e/installplan_e2e_test.go +++ b/test/e2e/installplan_e2e_test.go @@ -16,7 +16,6 @@ import ( "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gstruct" - "github.com/onsi/gomega/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" @@ -53,12 +52,6 @@ import ( ) var _ = Describe("Install Plan", func() { - HavePhase := func(goal operatorsv1alpha1.InstallPlanPhase) types.GomegaMatcher { - return WithTransform(func(plan *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase { - return plan.Status.Phase - }, Equal(goal)) - } - AfterEach(func() { TearDown(testNamespace) }) diff --git a/test/e2e/util_test.go b/test/e2e/util_test.go index 284d14d7ee..18be4dddf3 100644 --- a/test/e2e/util_test.go +++ b/test/e2e/util_test.go @@ -35,7 +35,9 @@ import ( "k8s.io/client-go/rest" k8scontrollerclient "sigs.k8s.io/controller-runtime/pkg/client" + gtypes "github.com/onsi/gomega/types" "github.com/operator-framework/api/pkg/operators/v1alpha1" + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry" controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client" @@ -897,3 +899,15 @@ func deploymentReplicas(replicas int32) predicateFunc { func Apply(obj controllerclient.Object, changeFunc interface{}) func() error { return ctx.Ctx().SSAClient().Apply(context.Background(), obj, changeFunc) } + +func HavePhase(goal operatorsv1alpha1.InstallPlanPhase) gtypes.GomegaMatcher { + return WithTransform(func(plan *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase { + return plan.Status.Phase + }, Equal(goal)) +} + +func HaveMessage(goal string) gtypes.GomegaMatcher { + return WithTransform(func(plan *operatorsv1alpha1.InstallPlan) string { + return plan.Status.Message + }, ContainSubstring(goal)) +}