Skip to content

Commit

Permalink
Ignore pre-upgrade-check on k8s-workers
Browse files Browse the repository at this point in the history
  • Loading branch information
addyess committed Dec 16, 2024
1 parent 147cc94 commit 963c140
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 20 deletions.
3 changes: 2 additions & 1 deletion charms/worker/k8s/src/inspector.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def _get_client(self) -> Client:
self.client = Client(config=config.get())
return self.client

def get_nodes(self, labels: LabelSelector) -> Optional[List[Node]]:
def get_nodes(self, labels: Optional[LabelSelector] = None) -> Optional[List[Node]]:
"""Get nodes from the cluster.
Args:
Expand All @@ -56,6 +56,7 @@ def get_nodes(self, labels: LabelSelector) -> Optional[List[Node]]:
Raises:
ClusterInspectorError: If the nodes cannot be retrieved.
"""
labels = labels or {}
client = self._get_client()
try:

Expand Down
7 changes: 4 additions & 3 deletions charms/worker/k8s/src/upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,11 @@ def pre_upgrade_check(self) -> None:
Raises:
ClusterNotReadyError: If the cluster is not ready for an upgrade.
"""
if self.charm.is_worker:
log.info("TODO: Find some pre-upgrade checks for worker application.")
return
try:
nodes = self.cluster_inspector.get_nodes(
labels={"juju-charm": "k8s-worker" if self.charm.is_worker else "k8s"}
)
nodes = self.cluster_inspector.get_nodes()
failing_pods = self.cluster_inspector.verify_pods_running(["kube-system"])
except ClusterInspector.ClusterInspectorError as e:
raise ClusterNotReadyError(
Expand Down
15 changes: 7 additions & 8 deletions charms/worker/k8s/tests/unit/test_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class TestK8sUpgrade(unittest.TestCase):
def setUp(self):
"""Set up common test fixtures."""
self.charm = MagicMock()
self.charm.is_worker = False
self.node_manager = MagicMock(spec=ClusterInspector)
self.upgrade = K8sUpgrade(
self.charm,
Expand Down Expand Up @@ -53,8 +54,8 @@ def test_pre_upgrade_check_worker_success(self):

self.upgrade.pre_upgrade_check()

self.node_manager.get_nodes.assert_called_once_with(labels={"juju-charm": "k8s-worker"})
self.node_manager.verify_pods_running.assert_called_once_with(["kube-system"])
self.node_manager.get_nodes.assert_not_called()
self.node_manager.verify_pods_running.assert_not_called()

def test_pre_upgrade_check_control_plane_success(self):
"""Test pre_upgrade_check succeeds for control plane nodes."""
Expand All @@ -64,15 +65,14 @@ def test_pre_upgrade_check_control_plane_success(self):

self.upgrade.pre_upgrade_check()

self.node_manager.get_nodes.assert_called_once_with(labels={"juju-charm": "k8s"})
self.node_manager.get_nodes.assert_called_once_with()

def test_pre_upgrade_check_unready_nodes(self):
"""Test pre_upgrade_check fails when nodes are not ready."""
self.charm.is_worker = True
self.node_manager.get_nodes.return_value = [
Node(metadata=ObjectMeta(name="worker-1")),
Node(metadata=ObjectMeta(name="worker-2")),
Node(metadata=ObjectMeta(name="worker-3")),
Node(metadata=ObjectMeta(name="k8s-1")),
Node(metadata=ObjectMeta(name="k8s-2")),
Node(metadata=ObjectMeta(name="k8s-3")),
]

with self.assertRaises(ClusterNotReadyError):
Expand All @@ -89,7 +89,6 @@ def test_pre_upgrade_check_cluster_inspector_error(self):

def test_pre_upgrade_check_pods_not_ready(self):
"""Test pre_upgrade_check fails when pods are not ready."""
self.charm.is_worker = True
self.node_manager.get_nodes.return_value = None
self.node_manager.verify_pods_running.return_value = "kube-system/pod-1"

Expand Down
7 changes: 4 additions & 3 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from pytest_operator.plugin import OpsTest

from .cos_substrate import LXDSubstrate
from .helpers import Bundle, Charm, cloud_type, get_unit_cidrs, is_deployed
from .helpers import Bundle, CharmUrl, cloud_type, get_unit_cidrs, is_deployed

log = logging.getLogger(__name__)
TEST_DATA = Path(__file__).parent / "data"
Expand Down Expand Up @@ -72,7 +72,8 @@ def pytest_configure(config):
config.addinivalue_line("markers", "cos: mark COS integration tests.")
config.addinivalue_line(
"markers",
"bundle(file='', apps_local={}, apps_channel={}, apps_resources={}): specify a YAML bundle file for a test.",
"bundle(file='', apps_local={}, apps_channel={}, apps_resources={}): "
"specify a YAML bundle file for a test.",
)


Expand Down Expand Up @@ -202,7 +203,7 @@ async def grafana_agent(kubernetes_cluster: Model):
machine_series = juju.utils.get_version_series(data["base"].split("@")[1])

await kubernetes_cluster.deploy(
Charm.craft_url("grafana-agent", machine_series, machine_arch),
str(CharmUrl.craft("grafana-agent", machine_series, machine_arch)),
channel="stable",
series=machine_series,
)
Expand Down
7 changes: 4 additions & 3 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from functools import cache, cached_property
from itertools import chain
from pathlib import Path
from typing import Dict, List, Mapping, Optional, Sequence, Tuple
from typing import Dict, List, Mapping, Optional, Tuple

import yaml
from juju import unit
Expand Down Expand Up @@ -326,6 +326,7 @@ def find(cls, name: str) -> Optional["Charm"]:
"""
if charmcraft := CHARMCRAFT_DIRS.get(name):
return cls(charmcraft)
return None

async def resolve(self, ops_test: OpsTest, arch: str) -> "Charm":
"""Build or find the charm with ops_test.
Expand Down Expand Up @@ -440,7 +441,6 @@ async def discover_charm_files(self, ops_test: OpsTest) -> Dict[str, Charm]:
Args:
ops_test: Instance of the pytest-operator plugin
arch: Cloud architecture
Returns:
Mapping: application name to Charm object
Expand All @@ -457,6 +457,7 @@ async def apply_marking(self, ops_test: OpsTest, markings: Markings):
Args:
ops_test: Instance of the pytest-operator plugin
markings: Markings from the test
"""
_type, _vms = await cloud_type(ops_test)
if _type == "lxd" and not _vms:
Expand Down Expand Up @@ -496,7 +497,7 @@ def switch(
resources (dict): Optional resources to add
Raises:
ValueError: if both path and channel are provided, or neither are provided
FileNotFoundError: if the local charm file is not found
"""
app = self.applications.get(name)
if not app:
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import yaml
from pytest_operator.plugin import OpsTest

from .helpers import Bundle, Charm, get_leader
from .helpers import Bundle, get_leader

# This pytest mark configures the test environment to use the Canonical Kubernetes
# deploying charms from the edge channels, then upgrading them to the built charm.
Expand Down Expand Up @@ -53,7 +53,7 @@ async def _refresh(app_name: str):
app: Optional[juju.application.Application] = kubernetes_cluster.applications[app_name]
assert app is not None, f"Application {app_name} not found"

log.info(f"Refreshing {app_name}")
log.info("Refreshing %s", app_name)
leader_idx: int = await get_leader(app)
leader: juju.unit.Unit = app.units[leader_idx]
action = await leader.run_action("pre-upgrade-check")
Expand Down

0 comments on commit 963c140

Please sign in to comment.