-
Notifications
You must be signed in to change notification settings - Fork 99
Add lightweight netlab API server command #2969
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
base: dev
Are you sure you want to change the base?
Conversation
|
This is primarily an automation I built to hook netlab into Semaphore UI |
|
Thank you ! I certainly see some future in this. Could you please write some words describing the problem you are trying to solve with this commit? So we can start from a common understanding? |
|
Engineers here need to model and test lots of different vendors and networks. We've had some internal tooling in the past to do this manually (some hack expect style scripts), but have switched to netlab to make the environment a bit more composable. We started using Semaphore UI (https://semaphoreui.com/) as a job execution engine, which works well with the built in terraform/ansible worklows in a multi-user environment. It can also do manual bash/python scripts. I'd started with bash scripts with sshpass, but found them kinda brittle. I wanted to switch to an API based workflow to make it a bit more robust. I'd initially wrote a FastAPI based wrapper that imports these functions (which I'm still using), but I had the thought that this could be easily extended in the native CLI workflow. A typed version I think still has use and this could be extended for that. |
|
@captainpacket -- Thanks a million for the PR. Definitely an interesting idea ;) and I truly appreciate you crossing all the Ts and dotting all the Is. Give me a few days -- I'm stuck in the "we have to deal with another Ansible brokenness" bog, and have to get out of that first. |
61a1f3d to
76d3284
Compare
76d3284 to
57e732e
Compare
|
Cleaned up a few semaphore specific things (project/task) that I'd left behind; added minimal authentication and TLS option |
ipspace
left a comment
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.
Mostly minor details and improvement suggestions, the only showstopper is whether jobs can be run in parallel and how to handle that.
docs/netlab/cli.md
Outdated
| netlab config <config.md> | ||
| netlab connect <connect.md> | ||
| netlab create <create.md> | ||
| netlab api <api.md> |
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 this list is sorted, so "netlab api" should be at the very top
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.
Good catch — moved netlab api to the top so the list stays sorted.
netsim/cli/api.py
Outdated
| from . import status as netlab_status | ||
| from . import up as netlab_up | ||
|
|
||
| DATA_DIR = os.getenv("NETLAB_API_DATA_DIR", "/var/lib/netlab/api") |
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 this is user-writeable on most systems, so the HTTP server would have to be started as root. I usually use "/tmp/something" in these cases (or you could use "~/...")
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.
Agreed — changed the default data/log dir to a user-writable temp location; still overrideable with NETLAB_API_DATA_DIR.
| handler.wfile.write(data) | ||
|
|
||
|
|
||
| def require_auth(handler: BaseHTTPRequestHandler) -> bool: |
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.
Just wondering: isn't there a Python library to handle this?
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 couldn’t find a clean stdlib helper for server-side Basic Auth parsing; kept stdlib-only to avoid extra deps and refactored it into a small helper to keep the handler clean.
netsim/cli/api.py
Outdated
| def workspace_dir(payload: Dict[str, Any]) -> str: | ||
| workdir = (payload.get("workdir") or "").strip() | ||
| if workdir: | ||
| if os.path.isabs(workdir): |
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.
It looks to me like you're trying to get an absolute path to the working directory. If that's the case you could probably use payload.get("workdir",payload.get("workspaceRoot",os.getcwd())) to get the path you want to get or loop through the keywords instead of effectively having the same code twice.
Also, I'd usually use pathlib.Path(path).resolve() to get an absolute path.
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.
Refactored workdir/workspaceRoot selection and switched to pathlib.Path(...).resolve() for absolute paths; removed duplicated branches.
netsim/cli/api.py
Outdated
|
|
||
|
|
||
| def resolve_topology(payload: Dict[str, Any], workdir: str) -> str: | ||
| topology_url = (payload.get("topologyUrl") or "").strip() |
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.
As you're using the same expression so many times, it might make sense to turn it into a function
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.
Done — factored the repeated path/payload handling into helper functions.
netsim/cli/api.py
Outdated
| results: List[Dict[str, str]] = [] | ||
| if not template_dir: | ||
| return results | ||
| for root, dirs, files in os.walk(template_dir): |
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.
It looks like you're doing list(glob('**/*.yml')) + list(glob('**/*yaml')). You could use glob from os.path or from pathlib
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.
Switched template discovery to pathlib globbing for .yml/.yaml
netsim/cli/api.py
Outdated
| _run_with_output(netlab_down.run, args) | ||
| elif action == "collect": | ||
| args = [] | ||
| if payload.get("instance"): |
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 would probably do this with a dictionary mapping payload variable names into netlab args, and use the same approach for all commands.
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.
Agreed — refactored payload→argv construction to a consistent helper-based approach across actions.
| os.chdir(prev_cwd) | ||
|
|
||
|
|
||
| def start_job(payload: Dict[str, Any]) -> Dict[str, Any]: |
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.
Can you start jobs in parallel? If that's the case, you need multilab plugin, or you have to serialize the jobs.
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.
Good point; I’m serializing job execution (queueing requests but only running one netlab action at a time). If we later want true parallel jobs, we should detect/require multilab and unique ID per job rather than reinventing isolation.
netsim/cli/api.py
Outdated
| send_json(self, HTTPStatus.NOT_FOUND, {"error": "not found"}) | ||
| return | ||
|
|
||
| if parts == ["healthz"]: |
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 would usually implement something like this with a dictionary mapping keywords (parts in you case) into functions that do the actual 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.
Refactored routing to a dict mapping endpoints/keywords to handler functions (kept the /jobs//... special-case)
|
|
||
|
|
||
| def run(cli_args: List[str]) -> None: | ||
| parser = argparse.ArgumentParser(description="netlab API server") |
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.
All other CLI commands have parser in a separate function to keep the "run" function shorter (and cleaner)
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.
Aligned with other CLI modules — moved parser construction into a separate api_parse_args() function so run() stays short.
Adds a minimal netlab API server (stdlib HTTP) to expose CLI actions for automation. Includes new CLI command, help listing, and docs page.