Initial draft GitBook. #566

davidewatson · 2018-10-29T02:59:21Z

What this PR does / why we need it:

The Cluster API has seen a fair number of changes over the course of the past year. At the same time, there has been a significant increase in the number of providers. Clear developer documentation is needed in order to make implementation easier and maintain consistent behavior.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Begins to fix #505

Special notes for your reviewer:

I am going to push the generated code in a second commit for ease of review. It may also make sense for me to break this up further to both obtain consensus for this PR, and facilitate further writing.

To view this GitBook locally while we sort out which account will be used to serve the final product, you may install GitBook and then:

cd $GOPATH/src/sigs.k8s.io/cluster-api
git fetch origin pull/566/head:gitbook
git checkout gitbook
cd docs/book
gitbook install
gitbook build
gitbook serve

You may then view the book locally open http://localhost:4000...

Release note:

Initial developer documentation.

docs/book/SUMMARY.md

docs/book/common_code/architecture.md

docs/book/common_code/cluster_controller.md

msohn · 2018-10-29T15:34:52Z

docs/book/common_code/machineset_controller.md

+
+{% sample lang="go" %}
+```go
+// MachineTemplateSpec describes the data a machine should have when created from a template


s/a machine should have when created from a template/needed to create a Machine from a template/

Fixed with my latest commit.

docs/book/common_code/reponsitory_layout.md

docs/book/provider_implementations/building_running_and_testing.md

docs/book/provider_implementations/generate_crds.md

davidewatson · 2018-10-29T18:23:46Z

Remaining tasks which may or may not be dealt with within this PR: #571

@phyber, @randomvariable, @hardikdr, @oneilcin, @detiber, @chuckha, @kris-nova

detiber

A few comments and questions, but overall this is outstanding work!

docs/book/common_code/machine_controller.md

detiber · 2018-10-29T19:30:37Z

docs/book/common_code/machine_controller.md

+{% sample lang="go" %}
+```go
+// MachineStatus defines the observed state of Machine
+type MachineStatus struct {


Is there some tooling to sync these structs with the current versions? If not, we may want to track that as a followup item to avoid drift.

Agreed that duplicating this code is a Bad Idea (tm). I've found two GitBook plugins which support including code snippets: include-codeblock and snippet. Both of them assume Doxygen markers, which seems reasonable to me. If we are okay with adding those to the source I'll take a look.

Hopefully those play well with the code generation.... :)

include-codebook is more popular (29 vs 2 stars, 22 vs 2 forks) and has seen more recent development (though not by much).

As an example, with this code change:

diff --git a/pkg/apis/cluster/v1alpha1/machinedeployment_types.go b/pkg/apis/cluster/v1alpha1/machinedeployment_types.go index d3017c05d..e27c880da 100644 --- a/pkg/apis/cluster/v1alpha1/machinedeployment_types.go +++ b/pkg/apis/cluster/v1alpha1/machinedeployment_types.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/cluster-api/pkg/apis/cluster/common" ) +/// [marker0] // MachineDeploymentSpec defines the desired state of MachineDeployment type MachineDeploymentSpec struct { // Number of desired machines. Defaults to 1. @@ -67,6 +68,9 @@ type MachineDeploymentSpec struct { ProgressDeadlineSeconds *int32 `json:"progressDeadlineSteeconds,omitempty"` } +/// [marker0]

The coresponding markdown looks like this:

{% method %} ## MachineDeploymentSpec {% sample lang="go" %} [import:'marker0'](../../../pkg/apis/cluster/v1alpha1/machinedeployment_types.go) {% endmethod %}

So the resulting markdown is both less readable and more accurate...

detiber · 2018-10-29T19:33:13Z

docs/book/common_code/machine_controller.md

+`Cluster` in the same namespace as the `Machine`
+
+There are two consequences of this:
+ - There must be at exactly one `Cluster` per namespace.


I'd suggest rewording this to say "There must no be more than one Cluster per namespace". The current wording seems to suggest that namespaces without a cluster would also be problematic.

You are right. This is poorly worded.

For the machine actuator, I think we do require exactly one Cluster in the same namespace as the Machine:

cluster-api/pkg/controller/machine/controller.go

Line 216 in 2d88aef

