diff --git a/api/v1alpha1/operator_types.go b/api/v1alpha1/operator_types.go index 43148a2a8..dfea7b4df 100644 --- a/api/v1alpha1/operator_types.go +++ b/api/v1alpha1/operator_types.go @@ -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 @@ -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() { @@ -44,7 +46,10 @@ func init() { ) // TODO(user): add Reasons from above operatorutil.ConditionReasons = append(operatorutil.ConditionReasons, - ReasonNotImplemented, ReasonResolutionSucceeded, ReasonResolutionFailed, + ReasonResolutionSucceeded, + ReasonResolutionFailed, + ReasonBundleLookupFailed, + ReasonBundleDeploymentFailed, ) } diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 02b255528..6caa5b6b1 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -21,12 +21,8 @@ rules: resources: - operators verbs: - - create - - delete - get - list - - patch - - update - watch - apiGroups: - operators.operatorframework.io diff --git a/controllers/operator_controller.go b/controllers/operator_controller.go index 306dc5a06..24c09e479 100644 --- a/controllers/operator_controller.go +++ b/controllers/operator_controller.go @@ -18,14 +18,18 @@ 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" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -33,28 +37,19 @@ 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 type OperatorReconciler struct { client.Client - Scheme *runtime.Scheme - - resolver *resolution.OperatorResolver -} - -func NewOperatorReconciler(c client.Client, s *runtime.Scheme, r *resolution.OperatorResolver) *OperatorReconciler { - return &OperatorReconciler{ - Client: c, - Scheme: s, - resolver: r, - } + Scheme *runtime.Scheme + Resolver *resolution.OperatorResolver } -//+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators,verbs=get;list;watch //+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators/status,verbs=get;update;patch //+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators/finalizers,verbs=update - //+kubebuilder:rbac:groups=core.rukpak.io,resources=bundledeployments,verbs=get;list;watch;create;update;patch // Reconcile is part of the main kubernetes reconciliation loop which aims to @@ -112,92 +107,128 @@ 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) + solution, err := r.Resolver.Resolve(ctx) if err != nil { - status = metav1.ConditionTrue - 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 - } - dep, err := r.generateExpectedBundleDeployment(*op, bundlePath) - if err != nil { - return ctrl.Result{}, err - } - // Create bundleDeployment if not found or Update if needed - 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) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string) (*rukpakv1alpha1.BundleDeployment, error) { - dep := &rukpakv1alpha1.BundleDeployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: o.GetName(), +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 + // cause unrelated fields to be patched back to the default value even though that isn't the intention. Using an + // unstructured ensures that the patch contains only what is specified. Using unstructured like this is basically + // identical to "kubectl apply -f" + bd := &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": rukpakv1alpha1.GroupVersion.String(), + "kind": rukpakv1alpha1.BundleDeploymentKind, + "metadata": map[string]interface{}{ + "name": o.GetName(), }, - Spec: rukpakv1alpha1.BundleDeploymentSpec{ - //TODO: Don't assume plain provisioner - ProvisionerClassName: "core-rukpak-io-plain", - Template: &rukpakv1alpha1.BundleTemplate{ - ObjectMeta: metav1.ObjectMeta{ - // TODO: Remove - Labels: map[string]string{ - "app": "my-bundle", - }, - }, - Spec: rukpakv1alpha1.BundleSpec{ - Source: rukpakv1alpha1.BundleSource{ + "spec": map[string]interface{}{ + // TODO: Don't assume plain provisioner + "provisionerClassName": "core-rukpak-io-plain", + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + // TODO: Don't assume registry provisioner + "provisionerClassName": "core-rukpak-io-registry", + "source": map[string]interface{}{ // TODO: Don't assume image type - Type: rukpakv1alpha1.SourceTypeImage, - Image: &rukpakv1alpha1.ImageSource{ - Ref: bundlePath, + "type": string(rukpakv1alpha1.SourceTypeImage), + "image": map[string]interface{}{ + "ref": bundlePath, }, }, - - //TODO: Don't assume registry provisioner - ProvisionerClassName: "core-rukpak-io-registry", }, }, }, - } - - if err := ctrl.SetControllerReference(&o, dep, r.Scheme); err != nil { - return nil, err - } - return dep, nil + }} + bd.SetOwnerReferences([]metav1.OwnerReference{ + { + APIVersion: operatorsv1alpha1.GroupVersion.String(), + Kind: "Operator", + Name: o.Name, + UID: o.UID, + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + }, + }) + return bd } // SetupWithManager sets up the controller with the Manager. @@ -213,21 +244,30 @@ func (r *OperatorReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } -func (r *OperatorReconciler) ensureBundleDeployment(ctx context.Context, desiredBundleDeployment *rukpakv1alpha1.BundleDeployment) error { - existingBundleDeployment := &rukpakv1alpha1.BundleDeployment{} - err := r.Client.Get(ctx, types.NamespacedName{Name: desiredBundleDeployment.GetName()}, existingBundleDeployment) - if err != nil { - if client.IgnoreNotFound(err) != nil { - return err - } - return r.Client.Create(ctx, desiredBundleDeployment) +func (r *OperatorReconciler) ensureBundleDeployment(ctx context.Context, desiredBundleDeployment *unstructured.Unstructured) error { + existingBundleDeployment, err := r.existingBundleDeploymentUnstructured(ctx, desiredBundleDeployment.GetName()) + if client.IgnoreNotFound(err) != nil { + return err } - // Check if the existing bundleDeployment's spec needs to be updated - if equality.Semantic.DeepEqual(existingBundleDeployment.Spec, desiredBundleDeployment.Spec) { + // If the existing BD already has everything that the desired BD has, no need to contact the API server. + if equality.Semantic.DeepDerivative(desiredBundleDeployment, existingBundleDeployment) { return nil } + return r.Client.Patch(ctx, desiredBundleDeployment, client.Apply, client.ForceOwnership, client.FieldOwner("operator-controller")) +} - existingBundleDeployment.Spec = desiredBundleDeployment.Spec - return r.Client.Update(ctx, existingBundleDeployment) +func (r *OperatorReconciler) existingBundleDeploymentUnstructured(ctx context.Context, name string) (*unstructured.Unstructured, error) { + existingBundleDeployment := &rukpakv1alpha1.BundleDeployment{} + err := r.Client.Get(ctx, types.NamespacedName{Name: name}, existingBundleDeployment) + if err != nil { + return nil, err + } + existingBundleDeployment.APIVersion = rukpakv1alpha1.GroupVersion.String() + existingBundleDeployment.Kind = rukpakv1alpha1.BundleDeploymentKind + unstrExistingBundleDeploymentObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(existingBundleDeployment) + if err != nil { + return nil, err + } + return &unstructured.Unstructured{Object: unstrExistingBundleDeploymentObj}, nil } diff --git a/controllers/operator_controller_test.go b/controllers/operator_controller_test.go new file mode 100644 index 000000000..4e17284d4 --- /dev/null +++ b/controllers/operator_controller_test.go @@ -0,0 +1,344 @@ +package controllers_test + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/deppy/pkg/deppy/input" + rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/controllers" + "github.com/operator-framework/operator-controller/internal/resolution" + operatorutil "github.com/operator-framework/operator-controller/internal/util" +) + +var _ = Describe("Reconcile Test", func() { + var ( + ctx context.Context + reconciler *controllers.OperatorReconciler + ) + BeforeEach(func() { + ctx = context.Background() + reconciler = &controllers.OperatorReconciler{ + Client: cl, + Scheme: sch, + Resolver: resolution.NewOperatorResolver(cl, testEntitySource), + } + }) + When("the operator does not exist", func() { + It("returns no error", func() { + res, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: "non-existent"}}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).NotTo(HaveOccurred()) + }) + }) + When("the operator exists", func() { + var ( + operator *operatorsv1alpha1.Operator + opKey types.NamespacedName + ) + BeforeEach(func() { + opKey = types.NamespacedName{Name: fmt.Sprintf("operator-test-%s", rand.String(8))} + }) + When("the operator specifies a non-existent package", func() { + var pkgName string + BeforeEach(func() { + By("initializing cluster state") + pkgName = fmt.Sprintf("non-existent-%s", rand.String(6)) + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName}, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution failure status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + 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()) + + By("checking the expected conditions") + 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.ReasonResolutionFailed)) + Expect(cond.Message).To(Equal(fmt.Sprintf("package '%s' not found", pkgName))) + }) + }) + When("the operator specifies a valid available package", func() { + // TODO: add sub-scenarios -- When("BD does not exist") and When("BD already exists"). + const pkgName = "prometheus" + BeforeEach(func() { + By("initializing cluster state") + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName}, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + When("the BundleDeployment does not exist", func() { + BeforeEach(func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).NotTo(HaveOccurred()) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + }) + It("results in the expected BundleDeployment", func() { + bd := &rukpakv1alpha1.BundleDeployment{} + err := cl.Get(ctx, types.NamespacedName{Name: opKey.Name}, bd) + Expect(err).NotTo(HaveOccurred()) + Expect(bd.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain")) + Expect(bd.Spec.Template.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-registry")) + Expect(bd.Spec.Template.Spec.Source.Type).To(Equal(rukpakv1alpha1.SourceTypeImage)) + Expect(bd.Spec.Template.Spec.Source.Image).NotTo(BeNil()) + Expect(bd.Spec.Template.Spec.Source.Image.Ref).To(Equal("quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed")) + }) + It("sets resolution success status", func() { + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionSucceeded)) + Expect(cond.Message).To(Equal("resolution was successful")) + }) + }) + When("the expected BundleDeployment already exists", func() { + BeforeEach(func() { + By("patching the existing BD") + err := cl.Create(ctx, &rukpakv1alpha1.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: opKey.Name, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: operatorsv1alpha1.GroupVersion.String(), + Kind: "Operator", + Name: operator.Name, + UID: operator.UID, + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + }, + }, + }, + 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", + }, + }, + }, + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).NotTo(HaveOccurred()) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + }) + PIt("does not patch the BundleDeployment", func() { + // TODO: verify that no patch call is made. + }) + It("results in the expected BundleDeployment", func() { + bd := &rukpakv1alpha1.BundleDeployment{} + err := cl.Get(ctx, types.NamespacedName{Name: opKey.Name}, bd) + Expect(err).NotTo(HaveOccurred()) + Expect(bd.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain")) + Expect(bd.Spec.Template.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-registry")) + Expect(bd.Spec.Template.Spec.Source.Type).To(Equal(rukpakv1alpha1.SourceTypeImage)) + Expect(bd.Spec.Template.Spec.Source.Image).NotTo(BeNil()) + Expect(bd.Spec.Template.Spec.Source.Image.Ref).To(Equal("quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed")) + }) + It("sets resolution success status", func() { + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionSucceeded)) + Expect(cond.Message).To(Equal("resolution was successful")) + }) + }) + When("an out-of-date BundleDeployment exists", func() { + BeforeEach(func() { + By("creating the expected BD") + err := cl.Create(ctx, &rukpakv1alpha1.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: rukpakv1alpha1.BundleDeploymentSpec{ + ProvisionerClassName: "foo", + Template: &rukpakv1alpha1.BundleTemplate{ + Spec: rukpakv1alpha1.BundleSpec{ + ProvisionerClassName: "bar", + Source: rukpakv1alpha1.BundleSource{ + Type: rukpakv1alpha1.SourceTypeHTTP, + HTTP: &rukpakv1alpha1.HTTPSource{ + URL: "http://localhost:8080/", + }, + }, + }, + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).NotTo(HaveOccurred()) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + }) + It("results in the expected BundleDeployment", func() { + bd := &rukpakv1alpha1.BundleDeployment{} + err := cl.Get(ctx, types.NamespacedName{Name: opKey.Name}, bd) + Expect(err).NotTo(HaveOccurred()) + Expect(bd.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain")) + Expect(bd.Spec.Template.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-registry")) + Expect(bd.Spec.Template.Spec.Source.Type).To(Equal(rukpakv1alpha1.SourceTypeImage)) + Expect(bd.Spec.Template.Spec.Source.Image).NotTo(BeNil()) + Expect(bd.Spec.Template.Spec.Source.Image.Ref).To(Equal("quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed")) + }) + It("sets resolution success status", func() { + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionSucceeded)) + Expect(cond.Message).To(Equal("resolution was successful")) + }) + }) + }) + When("the selected bundle's image ref cannot be parsed", func() { + const pkgName = "badimage" + BeforeEach(func() { + By("initializing cluster state") + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName}, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + 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{})) + Expect(err).To(MatchError(ContainSubstring(`error determining bundle path for entity`))) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + 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() { + const pkgName = "prometheus" + BeforeEach(func() { + By("initializing cluster state") + err := cl.Create(ctx, &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("orig-%s", opKey.Name)}, + Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName}, + }) + Expect(err).NotTo(HaveOccurred()) + + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName}, + } + err = cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution failure status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).To(MatchError(`duplicate identifier "required package prometheus" in input`)) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + 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.ReasonResolutionFailed)) + Expect(cond.Message).To(Equal(`duplicate identifier "required package prometheus" in input`)) + }) + }) + AfterEach(func() { + verifyInvariants(ctx, operator) + + err := cl.Delete(ctx, operator) + Expect(err).To(Not(HaveOccurred())) + }) + }) +}) + +func verifyInvariants(ctx context.Context, op *operatorsv1alpha1.Operator) { + key := client.ObjectKeyFromObject(op) + err := cl.Get(ctx, key, op) + Expect(err).To(BeNil()) + + verifyConditionsInvariants(op) +} + +func verifyConditionsInvariants(op *operatorsv1alpha1.Operator) { + // Expect that the operator's set of conditions contains all defined + // condition types for the Operator API. Every reconcile should always + // ensure every condition type's status/reason/message reflects the state + // read during _this_ reconcile call. + Expect(op.Status.Conditions).To(HaveLen(len(operatorutil.ConditionTypes))) + for _, t := range operatorutil.ConditionTypes { + cond := apimeta.FindStatusCondition(op.Status.Conditions, t) + Expect(cond).To(Not(BeNil())) + Expect(cond.Status).NotTo(BeEmpty()) + Expect(cond.Reason).To(BeElementOf(operatorutil.ConditionReasons)) + Expect(cond.ObservedGeneration).To(Equal(op.GetGeneration())) + } +} + +var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ + "operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{ + "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`, + "olm.channel": `{"channelName":"beta","priority":0}`, + "olm.package": `{"packageName":"prometheus","version":"0.37.0"}`, + }), + "operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{ + "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`, + "olm.channel": `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`, + "olm.package": `{"packageName":"prometheus","version":"0.47.0"}`, + }), + "operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{ + "olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`, + "olm.package": `{"packageName":"badimage","version":"0.1.0"}`, + }), +}) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 8925bfcb8..556b0bc1c 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -17,42 +17,30 @@ limitations under the License. package controllers_test import ( - "context" - "fmt" "path/filepath" "testing" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - apimeta "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/rand" - "k8s.io/client-go/kubernetes/scheme" + rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/controllers" - "github.com/operator-framework/operator-controller/internal/resolution" - operatorutil "github.com/operator-framework/operator-controller/internal/util" - rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" - //+kubebuilder:scaffold:imports ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. var ( - cfg *rest.Config - k8sClient client.Client - testEnv *envtest.Environment - ctx context.Context - cancel context.CancelFunc + cfg *rest.Config + cl client.Client + sch *runtime.Scheme + testEnv *envtest.Environment ) func TestAPIs(t *testing.T) { @@ -77,123 +65,19 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(cfg).NotTo(BeNil()) - err = operatorsv1alpha1.AddToScheme(scheme.Scheme) + sch = runtime.NewScheme() + err = operatorsv1alpha1.AddToScheme(sch) Expect(err).NotTo(HaveOccurred()) - - err = rukpakv1alpha1.AddToScheme(scheme.Scheme) + err = rukpakv1alpha1.AddToScheme(sch) Expect(err).NotTo(HaveOccurred()) - //+kubebuilder:scaffold:scheme - - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + cl, err = client.New(cfg, client.Options{Scheme: sch}) Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) - - k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, - }) - Expect(err).ToNot(HaveOccurred()) - - err = controllers.NewOperatorReconciler( - k8sManager.GetClient(), - k8sManager.GetScheme(), - resolution.NewOperatorResolver(k8sManager.GetClient(), resolution.HardcodedEntitySource), - ).SetupWithManager(k8sManager) - Expect(err).ToNot(HaveOccurred()) - - ctx, cancel = context.WithCancel(context.Background()) - go func() { - defer GinkgoRecover() - err = k8sManager.Start(ctx) - Expect(err).ToNot(HaveOccurred(), "failed to run manager") - }() - + Expect(cl).NotTo(BeNil()) }) var _ = AfterSuite(func() { - cancel() By("tearing down the test environment") err := testEnv.Stop() Expect(err).NotTo(HaveOccurred()) }) - -var _ = Describe("Reconcile Test", func() { - const ( - timeout = time.Second * 10 - interval = time.Millisecond * 250 - ) - - When("an Operator is created", func() { - var ( - operator *operatorsv1alpha1.Operator - ctx context.Context - opName string - pkgName string - err error - ) - BeforeEach(func() { - ctx = context.Background() - opName = fmt.Sprintf("operator-test-%s", rand.String(8)) - pkgName = fmt.Sprintf("package-test-%s", rand.String(8)) - operator = &operatorsv1alpha1.Operator{ - ObjectMeta: metav1.ObjectMeta{ - Name: opName, - }, - Spec: operatorsv1alpha1.OperatorSpec{ - PackageName: pkgName, - }, - } - err = k8sClient.Create(ctx, operator) - Expect(err).To(Not(HaveOccurred())) - }) - AfterEach(func() { - err = k8sClient.Delete(ctx, operator) - Expect(err).To(Not(HaveOccurred())) - }) - It("has all Conditions created", func() { - op := &operatorsv1alpha1.Operator{} - opLookupKey := client.ObjectKey{Name: opName} - Eventually(func() bool { - err := k8sClient.Get(ctx, opLookupKey, op) - if err != nil { - return false - } - return len(op.Status.Conditions) > 0 - }, timeout, interval).Should(BeTrue()) - - // All defined condition Types MUST exist after reconciliation - conds := op.Status.Conditions - Expect(conds).To(Not(BeEmpty())) - Expect(conds).To(HaveLen(len(operatorutil.ConditionTypes))) - for _, t := range operatorutil.ConditionTypes { - Expect(apimeta.FindStatusCondition(conds, t)).To(Not(BeNil())) - } - }) - It("has matching generations in Conditions", func() { - op := &operatorsv1alpha1.Operator{} - - err = k8sClient.Get(ctx, client.ObjectKey{ - Name: opName, - }, op) - Expect(err).To(Not(HaveOccurred())) - - // The ObservedGeneration MUST match the resource generation after reconciliation - for _, c := range op.Status.Conditions { - Expect(c.ObservedGeneration).To(Equal(op.GetGeneration())) - } - }) - It("has only pre-defined Reasons", func() { - op := &operatorsv1alpha1.Operator{} - - err = k8sClient.Get(ctx, client.ObjectKey{ - Name: opName, - }, op) - Expect(err).To(Not(HaveOccurred())) - - // A given Reason MUST be in the list of ConditionReasons - for _, c := range op.Status.Conditions { - Expect(c.Reason).To(BeElementOf(operatorutil.ConditionReasons)) - } - }) - }) -}) diff --git a/go.mod b/go.mod index 2b032d70e..4b1156756 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/operator-framework/rukpak v0.11.0 k8s.io/apimachinery v0.25.0 k8s.io/client-go v0.25.0 + k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed sigs.k8s.io/controller-runtime v0.13.1 ) @@ -80,7 +81,6 @@ require ( k8s.io/component-base v0.25.0 // indirect k8s.io/klog/v2 v2.70.1 // indirect k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect - k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed // indirect sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect diff --git a/internal/resolution/variable_sources/entity/bundle_entity.go b/internal/resolution/variable_sources/entity/bundle_entity.go index 350566a1d..af59cda7d 100644 --- a/internal/resolution/variable_sources/entity/bundle_entity.go +++ b/internal/resolution/variable_sources/entity/bundle_entity.go @@ -121,7 +121,7 @@ func (b *BundleEntity) loadPackage() error { b.mu.Lock() defer b.mu.Unlock() if b.bundlePackage == nil { - bundlePackage, err := loadFromEntity[property.Package](b.Entity, property.TypePackage) + bundlePackage, err := loadFromEntity[property.Package](b.Entity, property.TypePackage, true) if err != nil { return fmt.Errorf("error determining package for entity '%s': %w", b.ID, err) } @@ -141,7 +141,7 @@ func (b *BundleEntity) loadProvidedGVKs() error { b.mu.Lock() defer b.mu.Unlock() if b.providedGVKs == nil { - providedGVKs, err := loadFromEntity[[]GVK](b.Entity, property.TypeGVK) + providedGVKs, err := loadFromEntity[[]GVK](b.Entity, property.TypeGVK, false) if err != nil { return fmt.Errorf("error determining bundle provided gvks for entity '%s': %w", b.ID, err) } @@ -154,7 +154,7 @@ func (b *BundleEntity) loadRequiredGVKs() error { b.mu.Lock() defer b.mu.Unlock() if b.requiredGVKs == nil { - requiredGVKs, err := loadFromEntity[[]GVKRequired](b.Entity, property.TypeGVKRequired) + requiredGVKs, err := loadFromEntity[[]GVKRequired](b.Entity, property.TypeGVKRequired, false) if err != nil { return fmt.Errorf("error determining bundle required gvks for entity '%s': %w", b.ID, err) } @@ -167,7 +167,7 @@ func (b *BundleEntity) loadRequiredPackages() error { b.mu.Lock() defer b.mu.Unlock() if b.requiredPackages == nil { - requiredPackages, err := loadFromEntity[[]PackageRequired](b.Entity, property.TypePackageRequired) + requiredPackages, err := loadFromEntity[[]PackageRequired](b.Entity, property.TypePackageRequired, false) if err != nil { return fmt.Errorf("error determining bundle required packages for entity '%s': %w", b.ID, err) } @@ -187,7 +187,7 @@ func (b *BundleEntity) loadChannelProperties() error { b.mu.Lock() defer b.mu.Unlock() if b.channelProperties == nil { - channel, err := loadFromEntity[ChannelProperties](b.Entity, property.TypeChannel) + channel, err := loadFromEntity[ChannelProperties](b.Entity, property.TypeChannel, true) if err != nil { return fmt.Errorf("error determining bundle channel properties for entity '%s': %w", b.ID, err) } @@ -200,7 +200,7 @@ func (b *BundleEntity) loadBundlePath() error { b.mu.Lock() defer b.mu.Unlock() if b.bundlePath == "" { - bundlePath, err := loadFromEntity[string](b.Entity, PropertyBundlePath) + bundlePath, err := loadFromEntity[string](b.Entity, PropertyBundlePath, true) if err != nil { return fmt.Errorf("error determining bundle path for entity '%s': %w", b.ID, err) } @@ -209,15 +209,15 @@ func (b *BundleEntity) loadBundlePath() error { return nil } -func loadFromEntity[T interface{}](entity *input.Entity, propertyName string) (T, error) { +func loadFromEntity[T interface{}](entity *input.Entity, propertyName string, required bool) (T, error) { deserializedProperty := *new(T) propertyValue, ok := entity.Properties[propertyName] - if !ok { + if ok { + if err := json.Unmarshal([]byte(propertyValue), &deserializedProperty); err != nil { + return deserializedProperty, fmt.Errorf("property '%s' ('%s') could not be parsed: %w", propertyName, propertyValue, err) + } + } else if required { return deserializedProperty, fmt.Errorf("property '%s' not found", propertyName) } - - if err := json.Unmarshal([]byte(propertyValue), &deserializedProperty); err != nil { - return deserializedProperty, fmt.Errorf("property '%s' ('%s') could not be parsed: %w", propertyName, propertyValue, err) - } return deserializedProperty, nil } diff --git a/internal/resolution/variable_sources/entity/bundle_entity_test.go b/internal/resolution/variable_sources/entity/bundle_entity_test.go index 328f5bb7c..52d667b51 100644 --- a/internal/resolution/variable_sources/entity/bundle_entity_test.go +++ b/internal/resolution/variable_sources/entity/bundle_entity_test.go @@ -96,12 +96,12 @@ var _ = Describe("BundleEntity", func() { {Group: "bar.io", Kind: "Bar", Version: "v1alpha1"}, })) }) - It("should return an error if the property is not found", func() { + It("should return nil if the property is not found", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) bundleEntity := olmentity.NewBundleEntity(entity) providedGvks, err := bundleEntity.ProvidedGVKs() + Expect(err).ToNot(HaveOccurred()) Expect(providedGvks).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle provided gvks for entity 'operatorhub/prometheus/0.14.0': property 'olm.gvk' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -127,12 +127,12 @@ var _ = Describe("BundleEntity", func() { {Group: "bar.io", Kind: "Bar", Version: "v1alpha1"}, })) }) - It("should return an error if the property is not found", func() { + It("should return nil if the property is not found", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) bundleEntity := olmentity.NewBundleEntity(entity) requiredGvks, err := bundleEntity.RequiredGVKs() + Expect(err).ToNot(HaveOccurred()) Expect(requiredGvks).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle required gvks for entity 'operatorhub/prometheus/0.14.0': property 'olm.gvk.required' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -158,12 +158,12 @@ var _ = Describe("BundleEntity", func() { {PackageRequired: property.PackageRequired{PackageName: "packageB", VersionRange: ">0.5.0 <0.8.6"}}, })) }) - It("should return an error if the property is not found", func() { + It("should return nil if the property is not found", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) bundleEntity := olmentity.NewBundleEntity(entity) requiredPackages, err := bundleEntity.RequiredPackages() + Expect(err).ToNot(HaveOccurred()) Expect(requiredPackages).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle required packages for entity 'operatorhub/prometheus/0.14.0': property 'olm.package.required' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ diff --git a/main.go b/main.go index a11ae31c1..8e098cd0b 100644 --- a/main.go +++ b/main.go @@ -20,13 +20,11 @@ import ( "flag" "os" - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) - // to ensure that exec-entrypoint and run can make use of them. - _ "k8s.io/client-go/plugin/pkg/client/auth" - + rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + _ "k8s.io/client-go/plugin/pkg/client/auth" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -34,8 +32,6 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/controllers" "github.com/operator-framework/operator-controller/internal/resolution" - rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" - //+kubebuilder:scaffold:imports ) var ( @@ -92,11 +88,11 @@ func main() { os.Exit(1) } - if err = controllers.NewOperatorReconciler( - mgr.GetClient(), - mgr.GetScheme(), - resolution.NewOperatorResolver(mgr.GetClient(), resolution.HardcodedEntitySource), - ).SetupWithManager(mgr); err != nil { + if err = (&controllers.OperatorReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Resolver: resolution.NewOperatorResolver(mgr.GetClient(), resolution.HardcodedEntitySource), + }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Operator") os.Exit(1) }