doc: clean up README and user-guide #661

hasbro17 · 2018-10-26T00:16:03Z

Description of the change:
README:

Change pre-alpha to alpha
Updated the workflow
Linked to ansible operator user-guide

user-guide:

Cleaned up some sections and added links to controller-runtime go-docs in the user-guide

estroz

A few cosmetic nits and one addition.

estroz · 2018-10-26T00:35:21Z

README.md


-The project is currently pre-alpha and it is expected that breaking changes to the API will be made in the upcoming releases.
+The project is currently alpha. The core APIs provided by the controller-runtime will most likely be unchanged. New features and APIs may still be added in the future.


Probably shouldn't mention controller-runtime before defining what it is below, or link it up here too.

Can we add a quick note what alpha means? There were questions about this on slack. See #608

There is talk about adding api breaking changes to the controller-runtime see just an FYI

@estroz I'm going to link it up here.

@shawn-hurley Thanks for pointing that out. That's why I used the phrase "most likely" but I'll try to make it clearer that there could still be breaking changes. Just hopefully nothing on the scale of what we're doing now. Most likely just function signature changes.

@lilic Alpha and beta are usually defined in the context of testing and missing features(and I might be wrong) but here's my understanding:

Alpha: Some features may still be missing(e.g metrics, csv generation) and incompatible changes may still be made.

Beta: All the features have been added and no backward incompatible changes will be made. Only changes expected are bug fixes.

I'll add a note on that.

estroz · 2018-10-26T00:39:09Z

README.md


-To learn more about the operator-sdk, see the [user guide][guide].
+The SDK also supports developing an operator using Ansible. See the [Ansible operator user guide][ansible_user_guide].


I would add a note about this in the Workflow section as well. Perhaps @shawn-hurley or another Ansible-focused team member could write a workflow in this or a separate PR.

I think we will add a follow up PR just to keep it easy

estroz · 2018-10-26T00:41:08Z

README.md

 - High level APIs and abstractions to write the operational logic more intuitively
 - Tools for scaffolding and code generation to bootstrap a new project fast
 - Extensions to cover common operator use cases

 ## Workflow

-The SDK provides the following workflow to develop a new operator:
+The SDK provides the following workflow to develop a new operator in Go:


Suggested change

The SDK provides the following workflow to develop a new operator in Go:

The SDK provides workflows to develop operators in Go or Ansible.

The following workflow is for a new Go operator:

estroz · 2018-10-26T00:42:29Z

doc/user-guide.md

-For this example replace the generated controller file `pkg/controller/memcached/memcached_controller.go` with the example [memcached_controller.go][memcached_controller] implementation.
+This will scaffold a new Controller implementation under `pkg/controller/memcached/...`.
+
+For this example replace the generated Controller file `pkg/controller/memcached/memcached_controller.go` with the example [memcached_controller.go][memcached_controller] implementation.


Suggested change

For this example replace the generated Controller file `pkg/controller/memcached/memcached_controller.go` with the example [memcached_controller.go][memcached_controller] implementation.

For this example replace the generated Controller file `pkg/controller/memcached/memcached_controller.go` with the example [`memcached_controller.go`][memcached_controller] implementation.

estroz · 2018-10-26T00:43:18Z

doc/user-guide.md

