update conditions

porch/controllers/packagevariantsets/pkg/controllers/packagevariantset/packagevariantset_controller.go

@@ -30,7 +30,6 @@ import (
 	api "github.com/GoogleContainerTools/kpt/porch/controllers/packagevariantsets/api/v1alpha1"
 
 	"github.com/GoogleContainerTools/kpt/internal/fnruntime"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -83,28 +82,26 @@ func (r *PackageVariantSetReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{}, nil
 	}
 
-	existingConditions := pvs.Status.Conditions
+	defer func() {
+		r.Client.Status().Update(ctx, pvs)
+	}()
 
 	if errs := r.validatePackageVariantSet(ctx, pvs); len(errs) > 0 {
 		validationErr := combineErrors(errs)
-		meta.SetStatusCondition(&existingConditions, metav1.Condition{
+		meta.SetStatusCondition(&pvs.Status.Conditions, metav1.Condition{
 			Type:    "Valid",
 			Status:  "False",
-			Reason:  "ValidationFailed",
+			Reason:  "Invalid",
 			Message: validationErr,
 		})
-		statusUpdateErr := r.Client.Status().Update(ctx, pvs)
-		return ctrl.Result{}, statusUpdateErr
+		return ctrl.Result{}, fmt.Errorf(validationErr)
 	} else {
-		meta.SetStatusCondition(&existingConditions, metav1.Condition{
+		meta.SetStatusCondition(&pvs.Status.Conditions, metav1.Condition{
 			Type:    "Valid",
 			Status:  "True",
-			Reason:  "ValidationFailed",
-			Message: "All validation checks passed",
+			Reason:  "Valid",
+			Message: "all validation checks passed",
 		})
-		if statusUpdateErr := r.Client.Status().Update(ctx, pvs); statusUpdateErr != nil {
-			return ctrl.Result{}, statusUpdateErr
-		}
 	}
 
 	upstream, err := r.getUpstreamPR(pvs.Spec.Upstream)
@@ -116,26 +113,19 @@ func (r *PackageVariantSetReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	}
 
 	downstreams, err := r.unrollDownstreamTargets(ctx, pvs, upstream.Package)
-
 	if err != nil {
-		if apierrors.IsNotFound(err) {
-			meta.SetStatusCondition(&existingConditions, metav1.Condition{
+		if meta.IsNoMatchError(err) {
+			meta.SetStatusCondition(&pvs.Status.Conditions, metav1.Condition{
 				Type:    "Valid",
 				Status:  "False",
-				Reason:  "ValidationFailed",
+				Reason:  "Invalid",
 				Message: err.Error(),
 			})
-			statusUpdateErr := r.Client.Status().Update(ctx, pvs)
-			return ctrl.Result{}, statusUpdateErr
 		}
 		return ctrl.Result{}, err
 	}
 
-	if err := r.ensurePackageVariants(ctx, upstream, downstreams, pvs); err != nil {
-		return ctrl.Result{}, err
-	}
-
-	return ctrl.Result{}, nil
+	return ctrl.Result{}, r.ensurePackageVariants(ctx, upstream, downstreams, pvs)
 }
 
 func (r *PackageVariantSetReconciler) init(ctx context.Context, req ctrl.Request) (*api.PackageVariantSet, error) {
@@ -149,16 +139,16 @@ func (r *PackageVariantSetReconciler) init(ctx context.Context, req ctrl.Request
 func (r *PackageVariantSetReconciler) validatePackageVariantSet(ctx context.Context, pvs *api.PackageVariantSet) []error {
 	var allErrs []error
 	if pvs.Spec.Upstream == nil {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "upstream"), "{}", "missing required field"))
+		allErrs = append(allErrs, fmt.Errorf("spec.upstream is a required field"))
 	} else {
 		if pvs.Spec.Upstream.Package == nil {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "upstream", "package"), "{}", "missing required field"))
+			allErrs = append(allErrs, fmt.Errorf("spec.upstream.package is a required field"))
 		} else {
 			if pvs.Spec.Upstream.Package.Name == "" {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "upstream", "package", "name"), "", "missing required field"))
+				allErrs = append(allErrs, fmt.Errorf("spec.upstream.package.name is a required field"))
 			}
 			if pvs.Spec.Upstream.Package.Repo == "" {
-				allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "upstream", "package", "repo"), "", "missing required field"))
+				allErrs = append(allErrs, fmt.Errorf("spec.upstream.package.repo is a required field"))
 			}
 		}
 		if (pvs.Spec.Upstream.Tag == "" && pvs.Spec.Upstream.Revision == "") ||
@@ -234,7 +224,7 @@ func combineErrors(errs []error) string {
 			errMsgs = append(errMsgs, e.Error())
 		}
 	}
-	return strings.Join(errMsgs, ";")
+	return strings.Join(errMsgs, "; ")
 }
 
 func (r *PackageVariantSetReconciler) getUpstreamPR(
@@ -394,7 +384,7 @@ func (r *PackageVariantSetReconciler) getSelectedObjects(ctx context.Context, se
 			opts = append(opts, client.MatchingLabelsSelector{Selector: labelSelector})
 		}
 		if err := r.Client.List(ctx, uList, opts...); err != nil {
-			return nil, fmt.Errorf("unable to list specified objects in cluster: %v", err)
+			return nil, err
 		}
 
 		for _, u := range uList.Items {