Investigate the use of CRD sub-resources. #643

mattmoor · 2018-04-13T14:31:35Z

IIRC K8s 1.10 added support for "status" sub-resources in CRDs.

There are a variety of places this is useful to us, including updateStatus in each of the controllers, and (likely) the validation logic in the webhook.

Let's scout this functionality in M4, and if useful adopt.

See also this issue, which tracks a 1.10 update.

The text was updated successfully, but these errors were encountered:

grantr · 2018-04-13T15:46:21Z

I also think the Scale sub-resource would be a useful addition to revisions as a standard way for autoscalers to effect scaling. This gives the revision controller the option of modifying or ignoring the request, unlike the alternative of autoscaling the deployment directly.

mattmoor · 2018-04-13T16:52:57Z

Yes perhaps, although I'm not sure the form that takes is appropriate for us?

I think the payload is a single integer (foggy recollection of things people told me), which feels like the kind of thing we have been avoiding in the spec, since that feels like "how many servers" and we're after "serverless".

That said, I could see this being useful to force Reserve -> Active with some minimum "concurrent request" capacity in anticipation of load spikes beyond our capacity to hyperscale up (e.g. if more cluster capacity is needed).

cc @josephburnett for this discussion, but let's track that with a separate issue since it involves a bit of design and specification.

grantr · 2018-04-13T18:43:59Z

It's less about users and more about implementors of autoscaling strategies. But you're right, off-topic, plus @josephburnett is out until Tuesday and I don't want to start any official discussions without him. 😃 This document in the team drive has more details if anyone is interested.

mattmoor · 2018-04-17T21:35:45Z

/assign @grantr

mattmoor · 2018-04-18T04:33:47Z

Moving to M5, since the GKE 1.10 alpha clusters have been showing some issues. I don't think the importance of this has diminished, but I'm less optimistic that we'll be able to make the switch to 1.10 smoothly in M4.

mattmoor · 2018-05-24T15:37:13Z

/assign @dprotaso @bsnchan

Since they have a PR. I believe this is blocked on 1.10, which is coming in hot for M5 (it is GA in GKE, but some of our monitoring stuff has issues with it). I'm not particularly optimistic that this will land.

sslavic · 2018-06-28T10:14:03Z

If I understood correctly, CRD with subresources is in v1beta1 in k8s 1.11 https://kubernetes.io/blog/2018/06/27/kubernetes-1.11-release-announcement/

mattmoor · 2018-11-06T01:11:45Z

@dprotaso @bsnchan This should be unblocked. GKE has 1.11, so we can actually test with sub-resources 🎉

dprotaso · 2019-01-02T16:12:01Z

Here's a backwards compatible strategy for adopting CRD status subresource with the aim of eventually dropping the generation bumping that's occurring in the webhook.

Phase 1 - Serving 0.3

Changes

Enable status subresource on all CRDs

This causes the API server to start bumping a resource's metadata.generation (this is actually the default in K8s 1.13 I believe).
Our controllers can and will now use the /status endpoint. This prevents them from 'accidentally' updating the reconciled object's spec

Drop our functional dependency on spec.generation

Don't depend on it for garbage collection - Don't use spec.generation when garbage collecting revisions #2765
Mark functions using spec.generation as deprecated ie. Configuration controller resourcenames.Revision
The configuration controller now adds a revision label based on a config's metadata.generation
The configuration controller uses this new label to determine the 'LatestCreatedRevision'
- Fallback on querying using the revision name (based on spec.generation)
- When falling back add the new label based on a configuration's metadata.generation (this is essentially a migration of the latest created revision)

Implications

Adding a new label implies we are now modifying a subset of revisions. This requires a change in the revision reconciler since all labels were propagated to the revision's deployment's selector
- Fixed here: Reduce a deployments selector to only match the revision uid #2818
- Downgrading should work as we don't reconcile deployments in Serving 0.2
Operators will need to reconcile all Configurations prior to upgrading to the next release (ie. 0.4)

Phase 2 - Serving 0.4

Changes

Fully drop our dependency on spec.generation

Remove Webhook logic for incrementing Spec.Generation
Change the naming strategy of revisions & builds
- Delete code old naming scheme code

Remove Spec.Generation from the API (under consideration)

This was never really a user facing detail that could be controlled
We should ensure if it appears in a spec it's a noop

