-
Notifications
You must be signed in to change notification settings - Fork 135
docs: grab workload version in k8s tutorial #2559
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: main
Are you sure you want to change the base?
Changes from all commits
cc1a517
9381cf9
2234b14
c448138
6f5e0e2
601f883
1bf544f
f41826e
e522d58
035a521
5ac4068
80bbc83
050a5d9
93f8331
5ef30e9
e25cec0
e2cd1b6
54d186e
0908fdb
8d596b3
38fff1d
49e6a12
272592c
7734bdc
befcb27
6ac7f7b
16190ec
790f3f2
9efb148
1844a5b
c97ebc4
1ed5882
1d9f2f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,6 +236,9 @@ def _replan_workload(self) -> None: | |
| logger.info(f"Replanned with '{self.pebble_service_name}' service") | ||
| except (ops.pebble.APIError, ops.pebble.ConnectionError) as e: | ||
| logger.info("Unable to connect to Pebble: %s", e) | ||
| return | ||
| version = fastapi_demo.get_version(config.server_port) | ||
| self.unit.set_workload_version(version) | ||
| ``` | ||
|
|
||
| We removed three `self.unit.status = ` lines from this version of the method. We'll handle replacing those shortly. | ||
|
Comment on lines
+239
to
244
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following text for this code block is now kind of contradictory -- it's explaining that we removed If this is the right pattern, then I think it could use some explanation as to why it's not replaced. But is it the right pattern? It looks like our collect status handler will happily clobber this blocked status with active status.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Is that true? I had assumed that the collect-status handler wouldn't override higher priority status with a lower priority one - but I actually don't know. In tutorial text we say:
Which suggested to me that a maintenance status would survive collect-status. And so presumably blocked status would too. I think it would be good to test this to confirm.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh but I suppose the point is that maintenance status gets overridden after the work in the event execution has completed. Hmm |
||
|
|
@@ -419,7 +422,7 @@ Congratulations, your relation with PostgreSQL is functional! | |
| Now that our charm uses `fetch_database_relation_data` to extract database authentication data and endpoint information from the relation data, we should write a test for the feature. Here, we're not testing the `fetch_database_relation_data` function directly, but rather, we're checking that the response to a Juju event is what it should be: | ||
|
|
||
| ```python | ||
| def test_relation_data(): | ||
| def test_relation_data(mock_version): | ||
| ctx = testing.Context(FastAPIDemoCharm) | ||
| relation = testing.Relation( | ||
| endpoint="database", | ||
|
|
@@ -453,7 +456,7 @@ def test_relation_data(): | |
| In this chapter, we also defined a new method `_on_collect_status` that checks various things, including whether the required database relation exists. If the relation doesn't exist, we wait and set the unit status to `blocked`. We can also add a test to cover this behaviour: | ||
|
|
||
| ```python | ||
| def test_no_database_blocked(): | ||
| def test_no_database_blocked(mock_version): | ||
| ctx = testing.Context(FastAPIDemoCharm) | ||
| container = testing.Container(name="demo-server", can_connect=True) | ||
| state_in = testing.State( | ||
|
|
@@ -522,6 +525,10 @@ def test_database_integration(charm: pathlib.Path, juju: jubilant.Juju): | |
| juju.deploy("postgresql-k8s", channel="14/stable", trust=True) | ||
| juju.integrate(APP_NAME, "postgresql-k8s") | ||
| juju.wait(jubilant.all_active) | ||
|
|
||
| version = juju.status().apps[APP_NAME].version | ||
| # Hardcoded version for simplicity. Ideally we'd get the version directly from the workload. | ||
| assert version == "1.0.4" | ||
| ``` | ||
|
|
||
| This test depends on the `charm` fixture so that the test fails immediately if a `.charm` file isn't available. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,11 +127,13 @@ def _replan_workload(self) -> None: | |
| # service if required. | ||
| self.container.replan() | ||
| logger.info(f"Replanned with '{self.pebble_service_name}' service") | ||
|
|
||
| self.unit.status = ops.ActiveStatus() | ||
| except (ops.pebble.APIError, ops.pebble.ConnectionError) as e: | ||
| logger.info("Unable to connect to Pebble: %s", e) | ||
| self.unit.status = ops.MaintenanceStatus("Waiting for Pebble in workload container") | ||
| return | ||
| self.unit.status = ops.ActiveStatus() | ||
| version = fastapi_demo.get_version(config.server_port) | ||
| self.unit.set_workload_version(version) | ||
|
Comment on lines
+134
to
+136
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very minor, but perhaps
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, initially However, we decided that putting In chapter 1, section And this code propagated into Chapter 2 as you see here, and I kept it like that. |
||
| ``` | ||
|
|
||
| When the config is loaded as part of creating the Pebble layer, if the config is invalid (in our case, if the port is set to 22), then a `ValueError` will be raised. The `_replan_workload` method handles that by logging the error and setting the status of the unit to blocked, letting the Juju user know that they need to take action. | ||
|
|
@@ -242,7 +244,7 @@ Since we added a new feature to configure `server-port` and use it in the Pebble | |
| First, we'll add a test that sets the port in the input state and asserts that the port is used in the service's command in the container layer: | ||
|
|
||
| ```python | ||
| def test_config_changed(): | ||
| def test_config_changed(mock_version): | ||
|
tromai marked this conversation as resolved.
|
||
| ctx = testing.Context(FastAPIDemoCharm) | ||
| container = testing.Container(name="demo-server", can_connect=True) | ||
| state_in = testing.State( | ||
|
|
@@ -260,10 +262,12 @@ def test_config_changed(): | |
| assert "--port=8080" in command | ||
| ``` | ||
|
|
||
| We need the `mock_version` fixture because `_on_config_changed` calls `_replan_workload`, which gets the workload version using `fastapi_demo.get_version`. The fixture patches `get_version` to avoid making a real HTTP call, so that the unit test deterministic. | ||
|
|
||
| In `_on_config_changed`, we specifically don't allow port 22 to be used. If port 22 is configured, we set the unit status to `blocked`. So, we can add a test to cover this behaviour by setting the port to 22 in the input state and asserting that the unit status is blocked: | ||
|
|
||
| ```python | ||
| def test_config_changed_invalid_port(): | ||
| def test_config_changed_invalid_port(mock_version): | ||
| ctx = testing.Context(FastAPIDemoCharm) | ||
| container = testing.Container(name="demo-server", can_connect=True) | ||
| state_in = testing.State( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| # Copyright 2026 Canonical Ltd. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| """Functions for interacting with the workload. | ||
|
|
||
| The intention is that this module could be used outside the context of a charm. | ||
| """ | ||
|
|
||
| import json | ||
| import logging | ||
| import urllib.request | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def get_version(port: int) -> str: | ||
| """Get the version of fastapi_demo that is running. | ||
|
|
||
| Args: | ||
| port: The port where fastapi_demo web server is listening. | ||
| """ | ||
| response = urllib.request.urlopen(f"http://localhost:{port}/version") | ||
| data = json.loads(response.read()) | ||
| return data["version"] |
Uh oh!
There was an error while loading. Please reload this page.