-
Notifications
You must be signed in to change notification settings - Fork 137
[WIP] Add versioning to the docs #869
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
Conversation
the previous /images/ does not work when the content is versioned
otherwise, it did not appear because its in the same header_topic element that was getting hidden
in case release doesn't have v or has extra info after patch
❌ Deploy Preview for gateway-api-inference-extension failed. Why did it fail? →
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: artberger The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @artberger! |
Unfortunately I don't have access to Netlify here. I think there's a small set of admins that control Netlify for all Kubernetes projects. Maybe #github-management Slack channel would have an answer. In the past, @mrbobbytables has helped us get Netlify setup. Just to clarify, is the problem that these changes can't be configured in netlify.toml alone and instead require higher level changes? |
@robscott thanks for the information. I was not sure how the GIE project's Netlify worked, and so I was sharing how I got it working for my forked repo and the GIE Netlify project I set up in my personal Netlify account. Update: I updated the |
to match settings in Netlify
.gitignore
Outdated
@@ -30,3 +30,6 @@ go.work.sum | |||
|
|||
# generated docs | |||
site | |||
|
|||
# virtual environment for mkdocs | |||
.venv |
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.
nit: Add new line
netlify.toml
Outdated
# Specify that we want to build from the docs branch | ||
[context.production] | ||
branch = "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.
nit: New blank line
Makefile
Outdated
|
||
.PHONY: live-docs | ||
live-docs: | ||
docker build -t gaie/mkdocs hack/mkdocs/image | ||
docker run --rm -it -p 3000:3000 -v ${PWD}:/docs gaie/mkdocs | ||
docker run --rm -it -p 3000:3000 -v ${PWD}:/docs gaie/mkdocs mike serve -a 0.0.0.0:3000 --branch 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.
This change should not be required since it's implemented in entrypoint.sh
. Can you remove and test?
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.
This target fails when using your branch:
make live-docs
docker build -t gaie/mkdocs hack/mkdocs/image
[+] Building 0.5s (11/11) FINISHED
=> [internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 886B 0.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [internal] load metadata for docker.io/library/python:3.13-alpine 0.4s
=> [internal] load build context 0.0s
=> => transferring context: 1.55kB 0.0s
=> [1/6] FROM docker.io/library/python:3.13-alpine@sha256:a94caf6aab428e086bc398beaf64a6b7a0fad4589573462f52362fd760e64cc9 0.0s
=> CACHED [2/6] RUN apk add --no-cache git 0.0s
=> CACHED [3/6] COPY requirements.txt /requirements.txt 0.0s
=> CACHED [4/6] RUN pip install -r /requirements.txt 0.0s
=> CACHED [5/6] WORKDIR /docs 0.0s
=> CACHED [6/6] COPY entrypoint.sh / 0.0s
=> exporting to image 0.0s
=> => exporting layers 0.0s
=> => writing image sha256:3d3d10a2a4a520efc0ef0fc01e1aabb39b5911762e0226b637b2f80f56ce66b7 0.0s
=> => naming to docker.io/gaie/mkdocs 0.0s
Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them
docker run --rm -it -p 3000:3000 -v /Users/solo-system-dhansen/go/src/sigs.k8s.io/gateway-api-inference-extension:/docs gaie/mkdocs mike serve -a 0.0.0.0:3000 --branch docs
Config value 'plugins': Plugin 'macros' option 'j2_line_comment_prefix': Unrecognised configuration name: j2_line_comment_prefix
Starting server at http://0.0.0.0:3000/
Press Ctrl+C to quit.
172.17.0.1 - - [02/Jun/2025 18:01:58] code 404, message File not found. Did you forget to run `mike set-default`? Alternately, you can navigate to 36fe7b09f30a:3000/[some-version] to see your docs
172.17.0.1 - - [02/Jun/2025 18:01:58] "GET / HTTP/1.1" 404 -
Note that I see WARNING - Config value 'plugins': Plugin 'macros' option 'j2_line_comment_prefix': Unrecognised configuration name: j2_line_comment_prefix
using the main branch so that is not the root case.
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 removed the mike serve -a 0.0.0.0:3000 --branch docs
line, and was able to run the target.
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.
For the error you saw:
code 404, message File not found. Did you forget to run
mike set-default
? Alternately, you can navigate to 36fe7b09f30a:3000/[some-version] to see your docs
The entrypoint.sh
now sets a default version. I think the duplicate mike serve
command that I removed was throwing it off and since you didn't have a default set locally, it was throwing that error.
Makefile
Outdated
crd-ref-docs: ## Install crd-ref-docs if not already installed | ||
@which crd-ref-docs >/dev/null 2>&1 || { \ | ||
echo "Installing crd-ref-docs..."; \ | ||
GOBIN=$(LOCALBIN) go install github.com/elastic/crd-ref-docs@latest; \ | ||
} |
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 get the following error when calling this target (MacOS):
Installing crd-ref-docs...
chmod +x ./hack/mkdocs/make-docs.sh
...
bash: line 1: crd-ref-docs: command not found
make[1]: *** [api-ref-docs] Error 127
make: *** [docs] Error 2
Can you take a look at how the other dependencies are installed and follow the same pattern? Make sure to use a tagged release instead of latest
.
After resolving ^ issue, I now hit the following issue:
$ make docs
chmod +x ./hack/mkdocs/make-docs.sh
...
Error: Could not find any releases.
This version will be deployed, but not marked as 'latest'.
./hack/mkdocs/make-docs.sh: line 113: mike: command not found
make: *** [docs] Error 127
To avoid these local deps issues, why not run ./hack/mkdocs/make-docs.sh
in the Docker container instead of locally?
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 for pointing out the crd-ref-docs issue. I removed these from the Makefile and instead put it in the make-docs.sh similar to how the Gateway API project does it here: https://github.com/kubernetes-sigs/gateway-api/blob/main/hack/mkdocs/generate.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.
To avoid these local deps issues, why not run ./hack/mkdocs/make-docs.sh in the Docker container instead of locally?
So I might be misunderstanding this, but the Docker container I thought was just to make the local preview of the docs. If we run the make-docs script in the container, I think it would build and push the docs each time someone ran the container for a local preview, which might not be what we wanted.
exit 0; | ||
fi | ||
|
||
mkdocs serve --dev-addr=0.0.0.0:3000 --livereload |
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.
Does mike serve
support --livereload
by default?
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.
no, unfortunately I got this error and didn't see it in the --help
or project :(
mike: error: unrecognized arguments: --live-reload
# docs since it installs mkdocs and all the right dependencies. | ||
# | ||
# On Ubuntu, this requires the python3-venv package. | ||
virtualenv: .venv |
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 checkout a release-0.4
branch, call this target and run make docs
in the venv:
$ make docs
...
Deploying docs for version 0.4
unknown flag: --json
Usage: gh release list [flags]
Flags:
--exclude-drafts Exclude draft releases
--exclude-pre-releases Exclude pre-releases
-L, --limit int Maximum number of items to fetch (default 30)
Error: Could not find any releases.
This version will be deployed, but not marked as 'latest'.
...
error: error writing commit:
fatal: Can't create a branch from itself: docs
fast-import: dumping crash report to .git/fast_import_crash_11185
make: *** [docs] Error 1
See #869 (comment) for an example that does not use gh
.
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 updated the script not to use gh
per your other comment.
Regarding the errors:
could not find any release
- if this is from your fork, it might not have any releases to grab the latest version from, so it won't mark as latest, but it should still continue to generate it as a release
Can't create a branch from itself: docs
- So how
mike
works is that it puts the versioned directories on adocs
branch. - But if the branch does not exist, then it will get this error, which if it's your fork, I'm assuming it doesn't have that branch yet.
- I put the steps as a prereq in the PR description, as we'd also have to make this branch in the GIE repo: [WIP] Add versioning to the docs #869 (comment)
similar to https://github.com/kubernetes-sigs/gateway-api/blob/main/hack/mkdocs/generate.sh also to address peer review feedback about dependency install
was tagging as latest if equal or greater than the release marked latest
duplicate from the entrypoint.sh for the docker image
@@ -2,7 +2,12 @@ site_name: Kubernetes Gateway API Inference Extension | |||
repo_url: https://github.com/kubernetes-sigs/gateway-api-inference-extension | |||
repo_name: kubernetes-sigs/gateway-api-inference-extension | |||
site_dir: site |
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.
@artberger based on this CI error, Netlify is still expecting the site
directory.
docs_dir: site-src | ||
extra: | ||
version: | ||
provider: mike |
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.
@artberger does mike
also need to be added to the plugins
list?
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.
@danehans it can be if we want to change any of the default mike
configuration values: https://github.com/jimporter/mike?tab=readme-ov-file#configuration
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Is there anyone we need to chase down to unblock this? I saw a comment saying we are blocked somewhere |
@kfswain @artberger see #422 (comment). TLDR is I believe a CI job may be needed to implement docs versioning. @artberger I can create a |
@kfswain for what is blocking this PR, the Netlify CI checks are failing. I believe this is to be expected, because the PR changes this netlify.toml file. The change is to make Netlify build the content from a However, I haven't gotten a response about who would know to confirm the Netlify changes and whether we can change the checks for the new approach vs the existing one (Slack post). @danehans I will look more into your approach of using a GitHub Action later today to see whether I can revise this PR to use that approach. From my initial review, I couldn't tell where |
My approach intends to use the main or release branch instead of a separate branch for the versioned docs. I'm not against using your |
Addresses #422
Summary of changes
Updates the doc publishing process to add versioning capability. The docs are versioned using a MkDocs plug-in called
mike
(GitHub project here).Right now, I have the versioned docs working locally based off my fork in Netlify here: https://adb-inf-extension.netlify.app/main/
Overview of the new process:
make-docs.sh
script is added to runmike
to build versioned docs based on the branch, such asmain
or a release branch likerelease-0.1
. This script is turned into amake
target, which is also called as part of the release process.mike
builds the HTML files for the version based on the content in that branch.docs
branch.docs
branch.The versioned process is described in a Docs Contribution guide that is added in this PR. This way, people can see how the versioning process works, and manually update it if needed (such as to delete an old version). See preview: https://adb-inf-extension.netlify.app/main/contributing/docs/#version-the-docs
Order of steps to update the docs publishing process
Step 1: Before merging this PR, create the
docs
branch.You have to do a one-time creation of a clean
docs
branch so that themike
tool can put the versioned docs on that branch.Locally, I used the following commands to do that:
Step 2: Merge this PR
One caveat is, the Netlify builds will fail on this PR unless we change Netlify first (see the Netlify section). I think it would keep the currently published content on the GIE URL until it has new content to replace it with, but I am not sure.
Step 3: Copy the netlify.toml file to the docs branch
We might have to copy this to the docs branch, I am not sure how the Netlify is set up in this repo. By putting it on the docs branch, the publishing branch has the Netlify settings so we wouldn't have to update the Netlify settings in the UI (see section later).
Step 4: Build versioned docs to the docs branch.
To build the versioned docs, we would check out each branch (main, the release branches) and run
make docs
.Netlify considerations
I am not sure who or how the GIE Netlify is currently set up.
In my Netlify account, I have the following settings. These are reflected in the
netlify.toml
that we would need to add to thedocs
branch.This way, Netlify builds based on changes to the
docs
branch. You don't need a specific build command because the HTML files are already built to thedocs
branch through themike
versioning.Build & Deploy > Build settings
Build & Deploy > Branches and deploy contexts
Fixes #422