Skip to content

Preview changes to controller documentation#15373

Closed
sftim wants to merge 1 commit intokubernetes:masterfrom
sftim:20190711_preview_controller_changes
Closed

Preview changes to controller documentation#15373
sftim wants to merge 1 commit intokubernetes:masterfrom
sftim:20190711_preview_controller_changes

Conversation

@sftim
Copy link
Copy Markdown
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
Copy Markdown

netlify bot commented Jul 11, 2019 •
edited
Loading

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

Built with commit aa61856

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

This was referenced Jul 11, 2019
Closed
Open
@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
@sftim sftim mentioned this pull request Jul 26, 2019
Closed
kbhawkey
kbhawkey reviewed Jul 30, 2019
View reviewed changes
content/en/docs/concepts/architecture/controller.md

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

Copy link
Copy Markdown
Contributor

@kbhawkey kbhawkey Jul 30, 2019 •
edited
Loading

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
Copy Markdown
Contributor

k8s-ci-robot commented Aug 7, 2019

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

Details 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

@sftim sftim mentioned this pull request Aug 8, 2019
Merged
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Aug 17, 2019

@sftim: PR needs rebase.

Details

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
@sftim sftim mentioned this pull request Aug 20, 2019
Closed
@zacharysarah
Copy link
Copy Markdown
Contributor

zacharysarah commented Oct 10, 2019

@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
Copy Markdown
Contributor

k8s-ci-robot commented Oct 10, 2019

@zacharysarah: Closed this PR.

Details

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

Reviewers

@ahmetb ahmetb Awaiting requested review from ahmetb

@bprashanth bprashanth Awaiting requested review from bprashanth

1 more reviewer

@kbhawkey kbhawkey kbhawkey left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sftim @k8s-ci-robot @zacharysarah @kbhawkey