Introduce label selector based class scheduling by negz · Pull Request #48 · crossplane/crossplane-runtime

negz · 2019-10-10T19:20:49Z

Description of your changes

This enables label based class matching per #926. Changes will also be required to crossplane, and all stacks.

Checklist

I have:

Run make reviewable to ensure this PR is ready for review.
Ensured this PR contains a neat, self documenting set of commits.
Updated any relevant documentation, examples, or release notes.
Updated the RBAC permissions in clusterrole.yaml to include any new types.

crossplane/crossplane#926 crossplane/crossplane#927 crossplane/crossplane-runtime#48 Updates angryjet to reflect the above changes. Signed-off-by: Nic Cope <negz@rk0n.org>

hasheddan

This looks great! Cross-checked with the stated design and feels like it adheres. None of my comments are blocking so I will go ahead and approve, but also will be looking for the first stack implementation to review. Awesome work :)

apis/core/v1alpha1/resource.go

pkg/resource/claim_defaulting_reconciler.go

pkg/resource/claim_scheduling_reconciler.go

hasheddan · 2019-10-17T22:51:09Z

pkg/resource/configurator.go

-	// garbage collection, to determine whether to delete a managed resource
-	// when its claim is deleted per https://github.com/crossplaneio/crossplane/issues/550
+
+	// TODO(negz): Avoid setting this owner reference? Kubernetes specifies that


Do you know what happens if we try and set this owner reference?

I haven't looked at this for some time, but as far as I can tell it works. Currently we almost always dynamically provision a managed resource in a separate namespace from the resource claim, yet deleting the resource claim automatically deletes the managed resource (via owner reference based garbage collection).

What's interesting is that I have no idea how this works - I'm not sure how the garbage collector figures out what to delete without knowing the owner's namespace.

I am going to dive into this a little bit more, but I don't think my investigation needs to block here :)

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

apis/core/v1alpha1/resource.go

pkg/resource/claim_defaulting_reconciler.go

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz · 2019-10-22T09:12:55Z

Here's an example of simple resource class selection in action:

$ kubectl get redisclass -l classy="true"
NAME                     PROVIDER-REF   RECLAIM-POLICY   AGE
azure-redis-labelled-a   example        Delete           51m
azure-redis-labelled-b   example        Delete           51m
azure-redis-labelled-c   example        Delete           51m
azure-redis-labelled-d   example        Delete           51m
azure-redis-labelled-e   example        Delete           51m

$ kubectl get cloudmemorystoreinstanceclass -l classy="true"
NAME                   PROVIDER-REF   RECLAIM-POLICY   AGE
gcp-redis-labelled-a   example        Delete           50m
gcp-redis-labelled-b   example        Delete           49m
gcp-redis-labelled-c   example        Delete           49m
gcp-redis-labelled-d   example        Delete           49m
gcp-redis-labelled-e   example        Delete           49m

$ kubectl get rediscluster
NAME          STATUS   CLASS-KIND                      CLASS-NAME               RESOURCE-KIND              RESOURCE-NAME               AGE
app-redis-0            RedisClass                      azure-redis-labelled-b   Redis                      default-app-redis-0-8rhkr   4m31s
app-redis-1            RedisClass                      azure-redis-labelled-b   Redis                      default-app-redis-1-tbn7g   4m11s
app-redis-2            CloudMemorystoreInstanceClass   gcp-redis-labelled-a     CloudMemorystoreInstance   default-app-redis-2-mrqpm   3m52s
app-redis-3            RedisClass                      azure-redis-labelled-c   Redis                      default-app-redis-3-glr4s   3m36s
app-redis-4            RedisClass                      azure-redis-labelled-b   Redis                      default-app-redis-4-vdzc9   3m27s
app-redis-5            CloudMemorystoreInstanceClass   gcp-redis-labelled-b     CloudMemorystoreInstance   default-app-redis-5-2qt5r   3m20s
app-redis-6            RedisClass                      azure-redis-labelled-a   Redis                      default-app-redis-6-t7g59   3m11s
app-redis-7            CloudMemorystoreInstanceClass   gcp-redis-labelled-a     CloudMemorystoreInstance   default-app-redis-7-cwmpl   2m25s
app-redis-8            CloudMemorystoreInstanceClass   gcp-redis-labelled-b     CloudMemorystoreInstance   default-app-redis-8-9dcm4   2m25s
app-redis-9            CloudMemorystoreInstanceClass   gcp-redis-labelled-b     CloudMemorystoreInstance   default-app-redis-9-fdxqf   2m25s

The claims are all variants of:

---
apiVersion: cache.crossplane.io/v1alpha1
kind: RedisCluster
metadata:
  name: app-redis-0
spec:
  classSelector:
    matchLabels:
      classy: "true"
  writeConnectionSecretToRef:
    name: app-redis-0

This commit renames "non portable resource class" back to "resource class", and requires that resource claims reference a (non portable) resource class in any namespace. Signed-off-by: Nic Cope <negz@rk0n.org>

Signed-off-by: Nic Cope <negz@rk0n.org>

This allows us to require that name (and namespace where appropriate) are set at the CRD level. In the case of cluster scoped resources that reference secrets this is less surprising than defaulting to the `default` namespace when the namespace is omitted. Signed-off-by: Nic Cope <negz@rk0n.org>

Signed-off-by: Nic Cope <negz@rk0n.org>

For resource claim controllers, now that we're unconcerned with indirect resource classes. Signed-off-by: Nic Cope <negz@rk0n.org>

Signed-off-by: Nic Cope <negz@rk0n.org>

