Skip to content

Commit

Permalink
feat: add GVK existence check for OLM supported resources by querying…
Browse files Browse the repository at this point in the history
… discovery in

case of 404 not found errors when applying installplan steps.
  • Loading branch information
exdx committed Jul 21, 2021
1 parent b4a2199 commit c267427
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 10 deletions.
69 changes: 69 additions & 0 deletions pkg/controller/operators/catalog/not_found.go
Original file line number Diff line number Diff line change
@@ -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
}
12 changes: 12 additions & 0 deletions pkg/controller/operators/catalog/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down
89 changes: 86 additions & 3 deletions test/e2e/deprecated_e2e_test.go
Original file line number Diff line number Diff line change
@@ -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...)
})
})
7 changes: 0 additions & 7 deletions test/e2e/installplan_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}

0 comments on commit c267427

Please sign in to comment.