:controller: Resurface bundledeployment errors

This PR enables the operator object to resurface the errors
in Rukpak by setting its conditions as "BundleDeploymentFailed"
if any of the BD errors.

Author: varshaprasad96

controllers/operator_controller.go

@@ -257,10 +257,16 @@ func (r *OperatorReconciler) ensureBundleDeployment(ctx context.Context, desired
 	}
 
 	// If the existing BD already has everything that the desired BD has, no need to contact the API server.
+	// Make sure the status of the existingBD from the server is as expected.
 	if equality.Semantic.DeepDerivative(desiredBundleDeployment, existingBundleDeployment) {
-		return nil
+		return verifySuccessfulBDStatus(existingBundleDeployment)
 	}
-	return r.Client.Patch(ctx, desiredBundleDeployment, client.Apply, client.ForceOwnership, client.FieldOwner("operator-controller"))
+
+	if err = r.Client.Patch(ctx, desiredBundleDeployment, client.Apply, client.ForceOwnership, client.FieldOwner("operator-controller")); err != nil {
+		return err
+	}
+
+	return verifySuccessfulBDStatus(desiredBundleDeployment)
 }
 
 func (r *OperatorReconciler) existingBundleDeploymentUnstructured(ctx context.Context, name string) (*unstructured.Unstructured, error) {
@@ -277,3 +283,30 @@ func (r *OperatorReconciler) existingBundleDeploymentUnstructured(ctx context.Co
 	}
 	return &unstructured.Unstructured{Object: unstrExistingBundleDeploymentObj}, nil
 }
+
+func verifySuccessfulBDStatus(dep *unstructured.Unstructured) error {
+	// convert the unstructured object from patch call to typed bundledeployment to make checking of Status easier.
+	existingTypedBundleDeployment := &rukpakv1alpha1.BundleDeployment{}
+
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(dep.UnstructuredContent(), existingTypedBundleDeployment); err != nil {
+		return err
+	}
+
+	ers := []error{}
+	conditions := existingTypedBundleDeployment.Status.Conditions
+
+	// Do we error here?
+	if conditions == nil || len(conditions) == 0 {
+		return nil
+	}
+
+	for _, condition := range conditions {
+		// Currently we are checking for only these two types, if any other condition is added to Rukpak, validate accordingly.
+		if condition.Type == rukpakv1alpha1.TypeHasValidBundle || condition.Type == rukpakv1alpha1.TypeInstalled {
+			if condition.Status == metav1.ConditionFalse {
+				ers = append(ers, fmt.Errorf("error observed by Rukpak: %v", condition.Message))
+			}
+		}
+	}
+	return utilerrors.NewAggregate(ers)
+}

controllers/operator_controller_test.go

@@ -293,10 +293,80 @@ var _ = Describe("Reconcile Test", func() {
 				Expect(cond.Message).To(Equal(`duplicate identifier "required package prometheus" in input`))
 			})
 		})