200ms seems low enough that GCP consistently beats Azure when scheduling RedisCluster claims in my experiments. Signed-off-by: Nic Cope <negz@rk0n.org>

crossplane/crossplane#926 crossplane/crossplane#927 crossplane/crossplane-runtime#48 Updates angryjet to reflect the above changes. Signed-off-by: Nic Cope <negz@rk0n.org>

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz force-pushed the notsoclassy branch 5 times, most recently from 8a53de7 to 933c6de Compare October 16, 2019 07:26

negz mentioned this pull request Oct 17, 2019

Implement Simple Resource Class Selection for Azure crossplane-contrib/provider-azure#34

Closed

negz mentioned this pull request Oct 17, 2019

Support simple resource class selection crossplane/crossplane-tools#7

Merged

2 tasks

hasheddan approved these changes Oct 17, 2019

View reviewed changes

negz force-pushed the notsoclassy branch 4 times, most recently from 25a5cfa to 6bd3d20 Compare October 20, 2019 07:34

negz added a commit to negz/provider-azure that referenced this pull request Oct 21, 2019

DO NOT MERGE: Use my crossplane and runtime branches

e81c325

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz added a commit to negz/provider-azure that referenced this pull request Oct 21, 2019

DO NOT MERGE: Use my crossplane and runtime branches

0c7a976

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz added a commit to negz/provider-azure that referenced this pull request Oct 21, 2019

DO NOT MERGE: Use my crossplane and runtime branches

be96a5b

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz marked this pull request as ready for review October 21, 2019 07:35

negz mentioned this pull request Oct 21, 2019

Simple Resource Class Selection crossplane/crossplane#952

Closed

11 tasks

negz added a commit to negz/provider-azure that referenced this pull request Oct 21, 2019

DO NOT MERGE: Use my crossplane and runtime branches

640d976

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

hasheddan reviewed Oct 21, 2019

View reviewed changes

apis/core/v1alpha1/resource.go Show resolved Hide resolved

pkg/resource/claim_defaulting_reconciler.go Outdated Show resolved Hide resolved

negz force-pushed the notsoclassy branch from 1d9d376 to aafb516 Compare October 21, 2019 21:57

negz added a commit to negz/provider-azure that referenced this pull request Oct 21, 2019

DO NOT MERGE: Use my crossplane and runtime branches

fc96a79

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz added a commit to negz/provider-azure that referenced this pull request Oct 22, 2019

DO NOT MERGE: Use my crossplane and runtime branches

5f4c46b

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

This was referenced Oct 22, 2019

Implement Simple Resource Class Selection for GCP crossplane-contrib/provider-gcp#57

Closed

Implement Simple Resource Class Selection for AWS crossplane-contrib/provider-aws#41

Closed

negz mentioned this pull request Oct 22, 2019

Migrate GCP to Simple Resource Class Selection crossplane-contrib/provider-gcp#56

Merged

4 tasks

negz added 7 commits October 22, 2019 17:12

Remove support for portable resource classes

a310ee6

This commit renames "non portable resource class" back to "resource class", and requires that resource claims reference a (non portable) resource class in any namespace. Signed-off-by: Nic Cope <negz@rk0n.org>

Assume resources, classes, and providers are cluster scoped

9f5188e

Signed-off-by: Nic Cope <negz@rk0n.org>

Add claim scheduling and defaulting controllers

d7b4db0

Signed-off-by: Nic Cope <negz@rk0n.org>

Add a HasResourceClass predicate

701387a

For resource claim controllers, now that we're unconcerned with indirect resource classes. Signed-off-by: Nic Cope <negz@rk0n.org>

Work around strange deepcopy bug

eb0bb8b

Signed-off-by: Nic Cope <negz@rk0n.org>

Increase claim scheduling and defaulting max jitter to 1.5 seconds

a81fb19

200ms seems low enough that GCP consistently beats Azure when scheduling RedisCluster claims in my experiments. Signed-off-by: Nic Cope <negz@rk0n.org>

negz mentioned this pull request Oct 23, 2019

Migrate AWS to Simple Resource Class Selection crossplane-contrib/provider-aws#44

Merged

9 tasks

negz force-pushed the notsoclassy branch from 8faf79f to a81fb19 Compare October 23, 2019 05:17

negz added a commit to negz/provider-azure that referenced this pull request Oct 23, 2019

DO NOT MERGE: Use my crossplane and runtime branches

2d5c3c7

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

hasheddan mentioned this pull request Oct 23, 2019

docs: stack-rook stacks guide crossplane/crossplane#964

Merged

4 tasks

negz added a commit to negz/provider-azure that referenced this pull request Oct 23, 2019

DO NOT MERGE: Use my crossplane and runtime branches

bebdaeb

crossplane/crossplane#927 crossplane/crossplane-runtime#48 Signed-off-by: Nic Cope <negz@rk0n.org>

negz merged commit 0f37bea into crossplane:master Oct 23, 2019

negz deleted the notsoclassy branch October 23, 2019 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce label selector based class scheduling#48

Introduce label selector based class scheduling#48
negz merged 7 commits intocrossplane:masterfrom
negz:notsoclassy

negz commented Oct 10, 2019 •

edited

Loading

Uh oh!

hasheddan left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hasheddan Oct 17, 2019

Uh oh!

negz Oct 17, 2019

Uh oh!

hasheddan Oct 18, 2019

Uh oh!

Uh oh!

Uh oh!

negz commented Oct 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

negz commented Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

Checklist

Uh oh!

hasheddan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hasheddan Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

negz Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

hasheddan Oct 18, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

negz commented Oct 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

negz commented Oct 10, 2019 •

edited

Loading