📖Add book build process #822

jichenjc · 2021-04-01T07:01:45Z

What this PR does / why we need it:

after this, we will have something like this

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

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
if necessary:
- includes documentation
- adds unit tests

/hold

k8s-ci-robot · 2021-04-01T07:01:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jichenjc

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~~ [jichenjc]

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

sbueringer · 2021-04-01T07:15:16Z

@jichenjc nice! I'll take a look when I've some more time. Am I right that this PR adds the source code and build for the book, but we independently have to find out how we get the domain and publish to it (as a follow-up)?

sbueringer · 2021-04-01T07:16:52Z

@jichenjc looks like you checked in hack/tools/bin. Can you please remove it again

jichenjc · 2021-04-01T07:20:53Z

@jichenjc nice! I'll take a look when I've some more time. Am I right that this PR adds the source code and build for the book, but we independently have to find out how we get the domain and publish to it (as a follow-up)?

yes, I don't know how to handle it now :) need more study

jichenjc · 2021-04-01T07:21:16Z

@jichenjc looks like you checked in hack/tools/bin. Can you please remove it again

some of them need to be remove, still need some polish ~

sbueringer · 2021-04-01T07:26:04Z

@jichenjc looks like you checked in hack/tools/bin. Can you please remove it again

some of them need to be remove, still need some polish ~

Okay no problem :). I think it shouldn't be necessary to check in any of the binaries, they are just build and used on-demand.

sbueringer · 2021-04-01T07:27:08Z

@jichenjc nice! I'll take a look when I've some more time. Am I right that this PR adds the source code and build for the book, but we independently have to find out how we get the domain and publish to it (as a follow-up)?

yes, I don't know how to handle it now :) need more study

Yup of course. I think it would be absolutely okay to just add the book in this PR and implement the publishing in a follow-up PR.

Take your time :) If I can help in any way, just let me know.

jichenjc · 2021-04-01T09:43:36Z

/test pull-cluster-api-provider-openstack-e2e-test

jichenjc · 2021-04-01T10:58:58Z

/test pull-cluster-api-provider-openstack-e2e-test

jichenjc · 2021-04-01T11:49:50Z

@sbueringer looks like our CI has some problem ? I didn't go detail but seems has problem in setting up the test env now ... I tried several times already

failed to solve with frontend dockerfile.v0: failed to solve with frontend gateway.v0: rpc error: code = Unknown desc = failed to build LLB: failed to load cache key: docker.io/library/golang:1.16.0 not found

versions.mk

jichenjc · 2021-04-02T01:15:59Z

@sbueringer polished a little bit (local test still can see the book is being built), please help take a look, thanks~

sbueringer

Great work, thank you!

Only a few nits

hack/tools/Makefile

go.mod

versions.mk

hack/tools/Makefile

docs/book/book.toml

docs/book/src/introduction.md

sbueringer · 2021-04-02T06:24:05Z

docs/book/src/clusteropenstack/configuration.md