+		When("the existing operator status is based on bundleDeployment", func() {
+			const pkgName = "prometheus"
+			var (
+				bd rukpakv1alpha1.BundleDeployment
+			)
+			BeforeEach(func() {
+				By("creating the expected BundleDeployment")
+				bd := &rukpakv1alpha1.BundleDeployment{
+					ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
+					Spec: rukpakv1alpha1.BundleDeploymentSpec{
+						ProvisionerClassName: "core-rukpak-io-plain",
+						Template: &rukpakv1alpha1.BundleTemplate{
+							Spec: rukpakv1alpha1.BundleSpec{
+								ProvisionerClassName: "core-rukpak-io-registry",
+								Source: rukpakv1alpha1.BundleSource{
+									Type: rukpakv1alpha1.SourceTypeImage,
+									Image: &rukpakv1alpha1.ImageSource{
+										Ref: "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed",
+									},
+								},
+							},
+						},
+					},
+				}
+				err := cl.Create(ctx, bd)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("creating the operator object")
+				operator = &operatorsv1alpha1.Operator{
+					ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
+					Spec: operatorsv1alpha1.OperatorSpec{
+						PackageName: pkgName,
+					},
+				}
+				err = cl.Create(ctx, operator)
+				Expect(err).NotTo(HaveOccurred())
+
+			})
+			It("update the Status of bundleDeployment", func() {
+				err := cl.Get(ctx, opKey, &bd)
+				Expect(err).NotTo(HaveOccurred())
+				apimeta.SetStatusCondition(&bd.Status.Conditions, metav1.Condition{
+					Type:               rukpakv1alpha1.TypeInstalled,
+					LastTransitionTime: metav1.Now(),
+					Status:             metav1.ConditionFalse,
+					Message:            "fail to unpack",
+					Reason:             rukpakv1alpha1.ReasonInstallFailed,
+				})
+				err = cl.Status().Update(ctx, &bd)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("running reconcile")
+				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
+				Expect(res).To(Equal(ctrl.Result{}))
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(ContainSubstring(`error observed by Rukpak`))
+
+				By("fetching the updated operator after reconcile")
+				op := &operatorsv1alpha1.Operator{}
+				err = cl.Get(ctx, opKey, op)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("checking the expected conditions")
+				cond := apimeta.FindStatusCondition(op.Status.Conditions, operatorsv1alpha1.TypeReady)
+				Expect(cond).NotTo(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
+				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
+				Expect(cond.Message).To(ContainSubstring(`error observed by Rukpak`))
+			})
+		})
 		AfterEach(func() {
 			verifyInvariants(ctx, operator)
 
-			err := cl.Delete(ctx, operator)
+			err := cl.DeleteAllOf(ctx, operator)
 			Expect(err).To(Not(HaveOccurred()))
 		})
 	})

internal/resolution/bundle_cache.go

@@ -48,4 +48,10 @@ var HardcodedEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Ent
 		"olm.gvk":         "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"AlertmanagerConfig\",\"version\":\"v1alpha1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"AlertmanagerConfig\",\"version\":\"v1alpha1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"PodMonitor\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"PodMonitor\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"Probe\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"Probe\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"PrometheusRule\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"PrometheusRule\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"ServiceMonitor\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"ServiceMonitor\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"ThanosRuler\",\"version\":\"v1\"},{\"group\":\"monitoring.coreos.com\",\"kind\":\"ThanosRuler\",\"version\":\"v1\"}]",
 		"olm.package":     "{\"packageName\":\"prometheus\",\"version\":\"0.47.0\"}",
 	}),
+	"operatorhub/lightbend-console-operator/0.0.1": *input.NewEntity("operatorhub/lightbend-console-operator/0.0.1", map[string]string{
+		"olm.bundle.path": `"quay.io/operatorhubio/lightbend-console-operator@sha256:2cf5f1abf71be29b7d2667ae9ca4102198c93cdef450d09faf1b26900443e285"`,
+		"olm.channel":     "{\"channelName\":\"alpha\",\"priority\":0}",
+		"olm.gvk":         "[{\"group\":\"app.lightbend.com\",\"kind\":\"Console\",\"version\":\"v1alpha1\"}]",
+		"olm.package":     "{\"packageName\":\"lightbend-console-operator\",\"version\":\"0.0.1\"}",
+	}),
 })

test/e2e/install_test.go

@@ -21,21 +21,25 @@ const (
 )
 
 var _ = Describe("Operator Install", func() {
+	var (
+		ctx          context.Context
+		pkgName      string
+		operatorName string
+		operator     *operatorv1alpha1.Operator
+	)
 	It("resolves the specified package with correct bundle path", func() {
-		var (
-			ctx          context.Context            = context.Background()
-			pkgName      string                     = "prometheus"
-			operatorName string                     = fmt.Sprintf("operator-%s", rand.String(8))
-			operator     *operatorv1alpha1.Operator = &operatorv1alpha1.Operator{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: operatorName,
-				},
-				Spec: operatorv1alpha1.OperatorSpec{
-					PackageName: pkgName,
-				},
-			}
-		)
 		ctx = context.Background()
+		pkgName = "prometheus"
+		operatorName = fmt.Sprintf("operator-%s", rand.String(8))
+		operator = &operatorv1alpha1.Operator{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: operatorName,
+			},
+			Spec: operatorv1alpha1.OperatorSpec{
+				PackageName: pkgName,
+			},
+		}
+
 		By("creating the Operator resource")
 		err := c.Create(ctx, operator)
 		Expect(err).ToNot(HaveOccurred())