-
Notifications
You must be signed in to change notification settings - Fork 95
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.
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
Fix support for static provisioning #24
Conversation
Controllers built against crossplane-runtime will currently panic if they encounter a managed resource without a class reference. Any dynamically provisioned managed resource will have a class reference, but this breaks the static provisioning workflow. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static binding compatible predicates introduced in the above PR. Signed-off-by: Nic Cope <negz@rk0n.org>
I've tested that both the static and dynamic provisioning flows work by using a build of crossplane-contrib/provider-gcp#24 to statically and then dynamically provision a |
crossplane/crossplane-runtime#24 Use the static provisioning compatible predicates introduced in the above PR. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static provisioning compatible predicates introduced in the above PR. Signed-off-by: Nic Cope <negz@rk0n.org>
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.
Reviewed in conjunction with crossplane-contrib/provider-gcp#24. LGTM!
We currently support dynamic provisioning in the resource claim reconciler by using a watch predicate that allows either managed resources that directly reference a non-portable resource class of a given kind, or resource claims that reference a non-portable resource class of a given kind indirectly via a portable resource class. To support static provisioning (i.e. explicitly claiming an existing managed resource) we must also allow resource claims that explicitly reference a managed resource. Writing one predicate to do all of this was getting cumbersome, so I have refactored the predicate interface a little. Signed-off-by: Nic Cope <negz@rk0n.org>
...because I am pedantic. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static binding compatible predicates introduced in the above PR. Note the updates to CRDs are due to recent documentation changes in core fields exposed by crossplane-runtime. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static provisioning compatible predicates introduced in the above PR. Note the updates to CRDs are due to recent documentation changes in core fields exposed by crossplane-runtime. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static provisioning compatible predicates introduced in the above PR. Note the updates to CRDs are due to recent documentation changes in core fields exposed by crossplane-runtime. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static provisioning compatible predicates introduced in the above PR. Note the updates to CRDs are due to recent documentation changes in core fields exposed by crossplane-runtime. Signed-off-by: Nic Cope <negz@rk0n.org>
crossplane/crossplane-runtime#24 Use the static provisioning compatible predicates introduced in the above PR. Note the updates to CRDs are due to recent documentation changes in core fields exposed by crossplane-runtime.
Description of your changes
This PR fixes support for managed resources that do not reference a class. Controllers built against crossplane-runtime will currently panic if they encounter a managed resource without a class reference. Any dynamically provisioned managed resource will have a class reference, but this breaks the static provisioning workflow in which a managed resource is explicitly provisioned, then later claimed.
The PR also fixes watch predicates for the static binding flow We currently support dynamic provisioning in the resource claim reconciler by using a watch predicate that allows either managed resources that directly reference a non-portable resource class of a given kind, or resource claims that reference a non-portable resource class of a given kind indirectly via a portable resource class.
To support static provisioning (i.e. explicitly claiming an existing managed resource) we must also allow resource claims that explicitly reference a managed resource. Writing one predicate to do all of this was getting cumbersome, so I have refactored the predicate interface a little.
This PR will require additional PRs to update Crossplane core, and each stack.
Checklist
I have:
make reviewable
to ensure this PR is ready for review.clusterrole.yaml
to include any new types.