Skip to content

Reduce reconcile complexity #118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Reduce reconcile complexity #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions api/v1alpha1/operator_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ limitations under the License.
package v1alpha1

import (
operatorutil "github.com/operator-framework/operator-controller/internal/util"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

operatorutil "github.com/operator-framework/operator-controller/internal/util"
)

// OperatorSpec defines the desired state of Operator
Expand All @@ -32,9 +33,10 @@ const (
// TODO(user): add more Types, here and into init()
TypeReady = "Ready"

ReasonNotImplemented = "NotImplemented"
ReasonResolutionFailed = "ResolutionFailed"
ReasonResolutionSucceeded = "ResolutionSucceeded"
ReasonResolutionSucceeded = "ResolutionSucceeded"
ReasonResolutionFailed = "ResolutionFailed"
ReasonBundleLookupFailed = "BundleLookupFailed"
ReasonBundleDeploymentFailed = "BundleDeploymentFailed"
)

func init() {
Expand All @@ -44,7 +46,10 @@ func init() {
)
// TODO(user): add Reasons from above
operatorutil.ConditionReasons = append(operatorutil.ConditionReasons,
ReasonNotImplemented, ReasonResolutionSucceeded, ReasonResolutionFailed,
ReasonResolutionSucceeded,
ReasonResolutionFailed,
ReasonBundleLookupFailed,
ReasonBundleDeploymentFailed,
)
}

Expand Down
106 changes: 71 additions & 35 deletions controllers/operator_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package controllers

import (
"context"
"fmt"

"github.com/operator-framework/deppy/pkg/deppy/solver"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
"k8s.io/apimachinery/pkg/api/equality"
apimeta "k8s.io/apimachinery/pkg/api/meta"
Expand All @@ -35,6 +37,7 @@ import (
operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/resolution"
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
)

// OperatorReconciler reconciles a Operator object
Expand Down Expand Up @@ -105,54 +108,87 @@ func checkForUnexpectedFieldChange(a, b operatorsv1alpha1.Operator) bool {

// Helper function to do the actual reconcile
func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha1.Operator) (ctrl.Result, error) {
// define condition parameters
var status = metav1.ConditionTrue
var reason = operatorsv1alpha1.ReasonResolutionSucceeded
var message = "resolution was successful"

// run resolution
solution, err := r.Resolver.Resolve(ctx)
if err != nil {
status = metav1.ConditionFalse
reason = operatorsv1alpha1.ReasonResolutionFailed
message = err.Error()
} else {
// extract package bundle path from resolved variable
for _, variable := range solution.SelectedVariables() {
switch v := variable.(type) {
case *bundles_and_dependencies.BundleVariable:
packageName, err := v.BundleEntity().PackageName()
if err != nil {
return ctrl.Result{}, err
}
if packageName == op.Spec.PackageName {
bundlePath, err := v.BundleEntity().BundlePath()
if err != nil {
return ctrl.Result{}, err
}
// Create bundleDeployment if not found or Update if needed
dep := r.generateExpectedBundleDeployment(*op, bundlePath)
if err := r.ensureBundleDeployment(ctx, dep); err != nil {
return ctrl.Result{}, err
}
break
}
}
}
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: metav1.ConditionFalse,
Reason: operatorsv1alpha1.ReasonResolutionFailed,
Message: err.Error(),
ObservedGeneration: op.GetGeneration(),
})
return ctrl.Result{}, err
}

// lookup the bundle entity in the solution that corresponds to the
// Operator's desired package name.
bundleEntity, err := r.getBundleEntityFromSolution(solution, op.Spec.PackageName)
if err != nil {
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: metav1.ConditionFalse,
Reason: operatorsv1alpha1.ReasonBundleLookupFailed,
Message: err.Error(),
ObservedGeneration: op.GetGeneration(),
})
return ctrl.Result{}, err
}

// Get the bundle image reference for the bundle
bundleImage, err := bundleEntity.BundlePath()
if err != nil {
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: metav1.ConditionFalse,
Reason: operatorsv1alpha1.ReasonBundleLookupFailed,
Message: err.Error(),
ObservedGeneration: op.GetGeneration(),
})
return ctrl.Result{}, err
}

// Ensure a BundleDeployment exists with its bundle source from the bundle
// image we just looked up in the solution.
dep := r.generateExpectedBundleDeployment(*op, bundleImage)
if err := r.ensureBundleDeployment(ctx, dep); err != nil {
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: metav1.ConditionFalse,
Reason: operatorsv1alpha1.ReasonBundleDeploymentFailed,
Message: err.Error(),
ObservedGeneration: op.GetGeneration(),
})
return ctrl.Result{}, err
}

// update operator status
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: status,
Reason: reason,
Message: message,
Status: metav1.ConditionTrue,
Reason: operatorsv1alpha1.ReasonResolutionSucceeded,
Message: "resolution was successful",
ObservedGeneration: op.GetGeneration(),
})

return ctrl.Result{}, nil
}

func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*entity.BundleEntity, error) {
for _, variable := range solution.SelectedVariables() {
switch v := variable.(type) {
case *bundles_and_dependencies.BundleVariable:
entityPkgName, err := v.BundleEntity().PackageName()
if err != nil {
return nil, err
}
if packageName == entityPkgName {
return v.BundleEntity(), nil
}
}
}
return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
}

func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string) *unstructured.Unstructured {
// We use unstructured here to avoid problems of serializing default values when sending patches to the apiserver.
// If you use a typed object, any default values from that struct get serialized into the JSON patch, which could
Expand Down
14 changes: 8 additions & 6 deletions controllers/operator_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ var _ = Describe("Reconcile Test", func() {
By("running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
Expect(res).To(Equal(ctrl.Result{}))
// TODO: should resolution failure return an error?
Expect(err).NotTo(HaveOccurred())
Expect(err).To(MatchError(fmt.Sprintf("package '%s' not found", pkgName)))

By("fetching updated operator after reconcile")
Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
Expand Down Expand Up @@ -243,7 +242,7 @@ var _ = Describe("Reconcile Test", func() {
err := cl.Create(ctx, operator)
Expect(err).NotTo(HaveOccurred())
})
PIt("sets resolution failure status and returns an error", func() {
It("sets resolution failure status and returns an error", func() {
By("running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
Expect(res).To(Equal(ctrl.Result{}))
Expand All @@ -253,7 +252,11 @@ var _ = Describe("Reconcile Test", func() {
Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())

By("checking the expected conditions")
// TODO: there should be a condition update that sets Ready false in this scenario
cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleLookupFailed))
Expect(cond.Message).To(ContainSubstring(`error determining bundle path for entity`))
})
})
When("the operator specifies a duplicate package", func() {
Expand All @@ -277,8 +280,7 @@ var _ = Describe("Reconcile Test", func() {
By("running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
Expect(res).To(Equal(ctrl.Result{}))
// TODO: should this scenario return an error?
Expect(err).NotTo(HaveOccurred())
Expect(err).To(MatchError(Equal(`duplicate identifier "required package prometheus" in input`)))

By("fetching updated operator after reconcile")
Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
Expand Down