Skip to content

Commit

Permalink
Extend Step to surface whether a manifest is optional
Browse files Browse the repository at this point in the history 
Signed-off-by: Frederic Giloux <fgiloux@redhat.com>
  • Loading branch information
@fgiloux
fgiloux committed Mar 22, 2022
1 parent 14c0b96 commit b444a91
Show file tree
Hide file tree
Showing 16 changed files with 1,324 additions and 29 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/onsi/gomega v1.17.0
github.com/openshift/api v0.0.0-20200331152225-585af27e34fd
github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0
github.com/operator-framework/api v0.11.2-0.20220128160316-8e593f1c42b9
github.com/operator-framework/api v0.14.0
github.com/operator-framework/operator-registry v1.17.5
github.com/otiai10/copy v1.2.0
github.com/pkg/errors v0.9.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -934,8 +934,8 @@ github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJ
github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
github.com/operator-framework/api v0.7.1/go.mod h1:L7IvLd/ckxJEJg/t4oTTlnHKAJIP/p51AvEslW3wYdY=
github.com/operator-framework/api v0.11.2-0.20220128160316-8e593f1c42b9 h1:aOF1+e6amntViIbc9BrpD07cwmgcVFG4wxPbj/VGCjA=
github.com/operator-framework/api v0.11.2-0.20220128160316-8e593f1c42b9/go.mod h1:r/erkmp9Kc1Al4dnxmRkJYc0uCtD5FohN9VuJ5nTxz0=
github.com/operator-framework/api v0.14.0 h1:5nk8fQL8l+dDxi11hZi0T7nqhhoIQLn+qL2DhMEGnoE=
github.com/operator-framework/api v0.14.0/go.mod h1:r/erkmp9Kc1Al4dnxmRkJYc0uCtD5FohN9VuJ5nTxz0=
github.com/operator-framework/operator-registry v1.17.5 h1:LR8m1rFz5Gcyje8WK6iYt+gIhtzqo52zMRALdmTYHT0=
github.com/operator-framework/operator-registry v1.17.5/go.mod h1:sRQIgDMZZdUcmHltzyCnM6RUoDF+WS8Arj1BQIARDS8=
github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k=
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/operators/catalog/manifests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *Unpack
bundle.Properties = props
}

steps, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
steps, _, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
if err != nil {
return nil, errorwrap.Wrapf(err, "error calculating steps for ref %v", *ref)
}
Expand Down
40 changes: 33 additions & 7 deletions pkg/controller/operators/catalog/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1392,11 +1392,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(), 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
Expand Down Expand Up @@ -1590,7 +1591,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 {
Expand Down Expand Up @@ -1631,7 +1631,6 @@ func (o *Operator) syncInstallPlans(obj interface{}) (syncError error) {
return
}
}

outInstallPlan, syncError := transitionInstallPlanState(logger.Logger, o, *plan, o.now(), o.installPlanTimeout)

if syncError != nil {
Expand Down Expand Up @@ -1983,7 +1982,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)
Expand Down Expand Up @@ -2252,6 +2251,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
}

Expand All @@ -2266,7 +2266,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
Expand Down Expand Up @@ -2312,7 +2319,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
// no error
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
Expand All @@ -2337,7 +2363,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
}
Expand Down
112 changes: 112 additions & 0 deletions pkg/controller/operators/catalog/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: gvkNotFoundError{gvk: schema.GroupVersionKind{Group: "monitoring.coreos.com", Version: "v1", Kind: "PrometheusRule"}, name: "rule"},
},
{
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 {
Expand Down
Loading

0 comments on commit b444a91

Please sign in to comment.