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 docs and manifests #78

Merged
merged 6 commits into from
Sep 23, 2021
Merged

Auto-generate CRD docs and manifests #78

merged 6 commits into from
Sep 23, 2021

Conversation

ableuler
Copy link
Contributor

@ableuler ableuler commented Sep 20, 2021

This PR renders the helm chart into static manifests using a pre-commit hook and uses a github action to auto-generate a clickable markdown documentation of the custom resource. The manifests should probably be created in an action too instead of using the git hook...?

@ableuler ableuler requested review from olevski and a team as code owners September 20, 2021 08:34
@ableuler ableuler force-pushed the 000-crd-docs2 branch 3 times, most recently from 588ee67 to a47b098 Compare September 20, 2021 09:27
@ableuler ableuler changed the title Auto-generate CRD docs Auto-generate CRD docs and manifests Sep 20, 2021
@ableuler ableuler force-pushed the 000-crd-docs2 branch 2 times, most recently from 700f965 to af5201b Compare September 20, 2021 09:54
Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing...

@ableuler ableuler force-pushed the 000-crd-docs2 branch 19 times, most recently from 42d9399 to ebca42d Compare September 22, 2021 23:00
@ableuler
Copy link
Contributor Author

Ok, after some forth-and-back, learning a bit more about actions in general and also toying with kustomize, here's the overhauled version of this PR and what it does:

  • It adds a /manifests directory with all the rendered manifests and a very basic kustomization file which can serve as base for real-world kustomization.
  • It adds a /docs/crd.md file which is a clickable documentation of the custom resource definition
  • It adds a .github/workflows/generate-crd-docs.yaml workflow which is triggered after every push to main. It automatically builds the content of /manifests and /docs/crd.yaml and opens a PR if any of these are found to be outdated. The action also pushes an image tagged with both latest and the current git commit hash. This is to make sure that the latest image is really the latest image as we are referring to it in /manifests/deployment.yaml.

@ableuler
Copy link
Contributor Author

Note: To test I did some small edit to the crd.yaml and manually triggered the generate-crd-docs.yaml workflow on this PR branch (normally it would only run on main). This is the resulting run https://github.com/SwissDataScienceCenter/amalthea/runs/3684285085?check_suite_focus=true, which opened this PR as intended: #89

@ableuler ableuler requested a review from olevski September 23, 2021 07:31
@rokroskar
Copy link
Member

I'm coming to this a bit late, but is there a reason not to have actual docs for amalthea on readthedocs? Seems like that would be the natural place rather than in the repo itself. I understand you also want to have rendered manifests so you can easily apply them, but I guess the primary motivation is the docs?

Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ableuler I have one minor change request.

Everything else looks good.

Am I right in my understanding that whenever we merge or push something (i.e. new code) into the main branch this will trigger and submit a PR to update the manifests and crd docs?

What happens if there are no changes to the relevant files (i.e. templates and crd)? Do we still get a PR that has a blank diff?

@ableuler
Copy link
Contributor Author

What happens if there are no changes to the relevant files (i.e. templates and crd)? Do we still get a PR that has a blank diff?

No, in this case the PR will be omitted entirely.

@ableuler
Copy link
Contributor Author

ableuler commented Sep 23, 2021

I'm coming to this a bit late, but is there a reason not to have actual docs for amalthea on readthedocs? Seems like that would be the natural place rather than in the repo itself. I understand you also want to have rendered manifests so you can easily apply them, but I guess the primary motivation is the docs?

I originally came from the documentation aspect, but I think it's generally nice to publish the manifests too. I also want to have docs on read-the-docs and I actually hope that at some point when crdsdev/doc#156 is fixed that we can stop hosting the rendered CRD docs entirely and just point to https://doc.crds.dev/github.com/SwissDataScienceCenter/amalthea. But for the moment I think having the CRD docs inside the repo has a benefit.

@olevski olevski self-requested a review September 23, 2021 08:55
@ableuler
Copy link
Contributor Author

@rokroskar #92

@ableuler ableuler merged commit 7ed0795 into main Sep 23, 2021
@ableuler ableuler deleted the 000-crd-docs2 branch September 23, 2021 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants