capi: generate validate target help from build names - #2149
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
cncf-cla: yes
|
Welcome @yindia! |
kubernetes-prow
Bot
added
the
needs-ok-to-test
|
Hi @yindia. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
kubernetes-prow
Bot
added
the
size/L
|
/ok-to-test |
kubernetes-prow
Bot
added
ok-to-test
mboersma
left a comment
mboersma
left a comment
There was a problem hiding this comment.
Could you also update custom-build-targets.md to match this change? It still tells contributors to add manual validate help stubs, including one in the example. Please explain that per-image validate help is generated automatically and note that new provider families must be added to ALL_VALIDATE_TARGETS.
Update custom-build-targets.md to match the generated per-image validate help. Per-image validate stubs are no longer hand-maintained; only new provider families need adding to ALL_VALIDATE_TARGETS.
|
/test pull-ova-all |
|
@mboersma ready for review |
drew-viles
left a comment
drew-viles
left a comment
There was a problem hiding this comment.
Thanks, the docs update covers what @mboersma asked for and I confirmed make help now lists every per-image validate target. Two small things inline, plus one note: removing the stubs also removes the 16 validate-node-ova-local-vmx-* / -base-* targets, which previously exited 0 as no-ops. That is probably the right outcome, but could we mention it in the PR description?
|
|
||
| # All generated per-image validate targets, used by `make help` so the listing | ||
| # never drifts from the build targets it is derived from. | ||
| ALL_VALIDATE_TARGETS := $(sort \ |
There was a problem hiding this comment.
Could we derive this from .VARIABLES instead of hand-listing the families? Something like $(sort $(foreach v,$(filter %VALIDATE_TARGETS,$(.VARIABLES)),$($(v)))) produces the same 156 targets and removes the last place this can drift.
| @echo ' cleaning artifacts created from building OVAs using a local' | ||
| @echo ' hypervisor.' | ||
| @awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z0-9_-]+:.*?##/ { printf " \033[36m%-35s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST) | ||
| @printf "\n\033[1mValidate packer config (per-image, generated)\033[0m\n" |
There was a problem hiding this comment.
Nit: the generated lines sit one column left of the awk-rendered ones. %-36s or a second space before %s lines them up.
- Derive ALL_VALIDATE_TARGETS from *VALIDATE*_TARGETS vars via .VARIABLES so new provider families need no manual entry (removes last drift point). - Pad generated validate help to %-36s to align with awk-rendered targets. Addresses review feedback on kubernetes-sigs#2149.
|
Thanks for the reviews @drew-viles @mboersma. Addressed all feedback, PTAL when you get a chance 🙏 |
3 participants
Change description
The
##@ Validate packer configsection ofimages/capi/Makefilehand-listed avalidate-<image>: ## ...stub for every per-image validate target. These stubs were maintained separately from the*_BUILD_NAMES/*_VALIDATE_TARGETSlists they document, so they drifted: new images added to the build lists never got a help entry, and removed ones left stale stubs behind.This change removes the hand-maintained per-image stubs and derives the help listing from the same variables that generate the real targets:
Adds
ALL_VALIDATE_TARGETS, a sorted union of every*_VALIDATE_TARGETSlist.make helpnow prints a generated "Validate packer config (per-image)" section fromALL_VALIDATE_TARGETS, so the listing can no longer drift from the build targets it is derived from.Keeps the
-allaggregate targets as-is.Is this change including a new Provider or a new OS? (y/n) n
If yes, has the Provider/OS matrix been updated in the readme? (y/n) n/a
If adding a new provider, are you a representative of that provider? (y/n) n/a
Removes 16 no-op
validate-node-ova-local-vmx-*/-base-*targets thatpreviously existed only as
make helpstubs and exited 0 as no-ops. Theyare not derived from any
*_BUILD_NAMESlist and nothing in CI or scriptsreferences them.
Related issues
Additional context
Verified with:
The generated section lists every per-image
validate-*target sourced from the build-name lists.