timflannagan / rukpak

Rukpak runs in a Kubernetes cluster and defines an API for installing cloud native bundle content
Apache License 2.0
0 stars 0 forks source link

handle this case better as an IsAlreadyExists error could be due to a slow cache... #45

Open github-actions[bot] opened 2 years ago

github-actions[bot] commented 2 years ago

https://github.com/timflannagan/rukpak/blob/78956e26d6757f6b8308172a7eecfaa47c29e8b3/internal/provisioner/plain/controllers/bundleinstance_controller.go#L347


        Status: metav1.ConditionTrue,
        Reason: rukpakv1alpha1.ReasonInstallationSucceeded,
    })
    bi.Status.InstalledBundleRef = bundleToObjectReference(bundle)

    if err := r.cleanupBundleInstance(ctx, oldBundles, bundle); err != nil {
        // TODO: update BundleInstance's status
        l.Error(err, "failed to cleanup old bundle resources")
        return ctrl.Result{}, err
    }

    return ctrl.Result{}, nil
}

func (r *BundleInstanceReconciler) cleanupBundleInstance(ctx context.Context, oldBundles *rukpakv1alpha1.BundleList, currBundle *rukpakv1alpha1.Bundle) error {
    l := log.FromContext(ctx)

    var (
        errors []error
    )
    for _, b := range oldBundles.Items {
        if b.GetName() == currBundle.GetName() {
            continue
        }
        // TODO: cleanup or remove this log message
        l.Info("attempting to delete old bundle", "name", b.GetName())
        if err := r.Delete(ctx, &b); err != nil {
            errors = append(errors, err)
        }
    }
    return utilerrors.NewAggregate(errors)
}

func (r *BundleInstanceReconciler) getBundlesForBundleInstance(ctx context.Context, bi *rukpakv1alpha1.BundleInstance) (*rukpakv1alpha1.BundleList, error) {
    bundleSelector, err := metav1.LabelSelectorAsSelector(bi.Spec.Selector)
    if err != nil {
        return nil, err
    }
    bundleList := &rukpakv1alpha1.BundleList{}
    if err := r.List(ctx, bundleList, &client.ListOptions{
        LabelSelector: bundleSelector,
    }); err != nil {
        return nil, err
    }

    return bundleList, nil
}

func (r *BundleInstanceReconciler) ensureDesiredBundle(ctx context.Context, bi *rukpakv1alpha1.BundleInstance) (*rukpakv1alpha1.Bundle, *rukpakv1alpha1.BundleList, error) {
    l := log.FromContext(ctx)

    existingBundles, err := r.getBundlesForBundleInstance(ctx, bi)
    if err != nil {
        return nil, nil, err
    }

    // check whether any of the existing Bundles match the desired BundleSpec
    // TODO: this is potentially non-deterministic. do we need to sort by
    // metadata.CreationTimestamp like the Deployment controller does?
    // TODO: need to put a limit on how many bundles are generated to avoid
    // hotlooping scenarios. OR: support spec.Paused for the BI resource to skip
    // reconciliation loops for that resource.
    var b *rukpakv1alpha1.Bundle
    for _, bundle := range existingBundles.Items {
        if reflect.DeepEqual(bundle.Spec, bi.Spec.Template.Spec) {
            // TODO: cleanup or remove this log message
            l.Info("found existing bundle that matches the desired BundleSpec template", "name", b.GetName())
            b = &bundle
            break
        }
    }
    // check whether we need to generate a new Bundle resource
    if len(existingBundles.Items) == 0 || b == nil {
        // TODO: cleanup or remove this log message
        l.Info("failed to find any bundles that match the desired spec -- checking whether a new Bundle needs to be recreated")
        controllerRef := metav1.NewControllerRef(bi, bi.GroupVersionKind())
        b = &rukpakv1alpha1.Bundle{
            ObjectMeta: metav1.ObjectMeta{
                Name:            bi.GetName() + "-" + rand.String(5),
                OwnerReferences: []metav1.OwnerReference{*controllerRef},
                Labels:          bi.Spec.Template.Labels,
            },
            Spec: bi.Spec.Template.Spec,
        }
        if err := r.Create(ctx, b); err != nil {
            // TODO: handle this case better as an IsAlreadyExists error could be due to a slow cache?
            if apierrors.IsAlreadyExists(err) {
                return nil, nil, err
            }
            return nil, nil, err
        }
    }
    return b, existingBundles, err
}

func bundleToObjectReference(bundle *rukpakv1alpha1.Bundle) *corev1.ObjectReference {
    return &corev1.ObjectReference{
        APIVersion: bundle.APIVersion,
        Kind:       bundle.Kind,
        Name:       bundle.GetName(),
    }
}

type releaseState string

const (
github-actions[bot] commented 2 years ago

This issue has become stale because it has been open 60 days with no activity. The maintainers of this repo will remove this label during issue triage or it will be removed automatically after an update. Adding the lifecycle/frozen label will cause this issue to ignore lifecycle events.