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

Preview changes to controller documentation #15373

Closed

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Jul 11, 2019

This is related to issue #4135 about a concept-level guide to controllers.

This PR isn't here to be merged. Instead, I'm pushing it up as a way to let people see the set of changes I've got on mind all in one place.

Based on the work I've done here, I plan to carve off smaller chunks that are easier to review, and PR those in turn.

For feedback:

  • if you have a view on how to tackle Need top-level Controllers concept guide #4135 overall, I recommend commenting there.
  • if there is a particular pull request covering part of these changes, I'll reference this PR from the smaller ones, and I recommend giving feedback there
  • otherwise, or if it just feels like the right fit, comment on this draft PR.

/hold not-for-merging

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 11, 2019
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jul 11, 2019
@netlify
Copy link

netlify bot commented Jul 11, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit aa61856

https://deploy-preview-15373--kubernetes-io-master-staging.netlify.com

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2019

In applications of robotics and automation, a _control loop_ is
a non-terminating loop that regulates the state of the system.

Copy link
Contributor

@kbhawkey kbhawkey Jul 30, 2019

Choose a reason for hiding this comment

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

hello @sftim .
This is a good start to the Controller overview. I think this page is fairly important. This page could be treated as a separate PR to kick off the other controller additions/changes. Get tech review for this page.
Here are a few ideas/questions I have about What is a Controller?:

  • How does the first sentence tie in to the k8s controller concept (the second sentence)?
  • What is a built-in controller? How does a built-in controller differ from a custom controller?
  • Are the controllers executors as well as state maintainers?
  • What happens if a controller fails? Is the controller restarted?
  • The Controller pattern mentions well-implemented, but does not say how to implement one
  • Add a What's next section to tie in additional controller resources, such as kubebuilder?
  • Clients of controllers?

@sftim sftim force-pushed the 20190711_preview_controller_changes branch from e80d783 to aa61856 Compare August 7, 2019 22:26
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign makoscafee
You can assign the PR to them by writing /assign @makoscafee in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot
Copy link
Contributor

@sftim: PR needs rebase.

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.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2019
@zacharysarah
Copy link
Contributor

@sftim I'd love to see this move forward. Please feel free to /reopen this PR when you're ready!

/close

@k8s-ci-robot
Copy link
Contributor

@zacharysarah: Closed this PR.

In response to this:

@sftim I'd love to see this move forward. Please feel free to /reopen this PR when you're ready!

/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.

@sftim sftim deleted the 20190711_preview_controller_changes branch June 9, 2021 17:47
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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. language/en Issues or PRs related to English language needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants