BundleDeployment creation with BundleImage #104
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,10 +19,12 @@ package controllers | |
| import ( | ||
| "context" | ||
|
|
||
| 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/runtime" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| utilerrors "k8s.io/apimachinery/pkg/util/errors" | ||
| ctrl "sigs.k8s.io/controller-runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
@@ -41,10 +43,20 @@ type OperatorReconciler struct { | |
| resolver *resolution.OperatorResolver | ||
| } | ||
|
|
||
| func NewOperatorReconciler(c client.Client, s *runtime.Scheme, r *resolution.OperatorResolver) *OperatorReconciler { | ||
| return &OperatorReconciler{ | ||
| Client: c, | ||
| Scheme: s, | ||
| resolver: r, | ||
| } | ||
| } | ||
|
|
||
| //+kubebuilder:rbac:groups=operators.operatorframework.io,resources=operators,verbs=get;list;watch;create;update;patch;delete | ||
| //+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 | ||
| // move the current state of the cluster closer to the desired state. | ||
| // TODO(user): Modify the Reconcile function to compare the state specified by | ||
|
|
@@ -125,8 +137,14 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha | |
| 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 | ||
| } | ||
| } | ||
|
|
@@ -145,16 +163,71 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha | |
| 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(), | ||
| }, | ||
| Spec: rukpakv1alpha1.BundleDeploymentSpec{ | ||
| //TODO: Don't assume plain provisioner | ||
|
There was a problem hiding this comment. +1 Long term, I envision a separate configuration API that sets up mappings from bundle properties to provisioner class names. Mid term, perhaps we get some mileage around a property that sets the provisioner class name directly. We have to be careful with this though because I expect Short term, what you have here will be fine for quite awhile, I imagine. |
||
| 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{ | ||
| // TODO: Don't assume image type | ||
|
There was a problem hiding this comment. IMO, we should make that bundle path property something a bit more definitive (e.g. bundle image reference). At that point, it would be totally safe to assume that we use the rukpak image type if we have that property. And that would then leave the door open for bundles to have different properties that map to a different source type. There was a problem hiding this comment. Agreed here. I've created another issue here #108 to address this, since I was hoping it wouldn't be a blocker for our shorter term goals. |
||
| Type: rukpakv1alpha1.SourceTypeImage, | ||
| Image: &rukpakv1alpha1.ImageSource{ | ||
| 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 | ||
| } | ||
|
|
||
| // SetupWithManager sets up the controller with the Manager. | ||
| func (r *OperatorReconciler) SetupWithManager(mgr ctrl.Manager) error { | ||
| err := ctrl.NewControllerManagedBy(mgr). | ||
| For(&operatorsv1alpha1.Operator{}). | ||
| Owns(&rukpakv1alpha1.BundleDeployment{}). | ||
| Complete(r) | ||
|
|
||
| if err != nil { | ||
| return err | ||
| } | ||
| 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) | ||
| } | ||
|
|
||
| // Check if the existing bundleDeployment's spec needs to be updated | ||
| if equality.Semantic.DeepEqual(existingBundleDeployment.Spec, desiredBundleDeployment.Spec) { | ||
| return nil | ||
| } | ||
|
|
||
| existingBundleDeployment.Spec = desiredBundleDeployment.Spec | ||
| return r.Client.Update(ctx, existingBundleDeployment) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not something you did (probably
kubebuilder), butmanager-roleis just too generic a name for aClusterRoleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is kubebuilder - and it also gets prepended with a prefix set in
config/defaults/kustomization.yaml:namePrefix: operator-controller-, so it ends up asoperator-controller-manager-role. Personally I also don't like this way of doing it much but it's also kind of a pain to go through and replace all the names with different values. Plus some of them don't get prepended and I've never quite figured out the rule for when that happens...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid touching it for now, keep in mind that all kubebuilder controllers follow this convention, so there are some assumptions about the name of this role.