Extend Step to surface whether a manifest is optional

Signed-off-by: Frederic Giloux <fgiloux@redhat.com>

deploy/chart/crds/0000_50_olm_00-installplans.crd.yaml

@@ -222,6 +222,8 @@ spec:
                       - resource
                       - status
                     properties:
+                      optional:
+                        type: boolean
                       resolving:
                         type: string
                       resource:

go.mod

@@ -67,3 +67,4 @@ replace (
 	// informers until it can be resolved upstream:
 	sigs.k8s.io/controller-runtime v0.10.0 => github.com/timflannagan/controller-runtime v0.10.1-0.20211210161403-6756a4203e70
 )
+

pkg/controller/bundle/bundle_unpacker.go

@@ -47,8 +47,9 @@ const (
 type BundleUnpackResult struct {
 	*operatorsv1alpha1.BundleLookup
 
-	bundle *api.Bundle
-	name   string
+	bundle    *api.Bundle
+	filenames []string
+	name      string
 }
 
 func (b *BundleUnpackResult) Bundle() *api.Bundle {
@@ -59,6 +60,10 @@ func (b *BundleUnpackResult) Name() string {
 	return b.name
 }
 
+func (b *BundleUnpackResult) Filenames() []string {
+	return b.filenames
+}
+
 // SetCondition replaces the existing BundleLookupCondition of the same type, or adds it if it was not found.
 func (b *BundleUnpackResult) SetCondition(cond operatorsv1alpha1.BundleLookupCondition) operatorsv1alpha1.BundleLookupCondition {
 	for i, existing := range b.Conditions {
@@ -493,7 +498,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup,
 		return
 	}
 
-	result.bundle, err = c.loader.Load(cm)
+	result.bundle, result.filenames, err = c.loader.Load(cm)
 	if err != nil {
 		return
 	}

pkg/controller/operators/catalog/manifests.go

@@ -82,7 +82,7 @@ func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *Unpack
 		return nil, errorwrap.Wrapf(err, "error finding unpacked bundle configmap for ref %v", *ref)
 	}
 	loader := configmap.NewBundleLoader()
-	bundle, err := loader.Load(cm)
+	bundle, _, err := loader.Load(cm)
 	if err != nil {
 		return nil, errorwrap.Wrapf(err, "error loading unpacked bundle configmap for ref %v", *ref)
 	}

pkg/controller/operators/catalog/operator.go

@@ -1364,11 +1364,12 @@ func (o *Operator) unpackBundles(plan *v1alpha1.InstallPlan) (bool, *v1alpha1.In
 
 		// if packed condition is missing, bundle has already been unpacked into steps, continue
 		if res.GetCondition(resolver.BundleLookupConditionPacked).Status == corev1.ConditionUnknown {
+			o.logger.Debug("Bundle already unpacked")
 			continue
 		}
 
 		// Ensure that bundle can be applied by the current version of OLM by converting to steps
-		steps, err := resolver.NewStepsFromBundle(res.Bundle(), out.GetNamespace(), res.Replaces, res.CatalogSourceRef.Name, res.CatalogSourceRef.Namespace)
+		steps, err := resolver.NewQualifiedStepsFromBundle(res.Bundle(), res.Filenames(), out.GetNamespace(), res.Replaces, res.CatalogSourceRef.Name, res.CatalogSourceRef.Namespace, o.logger)
 		if err != nil {
 			errs = append(errs, fmt.Errorf("failed to turn bundle into steps: %v", err))
 			unpacked = false
@@ -1562,7 +1563,6 @@ func (o *Operator) syncInstallPlans(obj interface{}) (syncError error) {
 			syncError = fmt.Errorf("bundle unpacking failed: %v", err)
 			return
 		}
-
 		if !reflect.DeepEqual(plan.Status, out.Status) {
 			logger.Warnf("status not equal, updating...")
 			if _, err := o.client.OperatorsV1alpha1().InstallPlans(out.GetNamespace()).UpdateStatus(context.TODO(), out, metav1.UpdateOptions{}); err != nil {
@@ -1603,7 +1603,6 @@ func (o *Operator) syncInstallPlans(obj interface{}) (syncError error) {
 			return
 		}
 	}
-
 	outInstallPlan, syncError := transitionInstallPlanState(logger.Logger, o, *plan, o.now(), o.installPlanTimeout)
 
 	if syncError != nil {
@@ -1955,7 +1954,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			}
 
 			switch step.Status {
-			case v1alpha1.StepStatusPresent, v1alpha1.StepStatusCreated, v1alpha1.StepStatusWaitingForAPI:
+			case v1alpha1.StepStatusPresent, v1alpha1.StepStatusCreated, v1alpha1.StepStatusWaitingForAPI, v1alpha1.StepStatusNotCreated:
 				return nil
 			case v1alpha1.StepStatusUnknown, v1alpha1.StepStatusNotPresent:
 				manifest, err := r.ManifestForStep(step)
@@ -2224,6 +2223,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 					if !isSupported(step.Resource.Kind) {
 						// Not a supported resource
 						plan.Status.Plan[i].Status = v1alpha1.StepStatusUnsupportedResource
+						o.logger.WithError(err).Debug("resource kind not supported")
 						return v1alpha1.ErrInvalidInstallPlan
 					}
 
@@ -2238,7 +2238,14 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 					gvk := unstructuredObject.GroupVersionKind()
 					r, err := o.apiresourceFromGVK(gvk)
 					if err != nil {
-						return err
+						if step.Optional {
+							// Special handling of the unavailability of the API for optional resources: no error
+							o.logger.WithFields(logrus.Fields{"InstallPlan": plan.Name, "kind": step.Resource.Kind, "name": step.Resource.Name}).Warnf("Optional manifest could not be created. Optional capabilities of the operator may not be available.")
+							plan.Status.Plan[i].Status = v1alpha1.StepStatusNotCreated
+							return nil
+						} else {
+							return err
+						}
 					}
 
 					// Create the GVR
@@ -2284,7 +2291,26 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 					// Ensure Unstructured Object
 					status, err := ensurer.EnsureUnstructuredObject(resourceInterface, unstructuredObject)
 					if err != nil {
-						return err
+						if step.Optional {
+							switch k8serrors.ReasonForError(err) {
+							case
+								metav1.StatusReasonUnauthorized,
+								metav1.StatusReasonForbidden,
+								metav1.StatusReasonNotFound,
+								metav1.StatusReasonInvalid,
+								metav1.StatusReasonNotAcceptable,
+								metav1.StatusReasonUnsupportedMediaType,
+								metav1.StatusReasonConflict:
+								o.logger.WithFields(logrus.Fields{"kind": step.Resource.Kind, "name": step.Resource.Name}).WithError(err).Warnf("Optional manifest could not be created. Optional capabilities of the operator may not be available.")
+								status = v1alpha1.StepStatusNotCreated
+								err = nil
+							default:
+								o.logger.WithFields(logrus.Fields{"kind": step.Resource.Kind, "name": step.Resource.Name}).WithError(err).Warnf("Optional manifest could not be created, possibly due to a recoverable error, retrying")
+								return err
+							}
+						} else {
+							return err
+						}
 					}
 
 					plan.Status.Plan[i].Status = status
@@ -2309,7 +2335,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 	// Loop over one final time to check and see if everything is good.
 	for _, step := range plan.Status.Plan {
 		switch step.Status {
-		case v1alpha1.StepStatusCreated, v1alpha1.StepStatusPresent:
+		case v1alpha1.StepStatusCreated, v1alpha1.StepStatusPresent, v1alpha1.StepStatusNotCreated:
 		default:
 			return nil
 		}

pkg/controller/operators/catalog/operator_test.go

@@ -604,6 +604,118 @@ func TestExecutePlan(t *testing.T) {
 				},
 			},
 		},
+		{
+			testName: "OptionalManifestWithCRD",
+			in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"),
+				[]*v1alpha1.Step{
+					{
+						Resolving: "csv",
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "catalog",
+							CatalogSourceNamespace: namespace,
+							Group:                  "operators.coreos.com",
+							Version:                "v1alpha1",
+							Kind:                   "ClusterServiceVersion",
+							Name:                   "csv",
+							Manifest:               toManifest(t, csv("csv", namespace, nil, nil)),
+						},
+						Status: v1alpha1.StepStatusUnknown,
+					},
+					{
+						Resolving: "csv",
+						Optional:  true,
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "catalog",
+							CatalogSourceNamespace: namespace,
+							Group:                  "monitoring.coreos.com",
+							Version:                "v1",
+							Kind:                   "PrometheusRule",
+							Name:                   "rule",
+							Manifest:               toManifest(t, decodeFile(t, "./testdata/prometheusrule.cr.yaml", &unstructured.Unstructured{})),
+						},
+						Status: v1alpha1.StepStatusUnknown,
+					},
+				},
+			),
+			extObjs: []runtime.Object{decodeFile(t, "./testdata/prometheusrule.crd.yaml", &apiextensionsv1beta1.CustomResourceDefinition{})},
+			want: []runtime.Object{
+				csv("csv", namespace, nil, nil),
+				modify(t, decodeFile(t, "./testdata/prometheusrule.cr.yaml", &unstructured.Unstructured{}),
+					withNamespace(namespace),
+					withOwner(csv("csv", namespace, nil, nil)),
+				),
+			},
+			err: nil,
+		},
+		{
+			testName: "NotOptionalManifestWithoutCRD",
+			in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"),
+				[]*v1alpha1.Step{
+					{
+						Resolving: "csv",
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "catalog",
+							CatalogSourceNamespace: namespace,
+							Group:                  "operators.coreos.com",
+							Version:                "v1alpha1",
+							Kind:                   "ClusterServiceVersion",
+							Name:                   "csv",
+							Manifest:               toManifest(t, csv("csv", namespace, nil, nil)),
+						},
+						Status: v1alpha1.StepStatusUnknown,
+					},
+					{
+						Resolving: "csv",
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "catalog",
+							CatalogSourceNamespace: namespace,
+							Group:                  "monitoring.coreos.com",
+							Version:                "v1",
+							Kind:                   "PrometheusRule",
+							Name:                   "rule",
+							Manifest:               toManifest(t, decodeFile(t, "./testdata/prometheusrule.cr.yaml", &unstructured.Unstructured{})),
+						},
+						Status: v1alpha1.StepStatusUnknown,
+					},
+				},
+			),
+			err: fmt.Errorf("GroupVersion \"monitoring.coreos.com/v1\" not found"),
+		},
+		{
+			testName: "OptionalManifestWithoutCRD",
+			in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"),
+				[]*v1alpha1.Step{
+					{
+						Resolving: "csv",
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "catalog",
+							CatalogSourceNamespace: namespace,
+							Group:                  "operators.coreos.com",
+							Version:                "v1alpha1",
+							Kind:                   "ClusterServiceVersion",
+							Name:                   "csv",
+							Manifest:               toManifest(t, csv("csv", namespace, nil, nil)),
+						},
+						Status: v1alpha1.StepStatusUnknown,
+					},
+					{
+						Resolving: "csv",
+						Optional:  true,
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "catalog",
+							CatalogSourceNamespace: namespace,
+							Group:                  "monitoring.coreos.com",
+							Version:                "v1",
+							Kind:                   "PrometheusRule",
+							Name:                   "rule",
+							Manifest:               toManifest(t, decodeFile(t, "./testdata/prometheusrule.cr.yaml", &unstructured.Unstructured{})),
+						},
+						Status: v1alpha1.StepStatusUnknown,
+					},
+				},
+			),
+			err: nil,
+		},
 	}
 
 	for _, tt := range tests {