Skip to content

Latest commit

 

History

History
571 lines (451 loc) · 25.9 KB

File metadata and controls

571 lines (451 loc) · 25.9 KB

Contributing to Modelplane

Discuss first

Modelplane is a discuss-first project. For anything beyond a small bug fix, open an issue before you write code. The point is to agree on the shape of a change before you (or your agent) spend time and tokens building it, so nobody pours effort into a contribution we then can't accept.

PRs are welcome for small, self-evident fixes: a typo, a broken link, a one-line correction. For anything that changes behavior, an API, or a design, raise an issue first and let's align on the approach. When in doubt, open an issue, not a PR.

Using an agent is fine, but contributions still have to clear the bar the Writing style section sets out. We may close low-effort issues and PRs, including unreviewed AI-generated ones, without much discussion.

Writing style

Using an agent to help write code, commits, PRs, or issues is fine. But the prose it produces should be indistinguishable from something you'd write yourself. An agent's first draft rarely is: it tends to pad, hedge, and decorate. Treat that draft as a starting point and cut it hard, often by half or more. If a sentence isn't carrying information a reader needs, delete it.

The tells to edit out:

  • Filler and throat-clearing. "It's important to note that", "In order to", "This change serves to". Say the thing directly.
  • Table stakes. "Updated the tests", "Ran the linter", "Ensured the code is well-formatted". These are expected, so reporting them is noise.
  • Bragging. "Robust", "elegant", "comprehensive", "powerful", "seamless". A description should let the reader judge the change, not judge it for them.
  • Decorative em dashes. Agents reach for an em dash whenever a sentence could use a comma, a colon, or a full stop. An occasional one is fine; a paragraph built on them reads like a machine wrote it.
  • Restating the obvious. "This PR adds X" when the title says "Add X". Lead with the problem instead.
  • Over-formatting. Bold, headers, and bullets sprinkled in to look organized or authoritative. Prose carries reasoning; reserve structure for content that's genuinely structured.

Never invent rationale. The "why" behind a change comes from the author's intent, which a diff doesn't contain: a diff shows that a field was renamed, not why. A plausible-sounding reason made up to fill the gap is worse than no reason at all, because a reader can't tell it apart from a real one and it lands in the permanent record as fact. If you don't know why a change was made, describe what it does and stop.

The underlying goal is plain, dense prose that respects the reader's time. When in doubt, shorter.

Reporting issues

Open an issue with the bug report or feature request template. The Writing style section above applies here too: lead with the problem, be specific, and don't pad or invent. Don't hard-wrap the body; like a PR, GitHub renders it as Markdown and reflows to the viewport, so write each paragraph as one line and let it wrap.

For a bug, the title should name the symptom, and the root cause if you know it: "InferenceGateway never becomes ready on a fresh control plane: Gateway API CRDs not installed". Describe what you observed before what you think causes it, and back it with evidence a reader can't reconstruct themselves — the actual error message, status condition, or log output in a fenced block, and a link to the offending code with line numbers if you found it. Give numbered, copy-pasteable reproduction steps, and a workaround if you have one. List the versions of everything involved: Modelplane, Crossplane, the inference backend, the cluster and its provider, and Kubernetes.

For a feature request, describe the problem or limitation before any solution; a well-framed problem is worth more than a proposed fix. If you do have a shape in mind, show it concretely — the YAML, CLI, or API a user would write — and note the trade-offs and the alternatives you considered.

Development setup

Modelplane uses Nix for builds, checks, and the development environment. If you have Nix installed, nix develop (or direnv allow if you use direnv) drops you into a shell with everything you need: crossplane, kubectl, helm, kind, Python, linters, and formatters.

If you don't have Nix installed, nix.sh runs any Nix command inside a Docker container. The first run downloads dependencies into a Docker volume. Subsequent runs reuse the cache.

# With Nix installed:
nix develop

# Without Nix installed:
./nix.sh develop

To install Nix itself, the Determinate Systems installer is the easiest option. It enables flakes by default and supports a clean uninstall:

curl -fsSL https://install.determinate.systems/nix | sh -s -- install

Running checks

nix flake check runs all of the project's checks inside the Nix sandbox: Python, shell, and Nix linters and formatters, the ty type checker on every composition function and the end-to-end tests, plus unit tests for every function. Run nix flake show to see what else is available.