@@ -0,0 +1,309 @@
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+**Table of Contents**  *generated with [DocToc](https://github.com/thlorenz/doctoc)*


Should we already drop the duplicates in ./docs/* ?
(we can also do it after the book is published, I just don't want to have to keep them in sync long-term)

sbueringer · 2021-04-02T06:27:48Z

docs/book/src/topics/mover.md

+
+# Install OpenStack Cluster API provider into target cluster
+
+You need install OpenStack cluster api providers into `target` cluster first.


I know this doc has been there before.

I would rather not duplicate the documentation of the main book.

the main book is about bootstrap cluster,
here is the workload cluster...

hack/tools/Makefile

jichenjc · 2021-04-07T02:16:26Z

@sbueringer thanks for the review
I want to create build process first then update doc contents accordingly and find how to publish them ..

jichenjc · 2021-04-08T01:13:17Z

@sbueringer thanks for the guide, I think it works for me now :)

can you help review the latest update?

sbueringer · 2021-04-08T06:51:17Z

@jichenjc Great work!

Overall lgtm. I think that's the last one I would like to see fixed before merging this PR: #822 (comment)

Please keep the other comments in mind for follow-up PRs when finalizing the book.

Just some additional feedback:
It's a lot easier to incrementally review PRs when changes are made by adding new commits, so I just have to review the new commits. It's a lot harder if the commits are always squashed and I have to figure out what the changes are :). A squash after the reviews are done / before merge would be best.

jichenjc · 2021-04-08T07:02:56Z

@jichenjc Great work!

Overall lgtm. I think that's the last one I would like to see fixed before merging this PR: #822 (comment)

Please keep the other comments in mind for follow-up PRs when finalizing the book.

Just some additional feedback:
It's a lot easier to incrementally review PRs when changes are made by adding new commits, so I just have to review the new commits. It's a lot harder if the commits are always squashed and I have to figure out what the changes are :). A squash after the reviews are done / before merge would be best.

I used to work on openstack and use gerrit to review which asked to avoid multiple commit becuase gerrit will help us in doing that ,so yes, the suggestion is helpful and I will follow (I think big enough PRs fit to that model)
learned a lot here :)

Signed-off-by: jichenjc <[email protected]>

sbueringer · 2021-04-08T07:10:52Z

@jichenjc Great work!
Overall lgtm. I think that's the last one I would like to see fixed before merging this PR: #822 (comment)
Please keep the other comments in mind for follow-up PRs when finalizing the book.
Just some additional feedback:
It's a lot easier to incrementally review PRs when changes are made by adding new commits, so I just have to review the new commits. It's a lot harder if the commits are always squashed and I have to figure out what the changes are :). A squash after the reviews are done / before merge would be best.

I used to work on openstack and use gerrit to review which asked to avoid multiple commit becuase gerrit will help us in doing that ,so yes, the suggestion is helpful and I will follow (I think big enough PRs fit to that model)
learned a lot here :)

Ah nice to know. I never got to use gerrit. But yeah, GIthub is not especially good at doing this. I'm usually reviewing in my IDE so unfortunately even when GitHub would be better at this, I would still have the same problem :)

P.S. Yup, absolutely makes only sense for bigger PRs, on smaller PRs it's still easy to have an overview over the whole diff.

sbueringer · 2021-04-08T07:11:53Z

/lgtm

Thx for implementing this. I think the book will lead to great documentation for CAPO. :)

jichenjc · 2021-04-08T09:08:22Z

/hold cancel

let's merge this and keep updating docs

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 1, 2021

k8s-ci-robot requested review from hidekazuna and sbueringer April 1, 2021 07:01

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 1, 2021

jichenjc changed the title ~~📖Add book build process~~ WIP: 📖Add book build process Apr 1, 2021

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2021

jichenjc force-pushed the add_book branch 2 times, most recently from 1b6a8eb to 1be5cc3 Compare April 1, 2021 09:02

sbueringer reviewed Apr 1, 2021

View reviewed changes

versions.mk Outdated Show resolved Hide resolved

jichenjc force-pushed the add_book branch 3 times, most recently from 377aa88 to c908ed1 Compare April 2, 2021 01:12

jichenjc changed the title ~~WIP: 📖Add book build process~~ 📖Add book build process Apr 2, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 2, 2021

jichenjc force-pushed the add_book branch 2 times, most recently from bc5489f to 0e54066 Compare April 2, 2021 01:28

sbueringer reviewed Apr 2, 2021

View reviewed changes

jichenjc force-pushed the add_book branch from 0e54066 to 5677ff9 Compare April 7, 2021 02:15

jichenjc force-pushed the add_book branch 2 times, most recently from e2834e2 to e415b75 Compare April 7, 2021 03:25

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 7, 2021

jichenjc force-pushed the add_book branch from e415b75 to 1252fa4 Compare April 8, 2021 01:06

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2021

build book

ab5423b

Signed-off-by: jichenjc <[email protected]>

jichenjc force-pushed the add_book branch from 1252fa4 to ab5423b Compare April 8, 2021 07:04

k8s-ci-robot assigned sbueringer Apr 8, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 8, 2021

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 8, 2021

k8s-ci-robot merged commit fa64ad4 into kubernetes-sigs:master Apr 8, 2021

jichenjc mentioned this pull request Apr 9, 2021

Cleanup docs changes #838

Merged

3 tasks

sbueringer mentioned this pull request Apr 9, 2021

Provide documentation website (like CAPA/CAPZ) #783

Closed


		# Install OpenStack Cluster API provider into target cluster

		You need install OpenStack cluster api providers into `target` cluster first.

📖Add book build process #822

📖Add book build process #822

Uh oh!

Conversation

jichenjc commented Apr 1, 2021

Uh oh!

k8s-ci-robot commented Apr 1, 2021

Uh oh!

sbueringer commented Apr 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Apr 1, 2021

Uh oh!

jichenjc commented Apr 1, 2021

Uh oh!

jichenjc commented Apr 1, 2021

Uh oh!

sbueringer commented Apr 1, 2021

Uh oh!

sbueringer commented Apr 1, 2021

Uh oh!

jichenjc commented Apr 1, 2021

Uh oh!

jichenjc commented Apr 1, 2021

Uh oh!

jichenjc commented Apr 1, 2021

Uh oh!

Uh oh!

jichenjc commented Apr 2, 2021

Uh oh!

sbueringer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbueringer Apr 2, 2021

Choose a reason for hiding this comment

Uh oh!

sbueringer Apr 2, 2021

Choose a reason for hiding this comment

Uh oh!

jichenjc Apr 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jichenjc commented Apr 7, 2021

Uh oh!

jichenjc commented Apr 8, 2021

Uh oh!

sbueringer commented Apr 8, 2021

Uh oh!

jichenjc commented Apr 8, 2021

Uh oh!

sbueringer commented Apr 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Apr 8, 2021

Uh oh!

jichenjc commented Apr 8, 2021

Uh oh!

Uh oh!

sbueringer commented Apr 1, 2021 •

edited

Loading

sbueringer commented Apr 8, 2021 •

edited

Loading