From 66fd308cff1f469fdc7a0625700b40348e39122d Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 10 Sep 2019 12:38:09 -0700 Subject: [PATCH 1/8] internal/olm: make CLI output text more consistent; check resource errors --- internal/olm/client.go | 3 ++- internal/olm/client/client.go | 14 +++++++------- internal/olm/client/status.go | 26 ++++++++++++++------------ 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/internal/olm/client.go b/internal/olm/client.go index f90953af2d..c06ecbaaa5 100644 --- a/internal/olm/client.go +++ b/internal/olm/client.go @@ -73,7 +73,8 @@ func (c Client) InstallVersion(ctx context.Context, version string) (*olmresourc status := c.GetObjectsStatus(ctx, objs...) if status.HasExistingResources() { - return nil, errors.New("detected existing OLM resources: OLM must be completely uninstalled before installation") + return nil, errors.New("detected existing OLM resources: OLM must be completely uninstalled before installation" + + fmt.Sprintf("\nResources:\n%s", status.String())) } log.Print("Creating CRDs and resources") diff --git a/internal/olm/client/client.go b/internal/olm/client/client.go index f9f2f126fe..f98e36aa65 100644 --- a/internal/olm/client/client.go +++ b/internal/olm/client/client.go @@ -152,27 +152,27 @@ func (c Client) DoRolloutWait(ctx context.Context, key types.NamespacedName) err } if deployment.Spec.Replicas != nil && deployment.Status.UpdatedReplicas < *deployment.Spec.Replicas { onceReplicasUpdated.Do(func() { - log.Printf(" Waiting for deployment %q to rollout: %d out of %d new replicas have been updated", deployment.Name, deployment.Status.UpdatedReplicas, *deployment.Spec.Replicas) + log.Printf(" Waiting for Deployment %q to rollout: %d out of %d new replicas have been updated", key, deployment.Status.UpdatedReplicas, *deployment.Spec.Replicas) }) return false, nil } if deployment.Status.Replicas > deployment.Status.UpdatedReplicas { oncePendingTermination.Do(func() { - log.Printf(" Waiting for deployment %q to rollout: %d old replicas are pending termination", deployment.Name, deployment.Status.Replicas-deployment.Status.UpdatedReplicas) + log.Printf(" Waiting for Deployment %q to rollout: %d old replicas are pending termination", key, deployment.Status.Replicas-deployment.Status.UpdatedReplicas) }) return false, nil } if deployment.Status.AvailableReplicas < deployment.Status.UpdatedReplicas { onceNotAvailable.Do(func() { - log.Printf(" Waiting for deployment %q to rollout: %d of %d updated replicas are available", deployment.Name, deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas) + log.Printf(" Waiting for Deployment %q to rollout: %d of %d updated replicas are available", key, deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas) }) return false, nil } - log.Printf(" Deployment %q successfully rolled out", deployment.Name) + log.Printf(" Deployment %q successfully rolled out", key) return true, nil } onceSpecUpdate.Do(func() { - log.Printf(" Waiting for deployment %q to rollout: waiting for deployment spec update to be observed", deployment.Name) + log.Printf(" Waiting for Deployment %q to rollout: waiting for deployment spec update to be observed", key) }) return false, nil } @@ -192,7 +192,7 @@ func (c Client) DoCSVWait(ctx context.Context, key types.NamespacedName) error { if err != nil { if apierrors.IsNotFound(err) { once.Do(func() { - log.Printf(" Waiting for clusterserviceversion %q to appear", key.Name) + log.Printf(" Waiting for ClusterServiceVersion %q to appear", key) }) return false, nil } @@ -201,7 +201,7 @@ func (c Client) DoCSVWait(ctx context.Context, key types.NamespacedName) error { newPhase = csv.Status.Phase if newPhase != curPhase { curPhase = newPhase - log.Printf(" Found clusterserviceversion %q phase: %s", key.Name, curPhase) + log.Printf(" Found ClusterServiceVersion %q phase: %s", key, curPhase) } return curPhase == olmapiv1alpha1.CSVPhaseSucceeded, nil } diff --git a/internal/olm/client/status.go b/internal/olm/client/status.go index 98f4c88c8a..04b0865b57 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -24,6 +24,7 @@ import ( "text/tabwriter" log "github.com/sirupsen/logrus" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -45,24 +46,23 @@ type ResourceStatus struct { func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) Status { var rss []ResourceStatus for _, obj := range objs { + gvk := obj.GetObjectKind().GroupVersionKind() a, aerr := meta.Accessor(obj) if aerr != nil { - log.Fatalf("Object %s: %v", obj.GetObjectKind().GroupVersionKind(), aerr) + log.Fatalf("Object %s: %v", gvk, aerr) } nn := types.NamespacedName{ Namespace: a.GetNamespace(), Name: a.GetName(), } - u := unstructured.Unstructured{} - u.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) - err := c.KubeClient.Get(ctx, nn, &u) rs := ResourceStatus{ NamespacedName: nn, - GVK: obj.GetObjectKind().GroupVersionKind(), + GVK: gvk, } - if err != nil { - rs.Error = err - } else { + u := unstructured.Unstructured{} + u.SetGroupVersionKind(gvk) + rs.Error = c.KubeClient.Get(ctx, nn, &u) + if rs.Error == nil { rs.Resource = &u } rss = append(rss, rs) @@ -73,7 +73,9 @@ func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) St func (s Status) HasExistingResources() bool { for _, r := range s.Resources { - if r.Resource != nil { + // Either the resource was found and returned without error or returned + // an existence error. + if r.Resource != nil || (r.Error != nil && !apierrors.IsNotFound(r.Error)) { return true } } @@ -88,10 +90,10 @@ func (s Status) String() string { nn := r.NamespacedName kind := r.GVK.Kind var status string - if r.Resource != nil { - status = "Installed" - } else if r.Error != nil { + if r.Error != nil { status = r.Error.Error() + } else if r.Resource != nil { + status = "Installed" } else { status = "Unknown" } From cc6ebc00be4da71f05e33b6adc3e0857c9a89d67 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 12 Sep 2019 11:17:10 -0700 Subject: [PATCH 2/8] NoExistingResources checks if all resources are deleted --- internal/olm/client.go | 8 ++++---- internal/olm/client/client.go | 6 ++---- internal/olm/client/status.go | 15 +++++++++++---- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/internal/olm/client.go b/internal/olm/client.go index c06ecbaaa5..cd2f32a6d9 100644 --- a/internal/olm/client.go +++ b/internal/olm/client.go @@ -72,8 +72,8 @@ func (c Client) InstallVersion(ctx context.Context, version string) (*olmresourc objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if status.HasExistingResources() { - return nil, errors.New("detected existing OLM resources: OLM must be completely uninstalled before installation" + + if status.HasInstalledResources() { + return nil, errors.New("detected existing or errored OLM resources: OLM must be completely uninstalled before installation" + fmt.Sprintf("\nResources:\n%s", status.String())) } @@ -130,7 +130,7 @@ func (c Client) UninstallVersion(ctx context.Context, version string) error { objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if !status.HasExistingResources() { + if status.NoExistingResources() { return olmresourceclient.ErrOLMNotInstalled } @@ -149,7 +149,7 @@ func (c Client) GetStatus(ctx context.Context, version string) (*olmresourceclie objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if !status.HasExistingResources() { + if status.NoExistingResources() { return nil, olmresourceclient.ErrOLMNotInstalled } return &status, nil diff --git a/internal/olm/client/client.go b/internal/olm/client/client.go index f98e36aa65..1d96a91cb7 100644 --- a/internal/olm/client/client.go +++ b/internal/olm/client/client.go @@ -118,12 +118,10 @@ func (c Client) DoDelete(ctx context.Context, objs ...runtime.Object) error { } log.Infof(" Waiting for deleted resources to disappear") - wait.PollImmediateUntil(time.Second, func() (bool, error) { + return wait.PollImmediateUntil(time.Second, func() (bool, error) { s := c.GetObjectsStatus(ctx, objs...) - return !s.HasExistingResources(), nil + return s.NoExistingResources(), nil }, ctx.Done()) - - return nil } func getName(namespace, name string) string { diff --git a/internal/olm/client/status.go b/internal/olm/client/status.go index 04b0865b57..fed9b8cc0b 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -71,17 +71,24 @@ func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) St return Status{Resources: rss} } -func (s Status) HasExistingResources() bool { +func (s Status) HasInstalledResources() bool { for _, r := range s.Resources { - // Either the resource was found and returned without error or returned - // an existence error. - if r.Resource != nil || (r.Error != nil && !apierrors.IsNotFound(r.Error)) { + if r.Resource != nil && r.Error == nil { return true } } return false } +func (s Status) NoExistingResources() bool { + for _, r := range s.Resources { + if r.Resource != nil || r.Error != nil && !apierrors.IsNotFound(r.Error) { + return false + } + } + return true +} + func (s Status) String() string { out := &bytes.Buffer{} tw := tabwriter.NewWriter(out, 8, 4, 4, ' ', 0) From 5fe8c79fbf1ae28ee69fc40f23a8bf5119f2c22d Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 12 Sep 2019 11:50:17 -0700 Subject: [PATCH 3/8] fix subcommand test string check --- hack/tests/alpha-olm-subcommands.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/tests/alpha-olm-subcommands.sh b/hack/tests/alpha-olm-subcommands.sh index ac951d4443..4143adbf66 100755 --- a/hack/tests/alpha-olm-subcommands.sh +++ b/hack/tests/alpha-olm-subcommands.sh @@ -25,7 +25,7 @@ test_version() { # Install should fail with OLM Installed commandoutput=$(operator-sdk alpha olm install $ver_flag 2>&1 || true) - echo $commandoutput | grep -F "Failed to install OLM version \\\"${version}\\\": detected existing OLM resources: OLM must be completely uninstalled before installation" + echo $commandoutput | grep -F "Failed to install OLM version \\\"${version}\\\": detected existing or errored OLM resources: OLM must be completely uninstalled before installation" # Status should succeed with OLM installed commandoutput=$(operator-sdk alpha olm status 2>&1) From b17a8b95c262cb33bf650de0ece3a5cb97e9ac21 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 31 Oct 2019 12:02:18 -0700 Subject: [PATCH 4/8] return bool and error from HasInstalledResources --- hack/tests/subcommand-olm-install.sh | 2 +- internal/olm/client.go | 14 ++++++++++---- internal/olm/client/client.go | 3 ++- internal/olm/client/status.go | 19 ++++++------------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hack/tests/subcommand-olm-install.sh b/hack/tests/subcommand-olm-install.sh index 4143adbf66..ac951d4443 100755 --- a/hack/tests/subcommand-olm-install.sh +++ b/hack/tests/subcommand-olm-install.sh @@ -25,7 +25,7 @@ test_version() { # Install should fail with OLM Installed commandoutput=$(operator-sdk alpha olm install $ver_flag 2>&1 || true) - echo $commandoutput | grep -F "Failed to install OLM version \\\"${version}\\\": detected existing or errored OLM resources: OLM must be completely uninstalled before installation" + echo $commandoutput | grep -F "Failed to install OLM version \\\"${version}\\\": detected existing OLM resources: OLM must be completely uninstalled before installation" # Status should succeed with OLM installed commandoutput=$(operator-sdk alpha olm status 2>&1) diff --git a/internal/olm/client.go b/internal/olm/client.go index cd2f32a6d9..c6384918ca 100644 --- a/internal/olm/client.go +++ b/internal/olm/client.go @@ -72,8 +72,12 @@ func (c Client) InstallVersion(ctx context.Context, version string) (*olmresourc objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if status.HasInstalledResources() { - return nil, errors.New("detected existing or errored OLM resources: OLM must be completely uninstalled before installation" + + installed, err := status.HasInstalledResources() + if installed { + return nil, errors.New("detected existing OLM resources: OLM must be completely uninstalled before installation" + + fmt.Sprintf("\nResources:\n%s", status.String())) + } else if err != nil { + return nil, errors.New("detected errored OLM resources:" + fmt.Sprintf("\nResources:\n%s", status.String())) } @@ -130,7 +134,8 @@ func (c Client) UninstallVersion(ctx context.Context, version string) error { objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if status.NoExistingResources() { + installed, err := status.HasInstalledResources() + if !installed && err == nil { return olmresourceclient.ErrOLMNotInstalled } @@ -149,7 +154,8 @@ func (c Client) GetStatus(ctx context.Context, version string) (*olmresourceclie objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if status.NoExistingResources() { + installed, err := status.HasInstalledResources() + if !installed && err == nil { return nil, olmresourceclient.ErrOLMNotInstalled } return &status, nil diff --git a/internal/olm/client/client.go b/internal/olm/client/client.go index 1d96a91cb7..31b4140506 100644 --- a/internal/olm/client/client.go +++ b/internal/olm/client/client.go @@ -120,7 +120,8 @@ func (c Client) DoDelete(ctx context.Context, objs ...runtime.Object) error { log.Infof(" Waiting for deleted resources to disappear") return wait.PollImmediateUntil(time.Second, func() (bool, error) { s := c.GetObjectsStatus(ctx, objs...) - return s.NoExistingResources(), nil + installed, err := s.HasInstalledResources() + return !installed, err }, ctx.Done()) } diff --git a/internal/olm/client/status.go b/internal/olm/client/status.go index fed9b8cc0b..4f1d48895f 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -71,22 +71,15 @@ func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) St return Status{Resources: rss} } -func (s Status) HasInstalledResources() bool { +func (s Status) HasInstalledResources() (bool, error) { for _, r := range s.Resources { - if r.Resource != nil && r.Error == nil { - return true + if r.Resource != nil { + return true, nil + } else if r.Error != nil && !apierrors.IsNotFound(r.Error) { + return false, r.Error } } - return false -} - -func (s Status) NoExistingResources() bool { - for _, r := range s.Resources { - if r.Resource != nil || r.Error != nil && !apierrors.IsNotFound(r.Error) { - return false - } - } - return true + return false, nil } func (s Status) String() string { From e51494293a5feba1eaa0d7be852f7a18e933b7eb Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 8 Nov 2019 12:46:37 -0800 Subject: [PATCH 5/8] check if CR errors are "no kind match" if their CRD is "not found" --- internal/olm/client/status.go | 77 ++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/internal/olm/client/status.go b/internal/olm/client/status.go index 4f1d48895f..963f36e049 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -20,18 +20,31 @@ package olm import ( "bytes" "context" + "errors" "fmt" "text/tabwriter" log "github.com/sirupsen/logrus" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/kubernetes/scheme" ) +var sch = scheme.Scheme + +func init() { + install.Install(sch) +} + type Status struct { Resources []ResourceStatus } @@ -41,6 +54,8 @@ type ResourceStatus struct { Resource *unstructured.Unstructured GVK schema.GroupVersionKind Error error + + requestObject runtime.Object // Needed for context on errors from requests on an object. } func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) Status { @@ -58,6 +73,7 @@ func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) St rs := ResourceStatus{ NamespacedName: nn, GVK: gvk, + requestObject: obj, } u := unstructured.Unstructured{} u.SetGroupVersionKind(gvk) @@ -71,17 +87,76 @@ func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) St return Status{Resources: rss} } +// HasInstalledResources only returns true if at least one resource in s +// was returned successfully by the API server. A resource error status +// containing any error except "not found", or "no kind match" errors +// for Custom Resources, will result in HasInstalledResources returning +// false and the error. func (s Status) HasInstalledResources() (bool, error) { + crdKindSet, err := s.getCRDKindSet() + if err != nil { + return false, fmt.Errorf("error getting set of CRD kinds in resources: %w", err) + } for _, r := range s.Resources { if r.Resource != nil { return true, nil } else if r.Error != nil && !apierrors.IsNotFound(r.Error) { - return false, r.Error + nkmerr := &meta.NoKindMatchError{} + if !errors.As(r.Error, &nkmerr) || !crdKindSet.Has(r.GVK.Kind) { + return false, r.Error + } } } return false, nil } +// getCRDKindSet returns the set of all kinds specified by all CRDs in s. +func (s Status) getCRDKindSet() (sets.String, error) { + crdKindSet := sets.NewString() + dec := serializer.NewCodecFactory(sch).UniversalDeserializer() + for _, r := range s.Resources { + if r.GVK.Kind == "CustomResourceDefinition" { + switch v := r.requestObject.(type) { + case *unstructured.Unstructured: + vb, err := v.MarshalJSON() + if err != nil { + return nil, err + } + // Use unversioned CustomResourceDefinition to avoid implementing cast + // for all versions. + obj, _, err := dec.Decode(vb, nil, nil) + if err != nil { + return nil, err + } + kind, err := getVersionedCRDKind(obj) + if err != nil { + return nil, err + } + crdKindSet.Insert(kind) + default: + kind, err := getVersionedCRDKind(v) + if err != nil { + return nil, err + } + crdKindSet.Insert(kind) + } + } + } + return crdKindSet, nil +} + +// getVersionedCRDKind returns the kind of a CRD if its version is known, +// otherwise an error. +func getVersionedCRDKind(obj runtime.Object) (string, error) { + switch crd := obj.(type) { + case *apiextensions.CustomResourceDefinition: + return crd.Spec.Names.Kind, nil + case *v1beta1.CustomResourceDefinition: + return crd.Spec.Names.Kind, nil + } + return "", fmt.Errorf("error getting CRD kind: gvk %q unknown", obj.GetObjectKind().GroupVersionKind()) +} + func (s Status) String() string { out := &bytes.Buffer{} tw := tabwriter.NewWriter(out, 8, 4, 4, ' ', 0) From dc63a9c49902506902ecfadc4747f1f4d6bd8b6e Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 22 Nov 2019 13:52:41 -0800 Subject: [PATCH 6/8] internal/olm/client: DoCreate and DoDelete handle "not found" errors correctly --- internal/olm/client/client.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/olm/client/client.go b/internal/olm/client/client.go index 31b4140506..b2318ceff4 100644 --- a/internal/olm/client/client.go +++ b/internal/olm/client/client.go @@ -89,11 +89,10 @@ func (c Client) DoCreate(ctx context.Context, objs ...runtime.Object) error { log.Infof(" Creating %s %q", kind, getName(a.GetNamespace(), a.GetName())) err = c.KubeClient.Create(ctx, obj) if err != nil { - if apierrors.IsAlreadyExists(err) { - log.Infof(" %s %q already exists", kind, getName(a.GetNamespace(), a.GetName())) - return nil + if !apierrors.IsAlreadyExists(err) { + return err } - return err + log.Infof(" %s %q already exists", kind, getName(a.GetNamespace(), a.GetName())) } } return nil @@ -109,11 +108,10 @@ func (c Client) DoDelete(ctx context.Context, objs ...runtime.Object) error { log.Infof(" Deleting %s %q", kind, getName(a.GetNamespace(), a.GetName())) err = c.KubeClient.Delete(ctx, obj, client.PropagationPolicy(metav1.DeletePropagationForeground)) if err != nil { - if apierrors.IsNotFound(err) { - log.Infof(" %s %q does not exist", kind, getName(a.GetNamespace(), a.GetName())) - continue + if !apierrors.IsNotFound(err) { + return err } - return err + log.Infof(" %s %q does not exist", kind, getName(a.GetNamespace(), a.GetName())) } } From e8af055e7dfa2c642ed40586f8dcfc5c03aeee3a Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 4 Dec 2019 00:56:45 -0800 Subject: [PATCH 7/8] add comment and sort Status resources in HasInstalledResources --- internal/olm/client.go | 6 ++---- internal/olm/client/status.go | 12 ++++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/internal/olm/client.go b/internal/olm/client.go index f06448597e..72ceab7da1 100644 --- a/internal/olm/client.go +++ b/internal/olm/client.go @@ -74,11 +74,9 @@ func (c Client) InstallVersion(ctx context.Context, version string) (*olmresourc status := c.GetObjectsStatus(ctx, objs...) installed, err := status.HasInstalledResources() if installed { - return nil, errors.New("detected existing OLM resources: OLM must be completely uninstalled before installation" + - fmt.Sprintf("\nResources:\n%s", status.String())) + return nil, errors.New("detected existing OLM resources: OLM must be completely uninstalled before installation") } else if err != nil { - return nil, errors.New("detected errored OLM resources:" + - fmt.Sprintf("\nResources:\n%s", status.String())) + return nil, errors.New("detected errored OLM resources, see resource statuses for more details") } log.Print("Creating CRDs and resources") diff --git a/internal/olm/client/status.go b/internal/olm/client/status.go index 963f36e049..080a11c1dc 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -22,6 +22,7 @@ import ( "context" "errors" "fmt" + "sort" "text/tabwriter" log "github.com/sirupsen/logrus" @@ -97,10 +98,21 @@ func (s Status) HasInstalledResources() (bool, error) { if err != nil { return false, fmt.Errorf("error getting set of CRD kinds in resources: %w", err) } + // Sort resources by whether they're installed or not to get consistent + // return values. + sort.Slice(s.Resources, func(i int, j int) bool { + return s.Resources[i].Resource != nil + }) for _, r := range s.Resources { if r.Resource != nil { return true, nil } else if r.Error != nil && !apierrors.IsNotFound(r.Error) { + // We know the error is not a "resource not found" error at this point. + // It still may be the equivalent for a CR, "no kind match", if its + // corresponding CRD has been deleted already. We want to make sure + // we're only allowing "no kind match" errors to be skipped for CR's + // since we do not know if a kind is a CR kind, hence checking + // crdKindSet for existence of a resource's kind. nkmerr := &meta.NoKindMatchError{} if !errors.As(r.Error, &nkmerr) || !crdKindSet.Has(r.GVK.Kind) { return false, r.Error From dd6459dec409a5652954730b6c3e0c5b7bd2dd93 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 4 Dec 2019 01:08:49 -0800 Subject: [PATCH 8/8] use unstructured converter in getCRDKindSet --- internal/olm/client/status.go | 53 +++++++++-------------------------- 1 file changed, 13 insertions(+), 40 deletions(-) diff --git a/internal/olm/client/status.go b/internal/olm/client/status.go index 080a11c1dc..d07ca9b2b4 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -27,25 +27,15 @@ import ( log "github.com/sirupsen/logrus" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/client-go/kubernetes/scheme" ) -var sch = scheme.Scheme - -func init() { - install.Install(sch) -} - type Status struct { Resources []ResourceStatus } @@ -125,50 +115,33 @@ func (s Status) HasInstalledResources() (bool, error) { // getCRDKindSet returns the set of all kinds specified by all CRDs in s. func (s Status) getCRDKindSet() (sets.String, error) { crdKindSet := sets.NewString() - dec := serializer.NewCodecFactory(sch).UniversalDeserializer() for _, r := range s.Resources { if r.GVK.Kind == "CustomResourceDefinition" { + u := &unstructured.Unstructured{} switch v := r.requestObject.(type) { case *unstructured.Unstructured: - vb, err := v.MarshalJSON() - if err != nil { - return nil, err - } - // Use unversioned CustomResourceDefinition to avoid implementing cast - // for all versions. - obj, _, err := dec.Decode(vb, nil, nil) - if err != nil { - return nil, err - } - kind, err := getVersionedCRDKind(obj) - if err != nil { - return nil, err - } - crdKindSet.Insert(kind) + u = v default: - kind, err := getVersionedCRDKind(v) + uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&v) if err != nil { return nil, err } - crdKindSet.Insert(kind) + // Other fields are not important, just the CRD kind. + u.Object = uObj } + // Use unversioned CustomResourceDefinition to avoid implementing cast + // for all versions. + crd := &apiextensions.CustomResourceDefinition{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &crd) + if err != nil { + return nil, err + } + crdKindSet.Insert(crd.Spec.Names.Kind) } } return crdKindSet, nil } -// getVersionedCRDKind returns the kind of a CRD if its version is known, -// otherwise an error. -func getVersionedCRDKind(obj runtime.Object) (string, error) { - switch crd := obj.(type) { - case *apiextensions.CustomResourceDefinition: - return crd.Spec.Names.Kind, nil - case *v1beta1.CustomResourceDefinition: - return crd.Spec.Names.Kind, nil - } - return "", fmt.Errorf("error getting CRD kind: gvk %q unknown", obj.GetObjectKind().GroupVersionKind()) -} - func (s Status) String() string { out := &bytes.Buffer{} tw := tabwriter.NewWriter(out, 8, 4, 4, ' ', 0)