feat(ASB-4540): centralise CLI API endpoints and report cluster health in info#127
feat(ASB-4540): centralise CLI API endpoints and report cluster health in info#127marioaxonops wants to merge 6 commits into
Conversation
Documents how to run the CLI and its unittest suite from cli/, the component read-modify-write convention, base-URL/auth resolution, and the stale dependency instructions in the CLI README. Signed-off-by: Mario Nugnes <mario.nugnes@digitalis.io>
Every component declared its own endpoint constant as a class attribute, and axonops.py repeated three paths inline. Move them all into a single axonopscli/urls.py module and import from there, so an endpoint change is a one-line edit in one file. The nodes path in find_nodes_ids was missing its leading slash, which would have concatenated it onto the host with no separator. Using the shared NODES_URL constant corrects it. Signed-off-by: Mario Nugnes <mario.nugnes@digitalis.io>
Add an Orgs component for /api/v1/orgs and call it from info. The endpoint returns an org -> type -> cluster tree; flatten it and report each cluster as healthy or not, mapping status 0/1/2 to OK/Warning/Error and anything else to Unknown. Clusters that are not OK are repeated in a summary at the end so problems are visible without reading the whole list. Signed-off-by: Mario Nugnes <mario.nugnes@digitalis.io>
Signed-off-by: Mario Nugnes <mario.nugnes@digitalis.io>
The info command prints cluster health before nodes; the README described and showed the reverse order. Signed-off-by: Mario Nugnes <mario.nugnes@digitalis.io>
rgooding
left a comment
There was a problem hiding this comment.
I'm not convinced that the cluster list really fits in the "info" output here. The "info" command shows you some information about the cluster you specify on the command line, and showing a list of all clusters in the output doesn't feel like it's in the right place.
This also does not achieve the original goal of having a simple command which you can run to see if any of your clusters are unhealthy.
I think we should have the cluster list as its own subcommand, maybe cluster-health, which just prints out the list of unhealthy clusters and exits with return code 0 if all clusters are healthy, or 1 if any clusters are not healthy. I think it would also be nice to include a couple of command-line arguments:
--show-healthy: displays all clusters in the output, including healthy clusters--cluster-type: allow filtering the output and health check to a specific cluster type (e.g. for if you only want to check the status of your kafka clusters)
|
Fair comments from @rgooding I overlooked. Perhaps change the output to json so that you can parse it. That way, if you script it or use the output from another action you can filter for healthy, unhealthy, etc?? Thoughts? |
The info command mixed connection diagnostics with cluster health. Rename it to health and make the health report the default output: either "All clusters are healthy" or the list of clusters that are not OK. --show-healthy adds the healthy clusters and the node inventory, --show-orgs adds the visible organisations, and the settings/auth summary is now printed only with -v. The command exits 1 when any cluster is not OK so it can be used as a gate in a script or CI job. Signed-off-by: Mario Nugnes <mario.nugnes@digitalis.io>
|
Addressed the review suggestion:
Added Note: the AxonOps API reports status per cluster, not per node, so the node list under |
This is a very good point; we should add a |
Added a |
Summary
axonops.pyrepeated three paths inline. All of them now live in a singleaxonopscli/urls.py, so changing an endpoint is a one-line edit in one file.info— a newOrgscomponent queries/api/v1/orgs, flattens the returned org → type → cluster tree, and reports each cluster as healthy or not (0/1/2→OK/Warning/Error, anything elseUnknown). Non-OK clusters are repeated in a summary at the end.cli/CLAUDE.md— guidance for theaxonopsclisubproject: how to run the CLI and its tests, the component read-modify-write convention, and base-URL/auth resolution.Jira: ASB-4540
Incidental fix
AxonOps.find_nodes_idsbuilt its path asapi/v1/nodes/...with no leading slash, which concatenated onto the host with no separator (https://host+api/v1/...). Adopting the sharedNODES_URLconstant corrects it. That function is unreachable as written anyway — it unpacksdo_requestas a(result, error)tuple whiledo_requestreturns a bare dict — so nothing depended on the broken form.Test plan
No molecule scenarios apply: this branch touches only
cli/axonopscliPython and documentation — no roles, templates, or workflows. The six-platform molecule matrix is therefore not exercised.pre-commit run --all-files— passed (trailing whitespace, end-of-files, check-yaml, yamllint).python3 -m unittest discover -s testsfromcli/— 9 tests, 3 errors, all pre-existing onmain(verified by stashing and re-running). They fail onAttributeError: 'AxonOps' object has no attribute 'do_login'indo_request, unrelated to these changes.do_requestthat every component builds a byte-identicalfull_urlto before the refactor, and thatOrgsrenders correctly for mixed-status, all-healthy, and empty responses.No new tests were added for the
Orgscomponent. The existing suite is already partly broken, and adding tests to it is better done alongside fixingdo_login/ thedo_requestreturn-type mismatch. Flagging rather than hiding it.Checklist
pre-commit run --all-filespassesansible-lint(production profile) — N/A, no Ansible files changedKnown issues not addressed here
cli/README.mdhas no AxonOps logo header or contact footer, against the branding standard. Pre-existing onmain; deliberately left for a separate docs-only PR rather than bundling a rewrite into a refactor.-vprints request headers including theAuthorization: Bearer <token>line (do_request). Pre-existing, but this PR documents-vin the README and so makes it more discoverable. Worth a follow-up to mask it.Assisted-by: Claude Code