diff --git a/internal/olm/client.go b/internal/olm/client.go index ff966f9b04..72ceab7da1 100644 --- a/internal/olm/client.go +++ b/internal/olm/client.go @@ -72,8 +72,11 @@ func (c Client) InstallVersion(ctx context.Context, version string) (*olmresourc objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if status.HasExistingResources() { + installed, err := status.HasInstalledResources() + if installed { 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, see resource statuses for more details") } log.Print("Creating CRDs and resources") @@ -129,7 +132,8 @@ func (c Client) UninstallVersion(ctx context.Context, version string) error { objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if !status.HasExistingResources() { + installed, err := status.HasInstalledResources() + if !installed && err == nil { return olmresourceclient.ErrOLMNotInstalled } @@ -148,7 +152,8 @@ func (c Client) GetStatus(ctx context.Context, version string) (*olmresourceclie objs := toObjects(resources...) status := c.GetObjectsStatus(ctx, objs...) - if !status.HasExistingResources() { + 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 e617cda3fb..c22b3a2b4e 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())) } } @@ -121,7 +119,8 @@ func (c Client) DoDelete(ctx context.Context, objs ...runtime.Object) error { return wait.PollImmediateUntil(time.Second, func() (bool, error) { s := c.GetObjectsStatus(ctx, objs...) - return !s.HasExistingResources(), nil + installed, err := s.HasInstalledResources() + return !installed, err }, ctx.Done()) } @@ -151,27 +150,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 } @@ -191,7 +190,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 } @@ -200,7 +199,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..d07ca9b2b4 100644 --- a/internal/olm/client/status.go +++ b/internal/olm/client/status.go @@ -20,15 +20,20 @@ package olm import ( "bytes" "context" + "errors" "fmt" + "sort" "text/tabwriter" log "github.com/sirupsen/logrus" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + 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/types" + "k8s.io/apimachinery/pkg/util/sets" ) type Status struct { @@ -40,29 +45,31 @@ 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 { 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, + requestObject: obj, } - 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) @@ -71,13 +78,68 @@ func (c Client) GetObjectsStatus(ctx context.Context, objs ...runtime.Object) St return Status{Resources: rss} } -func (s Status) HasExistingResources() bool { +// 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) + } + // 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 + 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 + } } } - return false + 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() + for _, r := range s.Resources { + if r.GVK.Kind == "CustomResourceDefinition" { + u := &unstructured.Unstructured{} + switch v := r.requestObject.(type) { + case *unstructured.Unstructured: + u = v + default: + uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&v) + if err != nil { + return nil, err + } + // 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 } func (s Status) String() string { @@ -88,10 +150,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" }