-
Notifications
You must be signed in to change notification settings - Fork 128
docs: switch chapter 1 to Charmcraft charm #2259
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: k8s-tutorial-uv
Are you sure you want to change the base?
docs: switch chapter 1 to Charmcraft charm #2259
Conversation
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.
Pull request overview
This PR modernizes the Chapter 1 Kubernetes charm example to use the current Charmcraft initialization template and tooling. The changes transition from a manual charm setup to one generated by charmcraft init --profile kubernetes, incorporating modern Python project management with uv and updated testing infrastructure.
Key changes:
- Replaces manual charm creation steps with
charmcraft initworkflow - Updates project structure to use
pyproject.tomlwith dependency groups instead ofrequirements.txt - Modernizes testing infrastructure with uv-based dependency management
- Updates copyright year from 2025 to 2026
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| examples/k8s-1-minimal/tox.ini | Updated to use uv-venv-lock-runner, dependency groups, reorganized test environments, and added format/lint configurations |
| examples/k8s-1-minimal/tests/unit/test_charm.py | Changed string quotes to double quotes, added testing documentation link |
| examples/k8s-1-minimal/tests/integration/test_charm.py | Updated string quotes, simplified docstrings, changed charm path handling to use resolve() |
| examples/k8s-1-minimal/tests/integration/conftest.py | Enhanced with better logging, CHARM_PATH environment variable support, and improved error handling |
| examples/k8s-1-minimal/src/charm.py | Changed all string literals to double quotes, simplified docstrings |
| examples/k8s-1-minimal/requirements.txt | Removed file (dependencies moved to pyproject.toml) |
| examples/k8s-1-minimal/pyproject.toml | Added comprehensive project configuration with dependency groups for lint, unit, and integration tests |
| examples/k8s-1-minimal/charmcraft.yaml | Modernized to use current Charmcraft format with base/platforms instead of bases, added uv plugin configuration |
| docs/tutorial/write-your-first-machine-charm.md | Updated tool installation instructions to mention uv separately from tox |
| docs/tutorial/from-zero-to-hero-write-your-first-kubernetes-charm/set-up-your-development-environment.md | Complete rewrite with detailed VM setup, Concierge installation, and project directory mounting instructions |
| docs/tutorial/from-zero-to-hero-write-your-first-kubernetes-charm/observe-your-charm-with-cos-lite.md | Updated VM name references from charm-dev to juju-sandbox-k8s, charm name from demo-api-charm to fastapi-demo, model name from welcome-k8s to testing |
| docs/tutorial/from-zero-to-hero-write-your-first-kubernetes-charm/make-your-charm-configurable.md | Updated charm and model name references to match new naming conventions |
| docs/tutorial/from-zero-to-hero-write-your-first-kubernetes-charm/integrate-your-charm-with-postgresql.md | Updated VM, charm, and model name references throughout |
| docs/tutorial/from-zero-to-hero-write-your-first-kubernetes-charm/expose-operational-tasks-via-actions.md | Updated charm and model name references in example commands |
| docs/tutorial/from-zero-to-hero-write-your-first-kubernetes-charm/create-a-minimal-kubernetes-charm.md | Major restructure to use charmcraft init workflow instead of manual file creation, updated all references to new naming conventions |
| .github/workflows/example-charm-integration-tests.yaml | Moved k8s-1-minimal from one test matrix to another |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
95628b9 to
66261ee
Compare
james-garner-canonical
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.
This looks great. I've asked a few questions and made a few wording suggestions, though I know that improving the wording was not the focus of this PR. Feel free to reject or defer these suggestions as needed.
| assumes: | ||
| - juju >= 3.1 | ||
| - k8s-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 wonder if it's worth retaining these. It seems they're not in the Charmcraft template. I'm kind of surprised that assumes: k8s-api isn't in the Kubernetes one. Is that a considered decision on our part, or an oversight?
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 guess containers means it's K8s so maybe it's just redundant?
| [vars] | ||
| src_path = {tox_root}/src | ||
| tests_path = {tox_root}/tests | ||
| ;lib_path = {tox_root}/lib/charms/operator_name_with_underscores |
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.
This is kinda odd. I guess it just comes from the template? I wonder if that should be updated. And shouldn't operator_name_with_underscores be templated in with the actual charm name during charmcraft init? Just thinking aloud, not a problem for this PR -- though maybe we should just drop the line if that would be less confusing for users.
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.
Maybe it needs an explanatory comment in the template like the instances below.
| # if this charm owns a lib, uncomment "lib_path" variable | ||
| # and uncomment the following line | ||
| # codespell {[vars]lib_path} | ||
| codespell {tox_root} |
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.
Also seems odd, would we need a separate codespell invocation or would codespell {tox_root} cover it?
| These files currently contain placeholder code and configuration. | ||
|
|
||
| Second, add a constraint assuming a Juju version with the required features and a Kubernetes-type cloud: | ||
| Charmcraft also created a module called `src/fastapi_demo.py`. We won't need this module. In general, it's a good place to put functions that interact with the running workload. |
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.
We won't need this module.
So delete it? Ignore it?
In general, it's a good place to put functions that interact with the running workload.
But in this tutorial, we'll just put everything in charm.py?
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.
Yeah, I agree I phrased this a bit strangely. What I meant to convey was, we can ignore the workload module here, but you shouldn't always ignore it. On second thought, maybe we should find a use for it - e.g., interacting with the workload to get its version, as we do in the machine charm tutorial.
This is an intermediate PR for switching the Kubernetes charm tutorial to use uv and the latest Charmcraft profiles. Target branch is
k8s-tutorial-uvThis PR switches "Create a minimal Kubernetes charm" to use a charm project generated by the latest version of Charmcraft. That lets us remove several of the manual steps.
Preview doc
I'm fully updating "Create a minimal Kubernetes charm" and the corresponding example charm. I'm also partially updating subsequent chapters, for consistency of naming, but I'm not updating the subsequent example charms yet. I'll fully update the subsequent chapters and example charms in follow-on PRs.
There's some noise in the PR due to:
demo-api-charmtofastapi-demoIn
examples/k8s-minimal-1, the charm code incharm.pyand the unit & integration tests are essentially the same as before. Other charm files match what comes from Charmcraft. (charmcraft.yamlcomes with a placeholder config option, which we'll replace in a later chapter of the tutorial, in a subsequent PR.)So I think the most important part to review is
create-a-minimal-kubernetes-charm.md.