func (c *ReconcileMachine) getCluster(machine *clusterv1.Machine) (*clusterv1.Cluster, error) {

#177 and #490 both discuss removing this restriction.

Maybe I should add informational panels to document these open issues.

detiber · 2018-10-29T19:40:12Z

docs/book/provider_implementations/building_running_and_testing.md

+ # Generate manifests e.g. CRD, RBAC etc.
+ manifests:
+-       go run vendor/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go all
+       go run vendor/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go crd


This probably isn't ideal. Ideally we'd be able to leverage more of the auto generation from kubebuilder. For the AWS implementation we disabled most of it for expedience as we were working on break-fix after the initial CRD migration.

Documenting the current state is a good idea though, because it points out these rough edges. I agree that we should fix. Let's file an issue (and it might even be a decent getting started issue).

docs/book/provider_implementations/generate_crds.md

roberthbailey · 2018-10-29T21:25:46Z

/cc @pwittrock

docs/book/common_code/architecture.md

roberthbailey · 2018-10-29T21:40:39Z

docs/book/common_code/architecture.md

+at any time with the Namespace and Name of an object (Resource instance), 
+and it will make the cluster state match the state declared in the object
+Spec. Upon completion, Reconcile updates the object Status to the new actual
+state.


If the Reconcile function fails to update the actual state, ...

docs/book/common_code/cluster_controller.md

docs/book/common_code/machine_controller.md

roberthbailey · 2018-10-29T23:17:26Z

docs/book/getting_started/existing_providers.md

+
+{% panel style="warning", title="Important" %}
+Does it make sense to identify a simple provider which may be easier to get 
+started with? Is that possible in a vendor neutral way?


Probably not possible in a vendor neutral way. I think the best bet is to point to the list above and tell people to use the one for the environment that they want to be running in.

roberthbailey · 2018-10-29T23:19:11Z

docs/book/provider_implementations/building_running_and_testing.md

+ # Generate manifests e.g. CRD, RBAC etc.
+ manifests:
+-       go run vendor/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go all
+       go run vendor/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go crd


Documenting the current state is a good idea though, because it points out these rough edges. I agree that we should fix. Let's file an issue (and it might even be a decent getting started issue).

docs/book/provider_implementations/building_running_and_testing.md

roberthbailey · 2018-10-29T23:20:19Z

docs/book/provider_implementations/building_running_and_testing.md

+
+```bash
+minikube start
+make deploy


Neat, I haven't actually tried this. Good to see that someone has.

roberthbailey · 2018-10-29T23:22:32Z

docs/book/provider_implementations/naming.md

+# Repository Naming
+
+The naming convention for new Cluster API [_provider_ repositories][repo-naming] 
+is generally of the form `cluster-api-provider-${cloud}`, where `${cloud}` is a,


instead of "cloud" I've been using the term "environment". A cloud is one type of environment, as vsphere or openstack or bare metal (of which there are probably lots of variants).

roberthbailey · 2018-10-29T23:23:39Z

Thanks for getting started on the docs. This looks great!

docs/book/common_code/architecture.md

phyber · 2018-10-30T11:17:39Z

This is looking great. 👍

scruplelesswizard · 2018-10-30T12:36:16Z

Awesome stuff @davidewatson! Thank you for picking this up

chuckha

This is awesome. Bunch of minor comments.

docs/book/common_code/cluster_controller.md

chuckha · 2018-10-30T14:12:56Z

docs/book/common_code/cluster_controller.md

+
+Cluster has 4 fields:
+
+`Spec` contains the desired cluster state specified by the object. While much


This could be a little clearer if the order here matched the order they appeared in the struct below. I think a personal preference is to put the struct first then the explanation after, but I think this ordering works as well

Good point, will fix.

Thinking more about this, I am not sure I agree. By placing the Spec and Status descriptions first aren't we are emphasizing the portion of the API which matters most to the implementor?

i can see it both ways, totally fine to keep it as is

docs/book/common_code/machine_controller.md

chuckha · 2018-10-30T14:20:21Z

docs/book/common_code/machine_controller.md

+```go
+// MachineSpec defines the desired state of Machine
+type MachineSpec struct {
+	// This ObjectMeta will autopopulate the Node created. Use this to


I opened a cluster-api issue since these have golint issues: #570

docs/book/provider_implementations/create_actuators.md

docs/book/provider_implementations/register_schemes.md

randomvariable · 2018-10-30T16:05:14Z

This looks pretty great.

For remaining tasks, we can track these in an "UMBRELLA: Gitbook" issue.

Just as a test, was able to get this to push to Github pages. Would require renaming the current docs directory to work.

davidewatson · 2018-10-31T01:20:22Z

@randomvariable: I like the features of firebase, and see some value in maintaining consistency with kubebuilder, but gh-pages are free and therefore more likely to be replicated by other kubebuilder projects. Maybe we can discuss briefly tomorrow and make a decision. ~~I'll also create an umbrella issue for the major items which are missing.~~ A rihanna isssue: #571 Thanks!

davidewatson · 2018-10-31T20:11:39Z

I have updated the deployment instructions. Once this is merged someone with admin privileges will need to:

Create the gh-pages branch:

git subtree push --prefix docs/book/_book origin gh-pages

Enable gh-pages from the Setting pane:

/assign @roberthbailey

roberthbailey · 2018-10-31T22:32:06Z

I'm happy to approve once someone lgtm's this who has reviewed it somewhat carefully (I took a quick pass on the docs but from the call today it sounded like other folks had done a much better job reviewing them than I have).

docs/book/_book/.firebase/hosting.cHVibGlj.cache

detiber · 2018-11-01T15:32:28Z

/test pull-cluster-api-test

msohn · 2018-11-02T00:22:46Z

docs/book/_book/GLOSSARY.html

+        <meta name="description" content="">
+        <meta name="generator" content="GitBook 3.2.3">
+        <meta name="author" content="SIG Lifecycle Team">
+


Can we get rid of the excessive number of empty lines in these html files ?

Anything under docs/book/_book/ is rendered so we would have to fix GitBook if we wanted to make their removal permanent...

It is strange. Maybe there is a stray new line (or two!) somewhere. I'll look into it.

Are you aware of this discussion on gitbook's future as an open source project:
GitbookIO/gitbook#1808
GitbookIO/gitbook#2148

it seems the new version does not support self-hosting anymore
https://docs.gitbook.com/v2-changes/important-differences

It may be we that we have to or should migrate to netlify in the future. @timothysc suggested this last week since it is what k8s uses.

If we do it might also be nice to migrate kubebuilder as well. I am unsure if there is interest on that front but I think there is value in leveraging kubebuilder to the extent possible since this will make it easier for others to create high quality CRDs. @pwittrock

For now I'd like to get this merged so that we can parallize and iterate. @detiber, @roberthbailey, thoughts?

I see no reason to block this now, but it'll definitely be worth tracking it for later followup.

+1 for merging, I didn't intend to block this, content is more important than the tool

k8s documentation is written in markdown, processed using Hugo and served using netlify
https://kubernetes.io/docs/contribute/start/#the-basics-about-our-docs

Thanks for the link!

…uded.

are documented.

copying it.

roberthbailey · 2018-11-03T05:53:39Z

/assign @detiber

detiber · 2018-11-05T14:50:56Z

/lgtm

roberthbailey · 2018-11-05T21:21:08Z

Let's get this in and we can iterate from there.

/approve

k8s-ci-robot · 2018-11-05T21:21:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidewatson, roberthbailey

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [roberthbailey]

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

roberthbailey · 2018-11-05T21:28:54Z

/test pull-cluster-api-test

roberthbailey · 2018-11-06T05:44:33Z

I tried to follow the instructions above to push to a branch to serve on gh pages but it didn't work:

$ git subtree push --prefix docs/book/_book upstream gh-pages
git: 'subtree' is not a git command. See 'git --help'.

Thoughts?

msohn · 2018-11-06T17:05:30Z

Usually git subtree comes in the ./share/git-core/contrib/ folder of a git installation.
On Mac it just works for me using git 2.19.1:

$ git subtree
usage: git subtree add   --prefix=<prefix> <commit>
   or: git subtree add   --prefix=<prefix> <repository> <ref>
   or: git subtree merge --prefix=<prefix> <commit>
   or: git subtree pull  --prefix=<prefix> <repository> <ref>
   or: git subtree push  --prefix=<prefix> <repository> <ref>
   or: git subtree split --prefix=<prefix> <commit...>

    -h, --help            show the help
    -q                    quiet
    -d                    show debug messages
    -P, --prefix ...      the name of the subdir to split out
    -m, --message ...     use the given message as the commit message for the merge commit

options for 'split'
    --annotate ...        add a prefix to commit message of new commits
    -b, --branch ...      create a new branch from the split subtree
    --ignore-joins        ignore prior --rejoin commits
    --onto ...            try connecting new tree to an existing one
    --rejoin              merge the new branch back into HEAD

options for 'add', 'merge', and 'pull'
    --squash              merge subtree changes as a single commit

davidewatson · 2018-11-06T19:37:40Z

My guess is either thegit-subtree command is not installed or it is not in your $PATH (see https://git-scm.com/docs/git#git---list-cmdsgroupgroup82308203). On MacOS, when git-subtree is installed using brew, I find it here:

rodin:~ dwat$ which git-subtree
/usr/local/bin/git-subtree
rodin:~ dwat$

The instructions I found for installing git-subtree on StackOverflow say to download git and compile from source if it is not available as a package for your distro. You can also install only git-subtree using these instructions.

roberthbailey · 2018-11-09T12:33:06Z

@davidewatson - I gave you write access to the repo; can you run the git subtree commands and get the branch pushed? I have a google-installed version of git and don't feel like breaking it right now to get the subtree command working via brew. :)

…const use govmomi types.VirtualEthernetCardMacType for NIC address type

k8s-ci-robot requested review from karan and krousey October 29, 2018 02:59

k8s-ci-robot added 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 Oct 29, 2018

msohn suggested changes Oct 29, 2018

View reviewed changes

detiber reviewed Oct 29, 2018

View reviewed changes

k8s-ci-robot requested a review from pwittrock October 29, 2018 21:25

roberthbailey reviewed Oct 29, 2018

View reviewed changes

msohn suggested changes Oct 29, 2018

View reviewed changes

docs/book/common_code/architecture.md Outdated Show resolved Hide resolved

roberthbailey mentioned this pull request Oct 30, 2018

Documentation: Basic internals docs #542

Closed

davidewatson mentioned this pull request Oct 30, 2018

Document how providers should deploy shared CRDs #567

Closed

phyber mentioned this pull request Oct 30, 2018

Add README to ./cloud/aws kubernetes-sigs/cluster-api-provider-aws#125

Closed

chuckha reviewed Oct 30, 2018

View reviewed changes

k8s-ci-robot assigned roberthbailey Oct 31, 2018

detiber reviewed Nov 1, 2018

View reviewed changes

docs/book/_book/.firebase/hosting.cHVibGlj.cache Outdated Show resolved Hide resolved

msohn reviewed Nov 2, 2018

View reviewed changes

davidewatson added 4 commits November 2, 2018 11:01

Initial draft GitBook.

fa99b24

Spelling and language corrections.

487fc51

Additional clarification.

7b92990

Correct factual errors and other PR feedback.

3357114

davidewatson added 13 commits November 2, 2018 11:01

Use the word environment instead of cloud when bare metal may be incl…

228d197

…uded.

Link to finalizer documentation.

47300f8

Document ProviderStatus and ensure both Cluster and Machine resources

9a924e5

are documented.

Use standard logging instead of glog.

ef1fc5e

Panic if actuators can not be created.

1d451b8

Spelling correction.

8853127

Fix boilerplate header to match.

4b423fb

Use the GitBook include-codeblock plugin to quote code instead of

cb80ea9

copying it.

Include MachineDeployment skeleton.

7370e72

Fix the Dockerfile boilerplate whitespace.

b4e3e2b

Fix golang code indentation.

04ea73c

Use include-codeblock for Machine Actuator.

a41625b

Document gh-pages deployments and remove firebase instructions.

fb33cf8

k8s-ci-robot assigned detiber Nov 3, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 5, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 5, 2018

k8s-ci-robot merged commit df337dc into kubernetes-sigs:master Nov 5, 2018

roberthbailey mentioned this pull request Nov 7, 2018

Upgrade lodash dependency #576

Closed

davidewatson deleted the gitbook branch December 21, 2018 16:57

jayunit100 pushed a commit to jayunit100/cluster-api that referenced this pull request Jan 31, 2020

Merge pull request kubernetes-sigs#566 from andrewsykim/address-type-…

01066f9

…const use govmomi types.VirtualEthernetCardMacType for NIC address type


		Cluster has 4 fields:

		`Spec` contains the desired cluster state specified by the object. While much

Initial draft GitBook. #566

Initial draft GitBook. #566

Conversation

davidewatson commented Oct 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidewatson commented Oct 29, 2018 • edited Loading

detiber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidewatson Oct 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberthbailey commented Oct 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberthbailey commented Oct 29, 2018

phyber commented Oct 30, 2018

scruplelesswizard commented Oct 30, 2018

chuckha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randomvariable commented Oct 30, 2018

davidewatson commented Oct 31, 2018 • edited Loading

davidewatson commented Oct 31, 2018

roberthbailey commented Oct 31, 2018

detiber commented Nov 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidewatson Nov 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberthbailey commented Nov 3, 2018

detiber commented Nov 5, 2018

roberthbailey commented Nov 5, 2018

k8s-ci-robot commented Nov 5, 2018

roberthbailey commented Nov 5, 2018

roberthbailey commented Nov 6, 2018

msohn commented Nov 6, 2018

davidewatson commented Nov 6, 2018

roberthbailey commented Nov 9, 2018

davidewatson commented Oct 29, 2018 •

edited

Loading

davidewatson commented Oct 29, 2018 •

edited

Loading

davidewatson Oct 29, 2018 •

edited

Loading

davidewatson commented Oct 31, 2018 •

edited

Loading

davidewatson Nov 2, 2018 •

edited

Loading