Repository navigation
Improve docs-builder dependency management - #49482
Conversation
|
Managed to trigger the sort of "Required" check failure I am looking for in the Documentation build over at https://github.com/cilium/cilium/actions/runs/39339590734/job/118357530268?pr=49482#step:6:325 . That one is a bit cryptic as it fails with a message like "can't find the docs-builder image", but I can get this to fail more clearly by moving the docs-builder dependency check to the start of the Documentation checks. Will fix that up. |
2288273 to
ab19f4b
Compare
ab19f4b to
b642b57
Compare
28c8171 to
b2f98e4
Compare
|
/test |
1 similar comment
This comment has been minimized.
This comment has been minimized.
b2f98e4 to
98b1afc
Compare
|
/test |
This comment has been minimized.
This comment has been minimized.
aanm
left a comment
There was a problem hiding this comment.
I'm applying my changes from #49469 on top of yours.
--- a/.github/renovate.json5
+++ b/.github/renovate.json5
@@ -837,3 +837,3 @@
"fileFilters": [
- "Documentation/requirements.txt"
+ "images/docs-builder/requirements.txt"
],--- a/.gitattributes
+++ b/.gitattributes
@@ -14 +14 @@
-Documentation/requirements.txt linguist-generated
+images/docs-builder/requirements.txt linguist-generated--- a/netlify.toml
+++ b/netlify.toml
@@ -4 +4 @@
- command = "make html-netlify"
+ command = "pip install -r ../images/docs-builder/requirements.txt && make html-netlify"0203f6c to
79e3264
Compare
This comment has been minimized.
This comment has been minimized.
qmonnet
left a comment
There was a problem hiding this comment.
Thanks a lot for this! Please find some comments inline below.
The docs-builder image build workflows have created some pain during maintenance due to the way that the files have been managed, including: * docs-builder image tag was calculated from all Documentation/ content. This means that _any_ change to Documentation files would cause the builder to need to be rebuilt. * Documentation/Makefile contains the docs-builder image tag, meaning that any time we update the docs-builder tag, it updates the input used to calculate the docs-builder tag. As soon as you make one change to any docs file dependency, the docs-builder update GitHub workflows would always trigger the deployment to rebuild the docs-builder image, even if all changes to the docs-builder have been accounted for. * Due to these discrepancies, there has been no way to detect whether the docs-builder needs updating, so the regular Documentation build did not depend on building a docs-builder image with the latest dependencies. This lead to cases where we would review and merge docs dependency updates without actually building the docs using the new dependencies. Move all of the build infrastructure for creating the docs-builder images under the images/ directory. By reducing the set of input files for calculating the builder sha to only the minimal dependencies under images/docs-builder directory, we can update the docs-builder only when the dependencies update, rather than any time there is any change in the documentation. Co-authored-by: André Martins <andre@cilium.io> Signed-off-by: Joe Stringer <joestringernz@gmail.com>
Mount the original host copy of the git repository into the docker containers when generating the requirements.txt so that a full build can be performed from a git worktree. Signed-off-by: Joe Stringer <joe@cilium.io>
79e3264 to
f578c3a
Compare
This new make target will check the docker tag for the docs-builder image based on dependencies in the images/docs-builder/ directory, and update the tag in all relevant locations if the tag would differ. This target is only intended to create the diff so that there is a hard dependency on the new builder image tag value, but it will not update the $DOCS_BUILDER_IMG variable to the new fully specified image. Add this new target to the 'builder-image' target so that any time the builder image is updated, the files in the tree are also updated. This should help to signal when the docs-builder references need to be updated. Signed-off-by: Joe Stringer <joe@cilium.io>
To ensure that the documentation directory is built against the latest copy of the docs-builder dependencies, add a check which updates the docs-builder image tag based on the docs-builder dependencies in the git tree, and then fail out with a message requiring the user to update the docs-builder image if the local tree is out of sync. This should ensure that renovate docs-builder updates are tested before we merge the changes, by virtue of being run from the standard GitHub workflow for documentation checks. Signed-off-by: Joe Stringer <joe@cilium.io>
Bump these to versions that have been released relatively recently. Readthedocs lists Python 3.14 as a supported build dependency, so pick that over more recent releases that are not yet listed on their site. Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Joe Stringer <joe@cilium.io>
There were references here to stale paths, but the paragraphs provided little value in documenting so it's better to remove stale / under-maintained docs than to update references in place and leave the remaining partially incorrect documentation. Signed-off-by: Joe Stringer <joe@cilium.io>
f578c3a to
92952a7
Compare
1 similar comment
This comment has been minimized.
This comment has been minimized.
qmonnet
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback! Alright, let's go for it 🙂

We've had some problems for a while where Renovate updates can break the docs build. Renovate
updates docs dependencies, but the actual documentation build workflow doesn't have a hard
requirement to build against the latest docs-builder images. This meant that we can end up merging
in dependency updates that break the build, but we don't notice until after these changes are merged.
This series updates the docs build to avoid this pattern of renovate breaking the tree:
dependencies derived from
Documentation/requirements-min/requirements.txtupdated to refer to the new builder image tag.
build if the workflow attempts to build against a stale docs-builder base.
This should reduce the overall churn on the docs-builder image and bring its update management in
line with the behavior of updating other docker images in the tree.
We can ignore the docs-builder image build run in this PR; I've manually updated the docs-builder image in this PR. The workflow itself won't work, because GitHub runs the default branch's copy of the workflow, but this PR changes the filepaths necessary to compile that image. I expect the docs-builder workflow will work correctly once we merge this PR, but I can also keep an eye on it to promptly address any breakage in that workflow in
mainfollowing merge.