nix flake check            # or: ./nix.sh flake check

nix run .#fix auto-fixes most lint and formatting issues. Run it before opening a PR.

nix flake check is the unit layer — fast and sandboxed, proving each composition function renders the right resources. The integration layer is nix run .#e2e, which brings up two local kind clusters and runs the whole path — scheduling, the serving-stack install on a registered cluster, gateway routing, a live request — with no cloud credentials. Add -- --verify and it runs the pytest suite in e2e/tests/, exiting non-zero if a test fails. That verify command is what the label-gated E2E workflow runs on CI (add the test-e2e label to a PR), so a green local --verify and a green CI run mean the same thing. See e2e/README.md.

Submitting changes

Before opening a PR, run nix flake check and make sure it passes. If you changed a composition function, make sure there's a test covering the change.

Commit messages

Each commit is a self-contained, logical layer of the change. The history tells the story of what the code does and why, not how you developed it. Fold incremental work into the commit it belongs to; don't leave behind "address review feedback", "fix tests", WIP, or rebase-merge commits.

Write the subject in the imperative mood, naming specifically what the change does, so it's legible at a glance in a log: "Order KServe CRD deletion after the resources that use it", not "fix deletion" or "update composition". Keep it under ~70 characters, capitalized, with no trailing period and no typed prefix like feat:, fix:, or chore:.

Write a body for every commit except the most mechanical, like a schema regen. Lead with the problem: what was wrong, missing, or how things behaved before. Then give the change and why it's right; the reasoning is what a future reader needs and can't recover from the diff. Note trade-offs honestly. Use prose, not a list recounting which files you touched; the diff already shows what changed, so the message is for what it doesn't show. Reserve bullets for genuinely parallel items. Wrap at ~72 characters, and reference issues with a terse trailer at the end: Fixes #96., Towards #92., Depends on crossplane/cli#24.

Sign off every commit with git commit -s. This adds a Signed-off-by line certifying you have the right to submit the code under the project's license (the Developer Certificate of Origin).

A representative commit:

Order KServe CRD deletion after the resources that use it

compose-kserve-backend installs KServe as two Helm releases: kserve-crds
provides the CRDs, and kserve-controller installs custom resources of those
CRDs.

Nothing ordered their deletion. On teardown the CRD release could uninstall
first, removing a CRD while the resources release still owned CRs of that kind.
The resources release's uninstall then failed and hung indefinitely, blocking
the rest of the teardown.

This adds a Usage holding the CRD release until the resources release is gone,
so the CRs are deleted while their CRD still exists.

Signed-off-by: Nic Cope <nicc@rk0n.org>

Pull requests

A PR description carries the same problem-first story as a commit, scaled up to the whole change. Open with the issue it resolves — Fixes #95. on its own line, one per issue — then describe what was wrong or missing, the change, and why it's the right one. Be honest about trade-offs, breaking changes, and anything still unresolved.

Don't single out parts of the diff as noteworthy or risky to fill a "things to review" section. Flag something only when you actually know it warrants a closer look; a list of callouts chosen just to have one reads as signal while carrying none.

Don't hard-wrap the body. GitHub renders it as Markdown and reflows to the viewport, so write each paragraph as one line and let it wrap; manual breaks show up as ragged lines in the rendered view. (This is the opposite of commit messages, which are read raw and so are wrapped at ~72.)

Be selective. A description isn't a summary of every commit; it's the headline of the change with enough context to review it. Lead with what matters, and let preparatory or secondary commits fall to a sentence or drop out entirely — the reviewer has the commits and the diff for the rest. Resist giving each commit its own ### section; reach for subheadings only when one genuinely large change has distinct parts worth separating. When the change is small, a paragraph or two is the whole description.

Reach for the things a diff can't show: before/after YAML for an API change, a plainly stated breaking-change note, or how you validated the change end to end. Use bullets only for genuinely parallel items.

A representative description:

Fixes #45.

The ModelDeployment scheduler considered all InferenceEnvironments as scheduling candidates regardless of their readiness. An environment still provisioning its cluster would be selected if its computed capacity matched, creating a ModelPlacement that targets an environment that can't serve traffic yet. The user then sees errors until the environment becomes Ready.