@@ -135,7 +139,7 @@ err := c.Watch(&source.Kind{Type: &appsv1.Deployment{}}, &handler.EnqueueRequest

 ### Reconcile loop

-Every controller has a Reconciler object with a `Reconcile()` method that implements the reconcile loop. The reconcile loop is passed the `Request` argument which is a Namespace/Name key used to lookup the primary resource object, Memcached, from the cache:
+Every Controller has a Reconciler object with a `Reconcile()` method that implements the reconcile loop. The reconcile loop is passed the [Request][request-go-doc] argument which is a Namespace/Name key used to lookup the primary resource object, Memcached, from the cache:


Suggested change

Every Controller has a Reconciler object with a `Reconcile()` method that implements the reconcile loop. The reconcile loop is passed the [Request][request-go-doc] argument which is a Namespace/Name key used to lookup the primary resource object, Memcached, from the cache:

Every Controller has a Reconciler object with a `Reconcile()` method that implements the reconcile loop. The reconcile loop is passed the [`Request`][request-go-doc] argument which is a Namespace/Name key used to lookup the primary resource object, Memcached, from the cache:

estroz · 2018-10-26T00:43:37Z

doc/user-guide.md

@@ -146,7 +150,8 @@ func (r *ReconcileMemcached) Reconcile(request reconcile.Request) (reconcile.Res
 }  
 ```

-Based on the return value of `Reconcile()` the reconcile `Request` may be requeued and the loop may be triggered again:
+Based on the return values, [Result][result_go_doc] and error, the `Request` may be requeued and the reconcile loop may be triggered again:


Suggested change

Based on the return values, [Result][result_go_doc] and error, the `Request` may be requeued and the reconcile loop may be triggered again:

Based on the return values, [`Result`][result_go_doc] and error, the `Request` may be requeued and the reconcile loop may be triggered again:

lilic

lgtm overall

lilic · 2018-10-26T08:55:01Z

README.md


-The project is currently pre-alpha and it is expected that breaking changes to the API will be made in the upcoming releases.
+The project is currently alpha. The core APIs provided by the controller-runtime will most likely be unchanged. New features and APIs may still be added in the future.


Can we add a quick note what alpha means? There were questions about this on slack. See #608

lilic · 2018-10-26T08:56:28Z

README.md

- [kubectl][kubectl_tool] version v1.10.0+.
- Access to a kubernetes v.1.10.0+ cluster.
+- [kubectl][kubectl_tool] version v1.11.0+.
+- Access to a kubernetes v.1.11.0+ cluster.


How come it doesn't work on v.1.10.0 clusters anymore?

It should work on v1.10, but we don't test v1.10 anymore, only v1.11 (technically openshift v3.11)

lilic · 2018-10-26T08:57:29Z

doc/user-guide.md

@@ -61,7 +61,7 @@ By default this will be the namespace that the operator is running in. To watch
 mgr, err := manager.New(cfg, manager.Options{Namespace: ""})
 ```

-**// TODO:** Doc on manager options(Sync period, leader election, registering 3rd party types)
+**// TODO:** Doc on leader election


Nit: I would prefer opening an issue for documentation on leader election rather than having it in the README.

I would also like that we start publishing our API's on godoc and link to our docs there.

IIRC @mhrivnak added package level docs that were pretty extensive and I think that might be the correct way to document usage of packages. Thoughts?

@shawn-hurley Agreed all of our APIs should have sufficient godocs. The leader election package is already quite well documented but the ToDo I have here is for an example on how to use our leader election pkg in the manager. And also an example on how to use the runtime's leader election pkg for comparison.

So either a separate doc that goes over both of those and links to the releavant godocs. Or we add the example to the godoc.
Either way I've created an issue for that #665

doc/user-guide.md

shawn-hurley

Just some thoughts and suggestions. LGTM though

shawn-hurley · 2018-10-26T19:13:33Z

README.md


-The project is currently pre-alpha and it is expected that breaking changes to the API will be made in the upcoming releases.
+The project is currently alpha. The core APIs provided by the controller-runtime will most likely be unchanged. New features and APIs may still be added in the future.


There is talk about adding api breaking changes to the controller-runtime see just an FYI

shawn-hurley · 2018-10-26T19:16:51Z

README.md


-To learn more about the operator-sdk, see the [user guide][guide].
+The SDK also supports developing an operator using Ansible. See the [Ansible operator user guide][ansible_user_guide].


I think we will add a follow up PR just to keep it easy

shawn-hurley · 2018-10-26T19:18:58Z

doc/user-guide.md

@@ -61,7 +61,7 @@ By default this will be the namespace that the operator is running in. To watch
 mgr, err := manager.New(cfg, manager.Options{Namespace: ""})
 ```

-**// TODO:** Doc on manager options(Sync period, leader election, registering 3rd party types)
+**// TODO:** Doc on leader election


I would also like that we start publishing our API's on godoc and link to our docs there.

IIRC @mhrivnak added package level docs that were pretty extensive and I think that might be the correct way to document usage of packages. Thoughts?

hasbro17 · 2018-10-28T22:15:16Z

Addressed the above comments. PTAL.

lilic

lgtm

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 26, 2018

hasbro17 requested review from lilic and estroz October 26, 2018 00:16

doc: clean up README and user-guide

036411b

hasbro17 force-pushed the update-readme branch from 260164d to 036411b Compare October 26, 2018 00:30

hasbro17 requested a review from AlexNPavel October 26, 2018 00:30

estroz suggested changes Oct 26, 2018

View reviewed changes

lilic suggested changes Oct 26, 2018

View reviewed changes

shawn-hurley approved these changes Oct 26, 2018

View reviewed changes

doc: clarify alpha, fix typos

69d4a11

lilic approved these changes Oct 29, 2018

View reviewed changes

estroz approved these changes Oct 29, 2018

View reviewed changes

hasbro17 merged commit a984491 into operator-framework:master Oct 29, 2018

hasbro17 deleted the update-readme branch October 29, 2018 18:44


		The project is currently pre-alpha and it is expected that breaking changes to the API will be made in the upcoming releases.
		The project is currently alpha. The core APIs provided by the controller-runtime will most likely be unchanged. New features and APIs may still be added in the future.


		To learn more about the operator-sdk, see the [user guide][guide].
		The SDK also supports developing an operator using Ansible. See the [Ansible operator user guide][ansible_user_guide].

-The SDK provides the following workflow to develop a new operator in Go:
+The SDK provides workflows to develop operators in Go or Ansible.
+The following workflow is for a new Go operator:

	For this example replace the generated Controller file `pkg/controller/memcached/memcached_controller.go` with the example [memcached_controller.go][memcached_controller] implementation.
	For this example replace the generated Controller file `pkg/controller/memcached/memcached_controller.go` with the example [`memcached_controller.go`][memcached_controller] implementation.

	Every Controller has a Reconciler object with a `Reconcile()` method that implements the reconcile loop. The reconcile loop is passed the [Request][request-go-doc] argument which is a Namespace/Name key used to lookup the primary resource object, Memcached, from the cache:
	Every Controller has a Reconciler object with a `Reconcile()` method that implements the reconcile loop. The reconcile loop is passed the [`Request`][request-go-doc] argument which is a Namespace/Name key used to lookup the primary resource object, Memcached, from the cache:

	Based on the return values, [Result][result_go_doc] and error, the `Request` may be requeued and the reconcile loop may be triggered again:
	Based on the return values, [`Result`][result_go_doc] and error, the `Request` may be requeued and the reconcile loop may be triggered again:

doc: clean up README and user-guide #661

doc: clean up README and user-guide #661

Uh oh!

Conversation

hasbro17 commented Oct 26, 2018

Uh oh!

estroz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shawn-hurley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hasbro17 commented Oct 28, 2018

Uh oh!

lilic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!