Change the label applied to revisions from configurationMetadataGeneration back to configurationGeneration

configurationGeneration became 'unavailable' during 0.3 but should now be able to be reclaimed
this would require falling back onto configurationMetadataGeneration and performing another migration

Implications

Changing the revision naming scheme will be potentially confusing for users

Phase 3 - Serving 0.5

Changes

Drop functional dependency on configurationMetadataGeneration

Phase 4 - Serving 0.6

Changes

No longer apply the configurationMetadataGeneration label to revisions

Phase X - when conversion webhooks are beta

Changes

Should be able to drop Spec.Generation in the API officially by creating a new CRD version
Webhooks drop Spec.Generation when we upgrade the CRD to the new version

Implications

Technically 'round-tripability' isn't possible if we drop the property. Unless in Phase 2 we have the webhook force spec.generation to always be 0

With Kubernetes 1.11+ metadata.generation now increments properly when the status subresource is enabled on CRDs For more details see: knative/serving#643

* Drop webhook logic to increment spec.generation With Kubernetes 1.11+ metadata.generation now increments properly when the status subresource is enabled on CRDs For more details see: knative/serving#643 * Drop the generational duck type

mattmoor · 2019-02-14T21:08:13Z

0.4 pieces have landed, moving out.

Addresses knative#643 In 0.4, Revisions have both `/configurationMetadataGeneration` and `/configurationGeneration` labels have a value equal to the Configuration's metadata.generation This commit has the Configuration's reconciler use `/configurationGeneration` when looking up the latest created revision We should be able to drop the use of `/configurationMetadataGeneration` in 0.6

Addresses knative#643 In 0.4, Revisions had generation labels migrated toward use of `/configurationGeneration` label to determine a Configuration's latest created Revision. This commit leaves the deprecated `/configurationMetadataGeneration` label intact to allow rollback to 0.4, but removes the migration of the deprecated label, along with the deletion of an annotation previously used for the same purpose. Functional dependency on `/configurationMetadataGeneration` is dropped in knative#3325.

mattmoor · 2019-03-19T18:05:32Z

/milestone Serving 0.6
PRs for 0.5 scope are in 0.5 now, so moving this to 0.6 to track remaining work.

…#3325) Addresses #643 In 0.4, Revisions have both `/configurationMetadataGeneration` and `/configurationGeneration` labels have a value equal to the Configuration's metadata.generation This commit has the Configuration's reconciler use `/configurationGeneration` when looking up the latest created revision We should be able to drop the use of `/configurationMetadataGeneration` in 0.6

Addresses #643 In 0.4, Revisions had generation labels migrated toward use of `/configurationGeneration` label to determine a Configuration's latest created Revision. This commit leaves the deprecated `/configurationMetadataGeneration` label intact to allow rollback to 0.4, but removes the migration of the deprecated label, along with the deletion of an annotation previously used for the same purpose. Functional dependency on `/configurationMetadataGeneration` is dropped in #3325.

mattmoor · 2019-04-26T05:05:47Z

@dprotaso Do you plan to do the 0.6 portion of this?

dprotaso · 2019-04-29T20:21:24Z

@dprotaso Do you plan to do the 0.6 portion of this?

Oh yeah

dprotaso · 2019-05-06T20:19:51Z

/close

knative-prow-robot · 2019-05-06T20:19:55Z

@dprotaso: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dprotaso · 2019-05-06T20:20:44Z

it is done.

This reverts commit 2f61813.

This is not mandatory but it's been a while since Go was available in CI so it bumps golang to 1.15.

This reverts commit 2f61813.

mattmoor added kind/feature Well-understood/specified features, ready for coding. area/API API objects and controllers labels Apr 13, 2018

mattmoor added this to the M4 milestone Apr 13, 2018

google-prow-robot assigned grantr Apr 17, 2018

mattmoor modified the milestones: M4, M5 Apr 18, 2018

google-prow-robot assigned bsnchan and dprotaso May 24, 2018

mattmoor modified the milestones: M5, M6 May 24, 2018

mattmoor removed this from the M6 milestone Oct 31, 2018

mattmoor unassigned grantr Oct 31, 2018

mattmoor mentioned this issue Oct 31, 2018

Update knative CRD status with "UpdateStatus" instead of "Update" #2274