This makes the scheduler skip environments without a `Ready=True` condition. When a new environment finishes provisioning, the next reconcile picks it up.

The pool-fitting logic moves into a `_best_pool_fit` helper to keep `schedule()` under the branch-count lint threshold. A new `test-model-deployment-not-ready-env` case covers the skip behavior.

Work from a fork

Open every PR from a branch on your own fork, never a branch of the main repo. This holds for maintainers too, and for any agent working on your behalf: point it at your fork.

Working on composition functions

Modelplane is a Crossplane project. The core logic lives in Python composition functions under functions/. compose-inference-gateway/function/fn.py is a good reference implementation to model new functions on.

Each function is a self-contained Python package, built as a hatch project and managed in the workspace uv.lock:

functions/<name>/
  pyproject.toml      # Hatch package metadata; declares SDK and models deps
  function/
    __init__.py
    __version__.py
    main.py           # CLI entrypoint (boilerplate)
    fn.py             # FunctionRunner gRPC service and Composer logic
  tests/
    test_fn.py        # pytest tests for fn.py

The Composer.compose() method in fn.py reads the XR from the request, composes resources into the response, and tracks readiness. FunctionRunner is the gRPC service that wires Composer to the SDK's runtime. Functions use generated Pydantic models (in schemas/python/) for type-safe access to XR specs and status.

Each function is self-contained; there is no shared library. Common patterns like setting conditions, updating status, and building child resource names are provided by the Crossplane Python Function SDK. Helpers specific to a single function live in that function's function/ package alongside fn.py.

The Pydantic models in schemas/python/ are generated from the XRDs under apis/ and the project's dependency CRDs. They're committed to git so tests and type checking don't need to run the Crossplane CLI first. Regenerate them by building after you change an XRD or bump a dependency:

nix run .#build

The build deletes and recreates the whole schemas/python/ tree, so models for XRDs or dependencies you've removed don't linger.

Tests

Every function has tests under functions/<name>/tests/, run with pytest. test_fn.py tests the function as a whole. A module with logic of its own, such as compose-model-deployment's scheduler, can have its own test_<module>.py too.

The canonical form is a table of Cases, each running the function on a RunFunctionRequest and comparing the whole RunFunctionResponse against an expected one, rather than asserting on individual fields. compose-model-cache is a good example. The skeleton:

@dataclasses.dataclass
class Case:
    name: str
    reason: str
    req: fnv1.RunFunctionRequest
    want: fnv1.RunFunctionResponse


COMPOSE_CASES = [
    Case(
        name="ClusterReady",
        reason="Once the cluster is ready, the XR reports Ready.",
        req=fnv1.RunFunctionRequest(...),
        want=fnv1.RunFunctionResponse(...),
    ),
]


def _to_dict(msg: message.Message) -> dict:
    """msg as a dict with sorted keys, so pytest's diff of two lines them up."""
    return json.loads(json_format.MessageToJson(msg, sort_keys=True))


@pytest.mark.parametrize("case", COMPOSE_CASES, ids=lambda case: case.name)
def test_compose(case: Case) -> None:
    """RunFunction composes the resources an XR needs."""
    got = asyncio.run(fn.FunctionRunner().RunFunction(case.req, None))
    assert _to_dict(got) == _to_dict(case.want), case.reason

Name a table for the test that runs it, and put it just above that test. A Case holds its name and reason, then the inputs of the call under test, named for its parameters, then want. Each case becomes its own test, with its name as its ID, so pytest -k can select it. With got on the left, pytest's diff shows the expected lines as - and the actual lines as +, the same way round as Go's cmp.Diff(want, got). Tests are plain functions, with no classes, fixtures, or conftest.py. They call the async RunFunction with asyncio.run rather than needing a plugin, and check errors with pytest.raises(..., match=...).

