Skip to content
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

Custom Resource Documentation #4071

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Conversation

enisoc
Copy link
Member

@enisoc enisoc commented Jun 13, 2017

Docs for CustomResourceDefinition and the concept of custom resources in general.

Preview links:


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 13, 2017
@chenopis chenopis added this to the 1.7 milestone Jun 14, 2017
@chenopis
Copy link
Contributor

@enisoc Is this ready for review now?

@enisoc
Copy link
Member Author

enisoc commented Jun 21, 2017

@chenopis Sorry, not yet. I'm planning to push a ready-for-review commit by the end of today.

@chenopis
Copy link
Contributor

@enisoc No worries, just wasn't sure. Ping me when it's ready. Thx

@enisoc enisoc force-pushed the crd-1.7 branch 2 times, most recently from 0926785 to de1ab47 Compare June 22, 2017 23:51
@enisoc
Copy link
Member Author

enisoc commented Jun 22, 2017

This is now ready for review.

/assign @chenopis
/assign @deads2k

cc @erictune @foxish

@deads2k
Copy link
Contributor

deads2k commented Jun 23, 2017

nice doc. lgtm.

@chenopis
Copy link
Contributor

@abiogenesis-now Can you take a look at this one as well? Thx

Copy link
Contributor

@cody-clark cody-clark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small rewrites [RW] to adhere to the style guide

It can take up to 10 seconds for the TPR controller to notice that you deleted the TPR definition
and initiate the migration. TPR data will remain accessible during this time.

Once the migration completes, the resource will begin serving through the CRD.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] Once the migration completes, the resource begins serving through the CRD.
Eliminates future tense


After verifying the CRD data, restart any clients you stopped before the migration, such as
custom controllers.
These controllers will now access CRD data when they make requests on the same API endpoints
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete "will"
Eliminates future tense


For custom resources that need specialized implementations, you can write and deploy a standalone
API server, then make the resource available to clients of the main API server through aggregation.
The main API server will delegate requests to you for the custom resources that you handle.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] The main API server delegates requests to you...
avoids future tense

1. **Install the CustomResourceDefinition**

While the source TPR is still active, install the matching CRD with `kubectl create`.
Existing TPR data will remain accessible because TPRs take precedence over CRDs when both try
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] Existing TPR data remains accessible...
avoids future tense


1. **Stop all clients that use the TPR**

The API server will attempt to prevent mutation of TPR data for the resource while it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] The API server attempts to prevent TPR data for the resource from mutation while it...
avoids future tense and nominalization (easier to read)

Stopping clients, such as TPR-based custom controllers, helps to avoid inconsistencies in
the copied data.

In addition, clients that watch TPR data will not receive any more events once the migration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] In addition, clients that watch TPR data do not receive any more events once the migration begins.
avoids future tense


Normally, when you delete a TPR definition, the API server tries to clean up any objects stored
in that resource.
Since you created a matching CRD, the server will copy objects to the CRD instead of deleting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] After you create a matching CRD, CRD, the server copies objects to the CRD instead of deleting...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "After" makes it sound like creating the CRD triggers something to happen, which it doesn't. Deleting the TPR is the trigger, and at that point it checks whether the CRD exists. I've reworded to hopefully make this more clear.


1. **Verify the new CRD data**

It can take up to 10 seconds for the TPR controller to notice that you deleted the TPR definition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] It can take up to 10 seconds for the TPR controller to notice that you delete the TPR definition and to initiate the migration. The data remains accessible during this time.

Once the migration completes, the resource begins serving through the CRD.

avoids future tense and adds parallel construction


This endpoint URL can then be used to create and manage custom objects.
The `kind` of these objects will be `CronTab` from the spec of the
`CustomResourceDefinition` object we created above.
CustomResourceDefinition object we created above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace "we" with "you"

custom object of kind `CronTab`. The kind `CronTab` comes from the spec of the
`CustomResourceDefinition` object we created above.
CustomResourceDefinition object we created above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace "we" with "you"


This frees you from writing your own API server to handle the custom resource,
but the generic nature of the implementation means you have less flexibility than with an
[aggregated API server](#aggregated-api-servers).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#namingishard

would also like to be part of this meeting! @chenopis dunno who organizes this kind of stuff, but let me know if you need help.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lavalamp I'm game. @abiogenesis-now I'll loop you in when we setup a time.

Copy link
Contributor

@jesscodez jesscodez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a lot of comments, but I've marked the ones that are potentially out-of-scope for this PR (I've just started looking at contributing to K8s docs, hence "all the ideas" :P ). Feel free to let me know if you disagree with anything!