Closed

mattmoor modified the milestone: Serving 0.3 Nov 20, 2018

This was referenced Dec 19, 2018

Verify K8s 1.11 fixes our generation woes #642

Closed

Best practices for updating a CRD's spec #1128

Closed

dprotaso mentioned this issue Jan 2, 2019

Make use of the status subresource for our custom resources #2829

Merged

dprotaso added a commit to dprotaso/pkg that referenced this issue Jan 21, 2019

Drop webhook logic to increment spec.generation

99cc227

With Kubernetes 1.11+ metadata.generation now increments properly when the status subresource is enabled on CRDs For more details see: knative/serving#643

This was referenced Feb 12, 2019

Change the naming strategy of revisions and builds #3179

Merged

Reclaim the use of the revision label configurationGeneration #3204

Merged

dprotaso added a commit to dprotaso/pkg that referenced this issue Feb 14, 2019

Drop webhook logic to increment spec.generation

6c5cd53

With Kubernetes 1.11+ metadata.generation now increments properly when the status subresource is enabled on CRDs For more details see: knative/serving#643

mattmoor modified the milestones: Serving 0.4, Serving 0.5 Feb 14, 2019

joshrider mentioned this issue Feb 26, 2019

Drop functional dependency on 'configurationMetadataGeneration' label #3325

Merged

joshrider mentioned this issue Feb 27, 2019

Remove migration of deprecated generation labels and annotation #3335

Merged

knative-prow-robot modified the milestones: Serving 0.5, Serving 0.6 Mar 19, 2019

dprotaso mentioned this issue May 6, 2019

Drop the use of the configurationMetadataGeneration label #4012

Merged

knative-prow-robot closed this as completed May 6, 2019

nak3 added a commit to nak3/serving that referenced this issue Jan 25, 2021

Revert "Use golang 1.15 (knative#643)"

7dedfaf

This reverts commit 2f61813.

nak3 added a commit to nak3/serving that referenced this issue Jan 25, 2021

Use golang 1.15 (knative#643)

2f61813

This is not mandatory but it's been a while since Go was available in CI so it bumps golang to 1.15.

nak3 added a commit to nak3/serving that referenced this issue Jan 25, 2021

Revert "Use golang 1.15 (knative#643)"

c07a2d9

This reverts commit 2f61813.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Investigate the use of CRD sub-resources. #643

Investigate the use of CRD sub-resources. #643

mattmoor commented Apr 13, 2018

grantr commented Apr 13, 2018

mattmoor commented Apr 13, 2018

grantr commented Apr 13, 2018

mattmoor commented Apr 17, 2018

mattmoor commented Apr 18, 2018

mattmoor commented May 24, 2018

sslavic commented Jun 28, 2018

mattmoor commented Nov 6, 2018

dprotaso commented Jan 2, 2019 •

edited

Loading

mattmoor commented Feb 14, 2019

mattmoor commented Mar 19, 2019

mattmoor commented Apr 26, 2019

dprotaso commented Apr 29, 2019

dprotaso commented May 6, 2019

knative-prow-robot commented May 6, 2019

dprotaso commented May 6, 2019

Investigate the use of CRD sub-resources. #643

Investigate the use of CRD sub-resources. #643

Comments

mattmoor commented Apr 13, 2018

grantr commented Apr 13, 2018

mattmoor commented Apr 13, 2018

grantr commented Apr 13, 2018

mattmoor commented Apr 17, 2018

mattmoor commented Apr 18, 2018

mattmoor commented May 24, 2018

sslavic commented Jun 28, 2018

mattmoor commented Nov 6, 2018

dprotaso commented Jan 2, 2019 • edited Loading

Phase 1 - Serving 0.3

Changes

Implications

Phase 2 - Serving 0.4

Changes

Implications

Phase 3 - Serving 0.5

Changes

Phase 4 - Serving 0.6

Changes

Phase X - when conversion webhooks are beta

Changes

Implications

mattmoor commented Feb 14, 2019

mattmoor commented Mar 19, 2019

mattmoor commented Apr 26, 2019

dprotaso commented Apr 29, 2019

dprotaso commented May 6, 2019

knative-prow-robot commented May 6, 2019

dprotaso commented May 6, 2019

dprotaso commented Jan 2, 2019 •

edited

Loading