-
Notifications
You must be signed in to change notification settings - Fork 4
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
Replace snap channel config for hard coded revision config #79
Changes from 1 commit
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 |
---|---|---|
|
@@ -42,10 +42,6 @@ bases: | |
architectures: [amd64] | ||
config: | ||
options: | ||
channel: | ||
default: edge | ||
type: string | ||
description: Snap channel of the k8s snap | ||
labels: | ||
default: "" | ||
type: string | ||
|
@@ -59,15 +55,17 @@ parts: | |
charm: | ||
build-packages: [git] | ||
charm-entrypoint: k8s/src/charm.py | ||
lib: | ||
# move the ./k8s/lib path to ./lib since | ||
# charmcraft assumes it to be there once the charm runs | ||
promote: | ||
# move paths out of ./k8s to ./ since | ||
# charmcraft assumes ./lib to be there | ||
# charmcraft assumes ./templates to be there | ||
after: [charm] | ||
plugin: nil | ||
source: ./ | ||
override-prime: | | ||
rm -rf $CRAFT_PRIME/lib | ||
rm -rf $CRAFT_PRIME/lib $CRAFT_PRIME/templates | ||
mv $CRAFT_PRIME/k8s/lib $CRAFT_PRIME/lib | ||
mv $CRAFT_PRIME/k8s/templates $CRAFT_PRIME/templates | ||
Comment on lines
-69
to
+68
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. This moves the templates folder from |
||
|
||
provides: | ||
cos-agent: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,10 +51,6 @@ bases: | |
architectures: [amd64] | ||
config: | ||
options: | ||
channel: | ||
default: edge | ||
type: string | ||
description: Snap channel of the k8s snap | ||
Comment on lines
-54
to
-57
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. Nope, revisions are hard coded |
||
datastore: | ||
default: dqlite | ||
type: string | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
from urllib.parse import urlparse | ||
|
||
import charms.contextual_status as status | ||
import charms.operator_libs_linux.v2.snap as snap_lib | ||
import ops | ||
import yaml | ||
from charms.contextual_status import WaitingStatus, on_error | ||
|
@@ -50,10 +51,9 @@ | |
) | ||
from charms.kubernetes_libs.v0.etcd import EtcdReactiveRequires | ||
from charms.node_base import LabelMaker | ||
from charms.operator_libs_linux.v2.snap import SnapError, SnapState | ||
from charms.operator_libs_linux.v2.snap import ensure as snap_ensure | ||
from charms.reconciler import Reconciler | ||
from cos_integration import COSIntegration | ||
from snap import management as snap_management | ||
from token_distributor import ClusterTokenType, TokenCollector, TokenDistributor, TokenStrategy | ||
|
||
# Log messages can be retrieved using juju debug-log | ||
|
@@ -198,12 +198,11 @@ def get_cloud_name(self) -> str: | |
""" | ||
return self.xcp.name or "" | ||
|
||
@on_error(ops.BlockedStatus("Failed to install k8s snap."), SnapError) | ||
def _install_k8s_snap(self): | ||
"""Install the k8s snap package.""" | ||
@on_error(ops.BlockedStatus("Failed to install snaps."), snap_lib.SnapError) | ||
def _install_snaps(self): | ||
"""Install snap packages.""" | ||
status.add(ops.MaintenanceStatus("Ensuring snap installation")) | ||
log.info("Ensuring k8s snap version") | ||
snap_ensure("k8s", SnapState.Latest.value, self.config["channel"]) | ||
snap_management() | ||
Comment on lines
-201
to
+205
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. let's move all the logic about managing the snap revision to another module |
||
|
||
@on_error(WaitingStatus("Waiting to apply snap requirements"), subprocess.CalledProcessError) | ||
def _apply_snap_requirements(self): | ||
|
@@ -527,7 +526,7 @@ def _reconcile(self, event): | |
return | ||
|
||
self._apply_proxy_environment() | ||
self._install_k8s_snap() | ||
self._install_snaps() | ||
self._apply_snap_requirements() | ||
self._check_k8sd_ready() | ||
if self.lead_control_plane: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
#!/usr/bin/env python3 | ||
|
||
# Copyright 2024 Canonical Ltd. | ||
# See LICENSE file for licensing details. | ||
|
||
# Learn more at: https://juju.is/docs/sdk | ||
|
||
"""Snap Installation Module.""" | ||
|
||
|
||
import logging | ||
import subprocess | ||
from pathlib import Path | ||
from typing import List, Literal, Optional, Union | ||
|
||
import charms.operator_libs_linux.v2.snap as snap_lib | ||
import yaml | ||
from pydantic import BaseModel, Field, ValidationError, parse_obj_as | ||
from typing_extensions import Annotated | ||
|
||
# Log messages can be retrieved using juju debug-log | ||
log = logging.getLogger(__name__) | ||
|
||
|
||
class SnapFileArgument(BaseModel): | ||
"""Structure to install a snap by file. | ||
|
||
Attributes: | ||
install_type (str): literal string defining this type | ||
name (str): The name of the snap after installed | ||
filename (Path): Path to the snap to locally install | ||
classic (bool): If it should be installed as a classic snap | ||
dangerous (bool): If it should be installed as a dangerouse snap | ||
devmode (bool): If it should be installed as with dev mode enabled | ||
""" | ||
|
||
install_type: Literal["file"] = Field("file", alias="install-type", exclude=True) | ||
name: str = Field(exclude=True) | ||
filename: Optional[Path] = None | ||
classic: Optional[bool] = None | ||
devmode: Optional[bool] = None | ||
dangerous: Optional[bool] = None | ||
Comment on lines
+25
to
+42
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. This looks like all the arguments in two arguments |
||
|
||
|
||
class SnapStoreArgument(BaseModel): | ||
"""Structure to install a snap by snapstore. | ||
|
||
Attributes: | ||
install_type (str): literal string defining this type | ||
name (str): The type of the request. | ||
state (SnapState): a `SnapState` to reconcile to. | ||
classic (bool): If it should be installed as a classic snap | ||
devmode (bool): If it should be installed as with dev mode enabled | ||
channel (bool): the channel to install from | ||
cohort (str): the key of a cohort that this snap belongs to | ||
revision (int): the revision of the snap to install | ||
""" | ||
|
||
install_type: Literal["store"] = Field("store", alias="install-type", exclude=True) | ||
name: str = Field(exclude=True) | ||
classic: Optional[bool] = None | ||
devmode: Optional[bool] = None | ||
state: Optional[snap_lib.SnapState] = Field(snap_lib.SnapState.Present) | ||
channel: Optional[str] = None | ||
cohort: Optional[str] = None | ||
revision: Optional[int] = None | ||
Comment on lines
+45
to
+66
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. This looks like all the arguments in Again, the two arguments |
||
|
||
|
||
SnapArgument = Annotated[ | ||
Union[SnapFileArgument, SnapStoreArgument], Field(discriminator="install_type") | ||
Comment on lines
+69
to
+70
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. Nice! I love the discriminator logic here. 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. this is what we have to work with in pydantic 1.x. This has been changed in pydantic 2.x a bit |
||
] | ||
Comment on lines
+69
to
+71
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. This is pydantic's fancy way of creating a type which can parse data as either the |
||
|
||
|
||
def _parse_management_arguments() -> List[SnapArgument]: | ||
"""Parse snap management arguments. | ||
|
||
Raises: | ||
SnapError: when the management issue cannot be resolved | ||
|
||
Returns: | ||
Parsed arguments list for the specific host architecture | ||
""" | ||
revision = Path("templates/snap_installation.yaml") | ||
if not revision.exists(): | ||
raise snap_lib.SnapError(f"Failed to find file={revision}") | ||
try: | ||
with revision.open() as f: | ||
body = yaml.safe_load(f) | ||
except yaml.YAMLError as e: | ||
log.error("Failed to load file=%s, %s", revision, e) | ||
raise snap_lib.SnapError(f"Failed to load file={revision}") | ||
dpkg_arch = ["dpkg", "--print-architecture"] | ||
arch = subprocess.check_output(dpkg_arch).decode("UTF-8").strip() | ||
Comment on lines
+92
to
+93
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 |
||
|
||
if not (isinstance(body, dict) and (arch_spec := body.get(arch))): | ||
log.warning("Failed to find revision for arch=%s", arch) | ||
raise snap_lib.SnapError(f"Failed to find revision for arch={arch}") | ||
|
||
try: | ||
args: List[SnapArgument] = [parse_obj_as(SnapArgument, arg) for arg in arch_spec] # type: ignore[arg-type] | ||
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. so, for every Since we made that nice annotated type we can use |
||
except ValidationError as e: | ||
log.warning("Failed to validate args=%s (%s)", arch_spec, e) | ||
raise snap_lib.SnapError("Failed to validate snap args") | ||
|
||
return args | ||
|
||
|
||
def management(): | ||
"""Manage snap installations on this machine.""" | ||
cache = snap_lib.SnapCache() | ||
for args in _parse_management_arguments(): | ||
which = cache[args.name] | ||
if isinstance(args, SnapFileArgument) and which.revision != "x1": | ||
snap_lib.install_local(**args.dict(exclude_none=True)) | ||
elif isinstance(args, SnapStoreArgument): | ||
log.info("Ensuring %s snap version", args.name) | ||
which.ensure(**args.dict(exclude_none=True)) | ||
Comment on lines
+111
to
+117
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. This We don't want to keep installing the local file over and over and over, -- so this checks for the revision 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. Definitely, let's add a TODO here |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
amd64: | ||
addyess marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- name: k8s | ||
channel: edge | ||
arm64: | ||
- name: k8s | ||
channel: edge |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
# Copyright 2024 Canonical Ltd. | ||
# See LICENSE file for licensing details. | ||
|
||
# Learn more about testing at: https://juju.is/docs/sdk/testing | ||
|
||
# pylint: disable=duplicate-code,missing-function-docstring | ||
"""Unit tests snap module.""" | ||
|
||
import io | ||
import unittest.mock as mock | ||
from pathlib import Path | ||
|
||
import pytest | ||
import snap | ||
|
||
|
||
@mock.patch("pathlib.Path.exists", mock.Mock(return_value=False)) | ||
def test_parse_no_file(): | ||
"""Test no file exists.""" | ||
with pytest.raises(snap.snap_lib.SnapError): | ||
snap._parse_management_arguments() | ||
|
||
|
||
@mock.patch("pathlib.Path.exists", mock.Mock(return_value=True)) | ||
@mock.patch("pathlib.Path.open") | ||
def test_parse_invalid_file(mock_open): | ||
"""Test file is invalid.""" | ||
mock_open().__enter__.return_value = io.StringIO("example: =") | ||
with pytest.raises(snap.snap_lib.SnapError): | ||
snap._parse_management_arguments() | ||
|
||
|
||
@mock.patch("pathlib.Path.exists", mock.Mock(return_value=True)) | ||
@mock.patch("pathlib.Path.open") | ||
@mock.patch("subprocess.check_output") | ||
def test_parse_invalid_arch(mock_checkoutput, mock_open): | ||
"""Test file has invalid arch.""" | ||
mock_open().__enter__.return_value = io.StringIO("{}") | ||
mock_checkoutput().decode.return_value = "amd64" | ||
with pytest.raises(snap.snap_lib.SnapError): | ||
snap._parse_management_arguments() | ||
|
||
|
||
@mock.patch("pathlib.Path.exists", mock.Mock(return_value=True)) | ||
@mock.patch("pathlib.Path.open") | ||
@mock.patch("subprocess.check_output") | ||
def test_parse_validation_error(mock_checkoutput, mock_open): | ||
"""Test file cannot be parsed.""" | ||
mock_open().__enter__.return_value = io.StringIO("amd64:\n- {}") | ||
mock_checkoutput().decode.return_value = "amd64" | ||
with pytest.raises(snap.snap_lib.SnapError): | ||
snap._parse_management_arguments() | ||
|
||
|
||
@mock.patch("pathlib.Path.exists", mock.Mock(return_value=True)) | ||
@mock.patch("pathlib.Path.open") | ||
@mock.patch("subprocess.check_output") | ||
def test_parse_valid_store(mock_checkoutput, mock_open): | ||
"""Test file parses as store content.""" | ||
content = """ | ||
amd64: | ||
- install-type: store | ||
name: k8s | ||
channel: edge | ||
""" | ||
mock_open().__enter__.return_value = io.StringIO(content) | ||
mock_checkoutput().decode.return_value = "amd64" | ||
args = snap._parse_management_arguments() | ||
assert args == [ | ||
snap.SnapStoreArgument(name="k8s", channel="edge"), | ||
] | ||
|
||
|
||
@mock.patch("pathlib.Path.exists", mock.Mock(return_value=True)) | ||
@mock.patch("pathlib.Path.open") | ||
@mock.patch("subprocess.check_output") | ||
def test_parse_valid_file(mock_checkoutput, mock_open): | ||
"""Test file parses as file content.""" | ||
content = """ | ||
amd64: | ||
- install-type: file | ||
name: k8s | ||
filename: path/to/thing | ||
""" | ||
mock_open().__enter__.return_value = io.StringIO(content) | ||
mock_checkoutput().decode.return_value = "amd64" | ||
args = snap._parse_management_arguments() | ||
assert args == [ | ||
snap.SnapFileArgument(name="k8s", filename=Path("path/to/thing")), | ||
] | ||
|
||
|
||
@mock.patch("snap._parse_management_arguments") | ||
@mock.patch("snap.snap_lib.install_local") | ||
@mock.patch("snap.snap_lib.SnapCache") | ||
def test_management_installs_local(cache, install_local, args): | ||
"""Test installer uses local installer.""" | ||
cache.return_value.__getitem__.return_value = mock.MagicMock(spec=snap.snap_lib.Snap) | ||
args.return_value = [ | ||
snap.SnapFileArgument(name="k8s", filename=Path("path/to/thing")), | ||
] | ||
snap.management() | ||
cache.called_once_with() | ||
cache["k8s"].ensure.assert_not_called() | ||
install_local.assert_called_once_with(filename=Path("path/to/thing")) | ||
|
||
|
||
@mock.patch("snap._parse_management_arguments") | ||
@mock.patch("snap.snap_lib.install_local") | ||
@mock.patch("snap.snap_lib.SnapCache") | ||
def test_management_installs_store(cache, install_local, args): | ||
"""Test installer uses store installer.""" | ||
cache.return_value.__getitem__.return_value = mock.MagicMock(spec=snap.snap_lib.Snap) | ||
args.return_value = [ | ||
snap.SnapStoreArgument(name="k8s", channel="edge"), | ||
] | ||
snap.management() | ||
cache.called_once_with() | ||
install_local.assert_not_called() | ||
cache()["k8s"].ensure.assert_called_once_with( | ||
state=snap.snap_lib.SnapState.Present, channel="edge" | ||
) |
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.
Nope, not going to use snap channels anymore