Cases are data, so a reader should be able to see everything a case asserts by reading it:

  • Write each case out in full. Repetition between cases is fine. Don't derive one case from another, or from a shared base, by copying and mutating it, and don't change a request or response once it's built. Pass requirements, conditions, and results to the constructor.
  • A resource that appears in three or more cases gets a helper, the XR included. Count resources by the role they play, such as "the GPU node pool" or "an endpoint's Backend". An observed resource plays a different role from the desired resource it reflects, so it gets its own helper. A helper builds that one resource and returns the fnv1.Resource that carries it, or a dict where another resource embeds it. Everything that varies between the cases that use it is a keyword argument with no default, so every call shows every value that varies. A desired resource's readiness is an fnv1.Ready value. A flag that sets an observed resource's Ready condition is a bool. Write a resource that appears in one or two cases inline. Never write a helper that builds a whole request, response, map of resources, or case.
  • Give each case a short name and a reason. The name is a few words of CamelCase, unique in its table, such as JobComplete. The reason is one sentence saying what the case's input sets up and what it expects, and the test passes it as the assertion's message, so pytest prints it when the case fails. Put any further comment on a case directly above its Case(. A short comment beside one value can explain that value.
  • Compare the whole output, once. A test that calls the same entry point with different data belongs in that entry point's table as another case.
  • Write values as literals, in requests and expectations alike, including names the function hashes. An expectation computed by code, whether the code under test or the SDK's child_name, passes whatever that code does.
  • Build the XR from its generated model, with resource.dict_to_struct(xr.model_dump(exclude_none=True, mode="json", by_alias=True)). Write composed and observed resources as dicts in their wire form. The generated models include schema defaults, so a model doesn't fix its own wire form: the SDK sends only the fields a function sets, while the API server fills in the defaults.
  • Say why where a test departs from a rule, in a comment beside the departure.

Because want is the whole response, it must include the parts the function always emits: meta.ttl (60s), an empty context, and any conditions, results, and requirements. Give observed conditions a fixed lastTransitionTime so the input is deterministic. Protobuf maps (desired.resources, requirements.resources) compare order-independently, but repeated fields (conditions, results, status arrays) must match the order the function emits.

nix flake check runs every function's tests, and so does nix run .#test, outside the sandbox. Name a function to run only its tests, and pass pytest arguments after it:

nix run .#test -- compose-usages -k namespace

Each function runs in a pytest session of its own, because every function names its package function.

Running locally

nix run .#run builds the project and runs it on a local development control plane: a KIND cluster with its own OCI registry, created and managed by the Crossplane CLI. It builds the functions, loads the packages into the local registry, installs the Configuration, and points kubectl at the cluster. Iterate by editing a function and rerunning it; tear down with nix run .#stop.

nix run .#run

This needs a running Docker-compatible container runtime (for KIND and the local registry). It works on Linux and macOS with no emulation: the function images are Linux images assembled entirely from data — a prebuilt Python interpreter and dependency wheels plus our own source — so there's no cross-compilation. The same is true of nix run .#build.

Bumping aicr

The serving stack's EKS, AKS and GKE component lists are generated from NVIDIA AICR recipes by functions/compose-serving-stack/generate.py (nix run .#stacks), pinned to one aicr release. A bump changes the components and versions every managed cluster runs on the next Modelplane release, so it lands as a reviewed stack change, never as an auto-merged dependency update. aicr releases about every two weeks and its schemas are still v1alpha*, so expect the generator's fail-closed checks to trip. In order:

  1. Update AICR_PIN in generate.py and version in nix/aicr.nix together, with hashes from the release's aicr_checksums.txt. The two must move in lockstep; the generator refuses a mismatched aicr.
  2. Re-sync the embedded gke-cos fork (GKE_COS_OVERLAY in generate.py): diff it against the new tag's recipes/overlays/gke-cos.yaml and re-apply the one change (the dra profile value, the 1.35 floor, and the DRA selector paths on the stock values for union totality). The file is high-churn upstream. The fork is deleted the day NVIDIA/aicr#2515 or #2517 lands.
  3. Expect ALLOW/DROP to fail closed on any component the new catalog adds; classify it with a reason.
  4. Expect the managed-path assertions to fail closed if a values path moved or a --set stopped landing; re-derive the path from the new chart before loosening anything. MODELPLANE_VALUES fails closed too if a new recipe starts carrying one of its paths - decide whose value wins and move the path to the right table.
  5. Both embedded catalog documents carry an apiVersion that can move in any release.
  6. The Kubernetes floor union re-checks against k8s_default in CLOUDS, which must track the cloud cluster XRD defaults.
  7. nix run .#stacks, run twice to confirm no diff, and review the generated diff as the release's stack change. The stacks-current flake check regenerates in the sandbox and fails CI on a stale or hand-edited generated file.

Where a bumped component also exists in the hand-written cloud halves (function/stacks/clouds/), mirror the shared pins there so one review moves both.

Working on the docs site

Docs prose lives here under docs/content/, with the example manifests it embeds under docs/manifests/ and the API reference's grouping in docs/data/. The site that renders it — the Hugo project, its layouts, theme, and asset pipelines — is the docs-site repo. Edit prose here; edit the site there.

Preview what you are editing, with live reload, from the root of this repo:

nix run github:modelplaneai/docs-site#preview  # http://localhost:1313

The site serves this working tree, so the pages you see are the files under your cursor: no branch to push, no pin to move, and nothing about Hugo checked in here. The version switcher lists every version and those links 404 locally, since only this one is being served.

Versions are this repo's release-X.Y branches: whatever is on release-0.2 is what the 0.2 docs say. The site repo builds each of them, decides which release is latest, and deploys; see RELEASING.md.

Manifest shortcodes

Annotated YAML manifests live under docs/manifests/, one subtree per docs section: getting-started/ backs the getting started guide, concepts/ backs the platform and model concept pages, recipes/ backs the Recipes section, and guides/ backs the Guides section. A page references only manifests from its own section's subtree. Two shortcodes render them in content pages.

manifests renders the file inline with syntax highlighting, followed by a kubectl apply -f <url> block pointing to the published file:

{{</* manifests "concepts/inference-gateway.yaml" */>}}

Optional named args:

Arg Default Effect
apply="false" — omit the kubectl block
command="kubectl delete -f" kubectl apply -f override the verb

Hugo forbids mixing positional and named arguments in one shortcode call, so a call that passes apply= or command= must pass the path as path= too:

{{</* manifests path="concepts/inference-gateway.yaml" apply="false" */>}}

manifest-url emits just the absolute URL of the file, for use inside an existing code fence:

kubectl delete -f {{</* manifest-url "concepts/inference-gateway.yaml" */>}}

Both shortcodes take a path relative to docs/manifests/ and fail the build with a clear error if the file doesn't exist.

The docs-manifests flake check validates every Modelplane manifest the docs show — all the files under docs/manifests/, including the API-reference examples under docs/manifests/reference/ — against the generated Pydantic models in schemas/python/. It runs the models with extra="forbid" at every level, so an example that drifts from the live API schema fails CI: a missing required field, a wrong type, a bad enum, or any field the schema doesn't define (a typo, or a field the API renamed or dropped). The docs can't show a manifest the current API would reject. Resources from other API groups (provider configs, core Kubernetes, Crossplane packages) have no model and are skipped. The validator is docs/utils/validate/validate_manifests.py.

Linting and link checking

Docs prose is linted with Vale, which runs as a flake check, so run it with the rest of CI:

nix flake check

Internal links are checked with htmltest against the built site, which means it runs in the site repo, not here. Nothing there pins a revision of this repo, so a content change that breaks a link fails on the next build there: the preview of your pull request, or the rebuild your merge triggers.

Custom Modelplane rules live in docs/utils/vale/styles/Modelplane/.

Vale flags brand names, acronyms, API types, and technical terms it doesn't recognise. Add them to docs/utils/vale/styles/config/vocabularies/Modelplane/accept.txt — that is the single place for all Vale exceptions. Entries are case-sensitive regular expressions, one per line.

CI runs them on every pull request via the same check (see .github/workflows/ci.yml).

Deployment

The site repo holds the only Vercel project. .github/workflows/docs.yml here asks it to render, and never renders anything itself:

Here There
a pull request touching docs/ or apis/ deploys that revision as a preview
a merge to main or a release-* branch rebuilds every version into production

A merge is what publishes: nothing there pins a content revision, so publishing is a rebuild that reads the tip of every branch.

The preview link is posted on the pull request as soon as it opens, because the hostname follows from the pull request number rather than from the deployment — so the site repo needs no write access here. It answers once that repo's Content workflow finishes, a minute or so later.

A pull request from a fork gets neither secrets nor a write token, so it gets no preview; use the local command above.

Releasing

Cutting a release and versioning the docs is a maintainer task; see RELEASING.md.