@@ -89,4 +89,6 @@ toc:
- docs/concepts/policy/security-context.md
- docs/concepts/policy/pod-security-policy.md


- title: Extensions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of organizing this content, might want to sync up with @chenopis regarding #4173 (comment) (Docs PR for API Aggregation)? e.g. it doesn't matter which PR implements it, but make sure you guys are aware in the case of merge conflicts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm creating a subsection in Concepts called Extending the Kubernetes API, so we should group this in that as well.

- title: Extending the Kubernetes API
  section:
  - docs/concepts/api-extension/apiserver-aggregation.md  

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woot sounds good!


Custom resources can appear and disappear in a running cluster through dynamic registration,
and cluster admins can update custom resources independently of the cluster itself.
Once it's installed, users can create and access objects in a custom resource with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW for clarity] Once a custom resource is installed, users can create and access its objects with kubectl, just as they do for built-in resources like pods.

certain kind.
For example, the built-in *pods* resource contains a collection of Pod objects.

A *custom resource* is an extension of the Kubernetes API that is not necessarily available on every
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] Second sentence: In other words, it represents a customization of a particular Kubernetes installation.

independently of the cluster's own lifecycle.
Custom controllers can work with any kind of resource, but they fit especially well with custom
resources.
For example, the [Operator](https://coreos.com/blog/introducing-operators.html) pattern is a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[RW] Custom controllers can work with any kind of resource, but they are especially effective when combined with custom resources. The Operator pattern is one example of such a combination. It allows developers to encode domain knowledge for specific applications into an extension of the Kubernetes API.


This frees you from writing your own API server to handle the custom resource,
but the generic nature of the implementation means you have less flexibility than with an
[aggregated API server](#aggregated-api-servers).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, might want to figure out what the standardized term for this is. In #4173, I saw "API Aggregation", and "Aggregation Layer", and "Aggregator"...not sure who makes these decisions (cc @lavalamp @cheftako), but would be super helpful to have the same terminology used throughout the site.

Some or all of these steps may be unnecessary if the custom controller handles the migration for
you.
* Be familiar with the concept of [custom resources](/docs/concepts/extensions/custom-resources/),
which were known as *third-party resources* until Kubernetes 1.7.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh boy, that's a bit confusing >< (just a comment about "third-party-resource", not asking for an edit here)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the confusion because we called the general concept "third-party resources" and the API object "ThirdPartyResource", and we are renaming both at the same time? We tried to make this better going forward by calling the API object CustomResourceDefinition so we could use the term "custom resource" to refer to the general concept, which is a thing you can achieve either with CRD or some other way. (see #45277)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be helpful to add an additional sentence to hammer home the point? Or is that overkill:

  • Be familiar with the concept of custom resources, which were known as third-party resources until Kubernetes 1.7.
  • Be familiar with CustomResourceDefinitions, which are the simplest way to make custom resources API-accessible.

* Be familiar with the concept of [custom resources](/docs/concepts/extensions/custom-resources/),
which were known as *third-party resources* until Kubernetes 1.7.
* Be familiar with [how to use CustomResourceDefinitions](/docs/tasks/access-kubernetes-api/extend-api-custom-resource-definitions/).
* **Before performing a migration on real data, conduct a dry run of these steps in a test cluster.**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead phrase this as "conduct a dry run by going through these steps in a test cluster"? It might be just me but for a second I thought "dry run" was a special command

1. **Stop all clients that use the TPR**

The API server will attempt to prevent mutation of TPR data for the resource while it
copies objects to the CR, but it can't guarantee consistency in all cases, such as with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/CR/CRD

1. **Restart clients**

After verifying the CRD data, restart any clients you stopped before the migration, such as
custom controllers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe not 100% necessary, but add "and watchers" ?

Check that all your objects were correctly copied:

```shell
kubectl get crontabs --all-namespaces -o yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this PR, but @chenopis, what do you think of eventually implementing custom Markdown like DigitalOcean has (https://www.digitalocean.com/community/tutorials/digitalocean-s-writing-guidelines#variables)? That way it's clearer that things like crontab are meant to be swapped out and are not part of the real command.

See:
screen shot 2017-06-23 at 3 43 10 pm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abiogenesis-now Yeah, that looks more clear. I'm not satisfied w/ our current flavor of markdown. Can you open an Issue and assign it to me? Maybe we can tackle this after moving the production site over to Netlify.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@chenopis
Copy link
Contributor

@enisoc FYI, all feedback must be addressed and LGTMs given by EOD Tue, June 27th so that this can be merged for the 1.7 release on June 28th.

@enisoc enisoc force-pushed the crd-1.7 branch 2 times, most recently from 6937b1f to 1fe0e88 Compare June 26, 2017 21:25
Copy link
Member Author

@enisoc enisoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've addressed most comments.

kubectl get ct -o yaml
```

You should see that it contains the custom `cronSpec` and `image` fields
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if you use kubectl 1.7+. Are you suggesting to show that output instead? It looks like this:

Name:           my-new-cron-object
Namespace:      default
Labels:         <none>
Annotations:    <none>
API Version:    stable.example.com/v1
Kind:           CronTab
Metadata:
  Cluster Name: 
  Creation Timestamp:   2017-06-26T20:23:26Z
  Generation:           0
  Resource Version:     427
  Self Link:            /apis/stable.example.com/v1/namespaces/default/crontabs/my-new-cron-object
  UID:                  4f18b46a-5aad-11e7-9f5e-a0481c9805b1
Spec:
  Cron Spec:    * * * * /5
  Image:        old-image
Events:         <none>

CustomResources (objects created in the schema defined by CustomResourceDefintions)
support finalizers. If you add a `metadata.finalizers` stanza like

Custom objects support finalizers. If you add a `metadata.finalizers` stanza like:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of this section is essentially a definition of what a finalizer is. I can't think of a good way to define it in one line, and I haven't found a good doc to link to. The one you found in the namespaces proposal appears to be a bit out of date, and refers only to the usage with namespaces, not the general concept of finalizers which can be on any object.

@@ -0,0 +1,163 @@
---
title: Migrate a ThirdPartyResource to CustomResourceDefinition
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's grammatically correcterer, but the second "a" feels weird to me... I think in my head I'm expanding my version of the title as, "Migrate a [single] ThirdPartyResource [object] to [use the] CustomResourceDefinition [API]."

If I'm the only one who thinks it feels weird, I'm fine going with your suggestion.

Some or all of these steps may be unnecessary if the custom controller handles the migration for
you.
* Be familiar with the concept of [custom resources](/docs/concepts/extensions/custom-resources/),
which were known as *third-party resources* until Kubernetes 1.7.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the confusion because we called the general concept "third-party resources" and the API object "ThirdPartyResource", and we are renaming both at the same time? We tried to make this better going forward by calling the API object CustomResourceDefinition so we could use the term "custom resource" to refer to the general concept, which is a thing you can achieve either with CRD or some other way. (see #45277)


Normally, when you delete a TPR definition, the API server tries to clean up any objects stored
in that resource.
Since you created a matching CRD, the server will copy objects to the CRD instead of deleting
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "After" makes it sound like creating the CRD triggers something to happen, which it doesn't. Deleting the TPR is the trigger, and at that point it checks whether the CRD exists. I've reworded to hopefully make this more clear.

@chenopis
Copy link
Contributor

@cody-clark @abiogenesis-now PTAL. Thx

@jesscodez
Copy link
Contributor

responded with some rewording suggestions, but these are optional (e.g. use them if you think they're useful). otherwise LGTM

@enisoc
Copy link
Member Author

enisoc commented Jun 26, 2017

Added some more edits for the parts about finalizers and prereqs for migration.

@chenopis
Copy link
Contributor

@enisoc Sorry, due to some other recent PR merges, there are now conflicts w/ the _data/concepts.yml and _data/tasks.yml files.

@cody-clark
Copy link
Contributor

@chenopis lgtm

@enisoc
Copy link
Member Author

enisoc commented Jun 27, 2017

Conflicts resolved.

@chenopis
Copy link
Contributor

:lgtm:


Review status: 0 of 10 files reviewed at latest revision, 30 unresolved discussions.


Comments from Reviewable

@chenopis chenopis merged commit 171a5ae into kubernetes:release-1.7 Jun 27, 2017
@enisoc enisoc deleted the crd-1.7 branch June 27, 2017 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants