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

Auto-generate CRD Documentation #1342 #1727

Merged
merged 15 commits into from
Feb 22, 2023
Merged

Conversation

Kartik-Garg
Copy link
Contributor

@Kartik-Garg Kartik-Garg commented Nov 9, 2022

Change Overview

Created CRD documentation using https://github.com/ahmetb/gen-crd-api-reference-docs/ , in response to the issue #1342 , added template folder, which contains files, which are used by the auto CRD generator tool, to create the CRD documentation, the documentation is named as "API.md" .

Signed-off-by: Kartik-Garg kartikgarg938@gmail.com

Plan of action :

  1. Cloned git repo : https://github.com/ahmetb/gen-crd-api-reference-docs/
  2. Created an executable binary by running go build command
  3. Copied template folder from this repto to Kanister repo
  4. Ran the following command:
    /path/to/gen-crd-api-reference-docs \ -config "/path/to/example-config.json" \ -api-dir "github.com/knative/build/pkg/apis/build/v1alpha1" \ -out-file docs.html
  5. Added above created binary to template folder.
  6. Modified docs target in makefile

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@Kartik-Garg
Copy link
Contributor Author

When the command make docs is ran:
running DOCS_CMD in the containerized build environment Removing everything under '_build'... Scanning... Validating... The HTML pages are in _build/html. /usr/local/lib/python3.6/site-packages/sphinx_rtd_theme/search.html:20: RemovedInSphinx30Warning: To modify script_files in the theme is deprecated. Please insert a <script> tag directly in your theme instead. {{ super() }} make[1]: Entering directory '/home/kartik/Documents/OpenSource/Kanister/kanister' running CMD in the containerized build environment I1108 08:43:15.790940 1 main.go:132] parsing go packages in directory ./pkg/apis/cr/v1alpha1 I1108 08:43:33.314445 1 main.go:234] using package=github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1 W1108 08:43:33.319322 1 main.go:445] not found external link source for type interface{} W1108 08:43:33.320886 1 main.go:445] not found external link source for type interface{} I1108 08:43:33.322448 1 main.go:170] written to API.md

A snippet of API.md :
Screenshot from 2022-11-08 14-52-53

@Kartik-Garg Kartik-Garg changed the title Internal pr Auto-generate CRD Documentation #1342 Nov 9, 2022
@PrasadG193
Copy link
Contributor

@Kartik-Garg let's not commit gen-crd-api-reference-docs binary to the repo.
We can add this binary to the build container that we use to build project. You can find the dockerfile here - https://github.com/kanisterio/kanister/blob/master/docker/build/Dockerfile

@PrasadG193
Copy link
Contributor

We use different image for docs build - https://github.com/kanisterio/kanister/tree/master/docker/docs-build. Sorry for the confusion. We should add the binary in this build image

@PrasadG193
Copy link
Contributor

First we need to decide when do we want to run this comment. And if this needs to be included in the public facing docs. Depending on that we can decide on the right docker image to add this binary.

@Kartik-Garg
Copy link
Contributor Author

We use different image for docs build - https://github.com/kanisterio/kanister/tree/master/docker/docs-build. Sorry for the confusion. We should add the binary in this build image

Since the crd_documentation tool does not have the latest release, we can not add it to the docs-build, because we will also have to run go build which will require additional packages to be installed, we already have a docker file which contains the go packages (the one which was referred earlier in the comment section).
I have added the additional command in the build docker file

@Kartik-Garg
Copy link
Contributor Author

First we need to decide when do we want to run this comment. And if this needs to be included in the public facing docs. Depending on that we can decide on the right docker image to add this binary.

makes sense, so I have created a new target called crd_docs , so when make crd_docs is run the API.md is created then.

@Kartik-Garg
Copy link
Contributor Author

@PrasadG193
I have added the additional changes:

  1. Removed the executable binary from template folder, instead downloading the tar.gz asset and building binary in the docker file itself (https://github.com/Kartik-Garg/kanister/blob/1a55c417be803518c50c2df03cb96beb2dad1616/docker/build/Dockerfile#L26)
  2. Added new target in the makefile which can be run explicitly whenever we want to create API.md file which contains documentation for API_CRD files : https://github.com/Kartik-Garg/kanister/blob/1a55c417be803518c50c2df03cb96beb2dad1616/Makefile#L211

Test Plan (manual testing) :

  1. first I created a custom image locally which has similar code to build dockerfile : https://github.com/kanisterio/kanister/blob/master/docker/build/Dockerfile , in addition added code to create executable binary file which looks something like :
    Screenshot from 2022-11-11 15-12-01

  2. after pushing this image as: kartikgarg/crd:0.1 , created a new image variable in makefile :
    Screenshot from 2022-11-11 15-11-17

  3. Added the new target and command to run and test with the docker image:
    Screenshot from 2022-11-11 15-11-38

  4. Result is successful :
    crd_docs_output

  5. And API.md file is created:
    Screenshot from 2022-11-11 15-32-57

@PrasadG193
Copy link
Contributor

Thanks a lot @Kartik-Garg for adding the Makefile target to build the docs.
We should move these docs to some directory. @pavannd1 @viveksinghggits I think the internal docs like API and Design docs should be in separate /docs folder and the website docs can be moved to /website dir.

@PrasadG193
Copy link
Contributor

@pavannd1 does it make sense to add API docs section in docs.kanister.io and move this doc there? Something like - https://argocd-operator.readthedocs.io/en/latest/reference/api.html/

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
docker/build/Dockerfile Outdated Show resolved Hide resolved
API.md Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
template/pkg.tpl Outdated Show resolved Hide resolved
docker/build/Dockerfile Outdated Show resolved Hide resolved
docker/build/Dockerfile Outdated Show resolved Hide resolved
Kartik-Garg and others added 14 commits February 22, 2023 17:44
Added the command in make docs now so when make docs command is ran, the API CRD documentation is also created automatically with "API.md" name, created a folder called template which contains all the necessary files (template files, place-holder go file, json file and executable binary file)
which are used/needed in the creating of the API CRD documentation

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Added the command in make docs now so when make docs command is ran, the API CRD documentation is also created automatically with "API.md" name, created a folder called template which contains all the necessary files (template files, place-holder go file, json file and executable binary file)
which are used/needed in the creating of the API CRD documentation

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Co-authored-by: kale-amruta <41624751+kale-amruta@users.noreply.github.com>
Added tool to create CRD documentation to the docker build file, so we dont have to explicitly add exec binary to our project

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
As per the suggestion from the PR comment, removed executable binary from this folder, also removed .json file which was not required

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Added a new target for creating on crd documentaion called as crd_docs and also added new command for it which is run inside the container image build

Signed-off-by: Kartik-Garg <kartikgarg938@gmail.com>
Removed unwanted space

Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
Improved the spacing as per the suggestion.

Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
Removed the template folder, updated API.md file, Made alignment changes

Signed-off-by: Kartik-Garg <kartik.garg@infracloud.io>
Modified space in makefile

Signed-off-by: Kartik-Garg <kartik.garg@infracloud.io>
Added new required files for the API CRD documentation tool and made changes in the docker build and the makefile command to run the target to generate the documentation.
Changed config file name.

Signed-off-by: Kartik-Garg <kartik.garg@infracloud.io>
@viveksinghggits
Copy link
Contributor

@Kartik-Garg
just to confirm this the documentation that is generated will be updated after every release right? Because we would be running make docs after every release.
Once new documentation is generated how would that we checked into the repo again, or thats not requried? Can you explain the workflow please?

@Kartik-Garg
Copy link
Contributor Author

@Kartik-Garg just to confirm this the documentation that is generated will be updated after every release right? Because we would be running make docs after every release. Once new documentation is generated how would that we checked into the repo again, or thats not requried? Can you explain the workflow please?

@viveksinghggits The documentation would be generated every time we run the make crd_docs command. It would just create the documentation as API.md file. Once, the documentation is created and we need to update the existing one, we can just commit it and push it to repo, since the command creates documentation locally

@viveksinghggits
Copy link
Contributor

viveksinghggits commented Feb 22, 2023

@Kartik-Garg
does that mean every time someone changes an API definition they will have to run this command and commit and push the documentation as well?

@Kartik-Garg
Copy link
Contributor Author

Kartik-Garg commented Feb 22, 2023

does that mean every time someone changes an API definition they will have to run this command and commit and push the documentation as well?

Yes

@viveksinghggits
Copy link
Contributor

does that mean every time someone changes an API definition they will have to run this command and commit and push the documentation as well?

Yes

Let's add this workflow here so that people are aware https://github.com/kanisterio/kanister/blob/master/CONTRIBUTING.md#contributing-code

@viveksinghggits
Copy link
Contributor

Just another sub heading maybe, stating what to do when you update api types. I am assuming it's just the api types that we are concerned about here, right?

@viveksinghggits
Copy link
Contributor

I would rather prefer adding a subheading under to https://github.com/kanisterio/kanister/blob/master/CONTRIBUTING.md#contributing-code.

### Updating the API types

If your changes involve the Kanister API types as well, make sure you generate the API documentation using below command and then push the updated `API.md` as well.

Update wording and grammar please 😄 .

Added steps to create the documentation when changes are made in Kanister API Types as well.

Signed-off-by: Kartik-Garg <kartik.garg@infracloud.io>
@Kartik-Garg
Copy link
Contributor Author

I would rather prefer adding a subheading under to https://github.com/kanisterio/kanister/blob/master/CONTRIBUTING.md#contributing-code.

### Updating the API types

If your changes involve the Kanister API types as well, make sure you generate the API documentation using below command and then push the updated `API.md` as well.

Update wording and grammar please smile .

Thanks a lot! @viveksinghggits
I have made changes accordingly, please do let me know if everything seems alright

@viveksinghggits
Copy link
Contributor

Looks good. Thanks @Kartik-Garg 🚀

@mergify mergify bot merged commit f448a96 into kanisterio:master Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-generate CRD Documentation
3 participants