-
Notifications
You must be signed in to change notification settings - Fork 813
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
Add Agones Kubernetes API docs generator #645
Conversation
Build Failed 😱 Build Id: 0d57e618-eea9-4b9a-a7d2-955b0ccb8f15 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Issue with resulting file position in Cloud Build:
Locally I have for
|
562a2bb
to
bd24ae0
Compare
Build Failed 😱 Build Id: 7d2b8038-01f1-489c-9e06-38b1c8f6bb5b To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
bd24ae0
to
790ac8d
Compare
Build Failed 😱 Build Id: ec4f0130-15f3-407e-8536-f52a56a023ac To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
790ac8d
to
b226b37
Compare
Found an issue, I have added the special kubebuilder declaration in
|
Build Succeeded 👏 Build Id: 96fa6cbf-bb93-4d46-8ea3-4b4bd8bbb781 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably reference the new page on https://agones.dev/site/docs/guides/access-api/
Otherwise, this looks amazing! Had some questions below about how to manage the bleeding edge.
build/includes/website.mk
Outdated
@@ -24,6 +24,23 @@ | |||
# Website targets | |||
# | |||
|
|||
REL_PATH := content/en/docs/Reference/agones_crd_api_reference.html | |||
API_DOC_PATH := "$(mount_path)/site/$(REL_PATH)" | |||
GEN_API_DOCS ?= docker run -e FILE=$(API_DOC_PATH) --rm -i $(common_mounts) $(build_tag) bash -c "/go/src/agones.dev/agones/site/gen-api-docs.sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious as to the motivation to put all of this in variables? Seems like it wouldn't change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added those options to be able to split test html result and normal one. GEN_API_DOCS used twice, that's why it was extracted.
@@ -59,6 +76,7 @@ site-deploy-preview: site-static-preview | |||
|
|||
site-test: | |||
# generate actual html and run test against - provides a more accurate tests | |||
$(MAKE) test-gen-api-docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a good answer for this - but because the website works off master - if we change our CRD data structures, that will change the website as well, and be out of sync until next release.
Only idea I've got - that in the advent of that happening - rename the old page and put an expiryDate
of the next release in the toml.... but how would we get a publishDate in the new one, and still pass tests?
I'm just thinking out loud here, but any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to use custom short code in front-matter and it seems that variables are not supported there.
So currently I am thinking of combining one full generated html with expiryVersion
shortcode. And new one with publishVersion
shortcode in the same file.
And during test just compare the second half with publishVersion
. And during release move bottom half to the top and generate new one at the bottom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds fancy - but makes sense to me! I assume you can make that work? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated gen-api-docs.sh
script. It now handles base_version
environment variable.
What it does is get all lines before publishVersion
line. Then extract what in the middle of this feature shortcode. Compares with what was generated if diff then do version compare.
Then set expiryVersion
of old part with new base_version
fresh generated doc now have new publishVersion
.
b226b37
to
16607a2
Compare
Build Failed 😱 Build Id: 75d62778-eb37-40d8-a9da-f1efd4fcafc9 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Reworked generation script it now handles the version in |
4fa7575
to
9e6b85a
Compare
Build Failed 😱 Build Id: 507ee387-19d4-4874-a52d-d1751d1e54cb To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: dd261584-206a-4d53-a568-3d419ea693c3 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
9e6b85a
to
2996acc
Compare
Build Succeeded 👏 Build Id: ff3a87dc-93b5-465d-8773-c8f900f38e91 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
2996acc
to
f711ddb
Compare
Build Failed 😱 Build Id: ca835708-a565-4674-a317-627f3f0af47c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
f711ddb
to
69a97ad
Compare
Build Succeeded 👏 Build Id: 615bafc8-a477-4be4-9411-534826f41cfe The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
I think I need to add md instructions on how to use new make target |
363d979
to
87798f8
Compare
Build Succeeded 👏 Build Id: 50f9b339-45ab-4cf6-9ea2-0c84f8b0f855 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: f83d51da-1af9-497d-8958-b857fd814143 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
87798f8
to
1d683f7
Compare
Fixed one issue, there was a commit-id in a default template. So I removed it, so now different git commit would generate the same html. |
Build Succeeded 👏 Build Id: dca396f0-df09-43bd-b952-a267c69edc80 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
build/includes/website.mk
Outdated
@@ -24,6 +24,22 @@ | |||
# Website targets | |||
# | |||
|
|||
REL_PATH := content/en/docs/Reference/agones_crd_api_reference.html | |||
GEN_API_DOCS ?= docker run -e FILE="$(mount_path)/site/$(REL_PATH)" -e VERSION=${base_version} --rm -i $(common_mounts) $(build_tag) bash -c "/go/src/agones.dev/agones/site/gen-api-docs.sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super minor nit on this - should this be := rather than ?= , since I don't think anyone should be overwriting the GEN_API_DOCS command?
which can be accessed and manipulated through the Kubernetes API. | ||
which can be accessed and manipulated through the Kubernetes API. | ||
|
||
{{% feature publishVersion="0.9.0" %}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the CRDs have changed - could we publish this all now? (0.8.0?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated publishVersion, test still pass.
Apologies - looks like I never submitted those comments :/ they've been sitting there forever. But otherwise, this is good to go. |
1d683f7
to
2b233fb
Compare
Merged master, fixed publishVersion and Makefile parameter. |
Build Failed 😱 Build Id: 22e8da75-dd1b-49d1-b804-ec69259ed6d7 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
2b233fb
to
8e3b89f
Compare
Build Succeeded 👏 Build Id: fc9feedc-adba-4e18-b303-ce1eaff06f66 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Add gen-crd-api-reference-docs utility into build-image, generate Agones CRD reference html document, map this doc to existent docs structure.
8e3b89f
to
4a1b4a9
Compare
Build Succeeded 👏 Build Id: f9a616c7-3582-465a-832e-fb93fa661b61 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! 👍
@@ -123,6 +123,11 @@ RUN mkdir /tmp/hugo && \ | |||
RUN curl -sL https://deb.nodesource.com/setup_11.x | bash - && \ | |||
apt-get install -y nodejs | |||
|
|||
# install API reference docs generator | |||
RUN mkdir -p /go/src/github.com/ahmetb && \ | |||
cd /go/src/github.com/ahmetb && git clone https://github.com/ahmetb/gen-crd-api-reference-docs && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should probably pin to a commit here.
Given there's a copy of the template in this repo, I don't want to accidentally break the downstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ahmetb for the review. I was thinking about that, will provide an update in a separate PR.
Add gen-crd-api-reference-docs utility into build-image, generate Agones CRD reference html document, map this doc to existent docs structure.
Covers #409.