Replace snap channel config for hard coded revision config #79

addyess · 2024-04-23T22:15:53Z

Applicable spec:

Overview

Rather than encode the snap revisions into the charm code directly, include a file installed by the charm to specify the revision or channel of the snap. Each charm branch may adjust this file's contents in order to either install by channel or by revision.

Rationale

The same snap-installation.yaml will be available for both the built k8s and k8s-worker charm.
the revision file is arch aware to support different snap revisions based on the architecture for which its built
later CI actions can be used to automate the update of the revisions
the charm can be influenced via adjustments to this file to allow for arbitrary snap revision installation

Checklist

The charm style guide was applied
The contributing guide was applied
The changes are compliant with ISD054 - Managing Charm Complexity
The documentation is generated using src-docs
The documentation for charmhub is updated
The PR is tagged with appropriate label (urgent, trivial, complex)

github-actions

license-eye has checked 91 files.

Valid	Invalid	Ignored	Fixed
31	1	59	0

Click to see the invalid file list

charms/worker/k8s/templates/snap_installation.yaml

Use this command to fix any missing license headers

```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

charms/worker/k8s/templates/snap_installation.yaml

addyess · 2024-04-24T15:21:08Z

charms/worker/charmcraft.yaml

-    channel:
-      default: edge
-      type: string
-      description: Snap channel of the k8s snap


Nope, not going to use snap channels anymore

addyess · 2024-04-24T15:22:02Z

charms/worker/charmcraft.yaml

-      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


This moves the templates folder from ./k8s/templates to ./templates once installed on the k8s-worker units, along with the ./lib folder

addyess · 2024-04-24T15:22:41Z

charms/worker/k8s/charmcraft.yaml

-    channel:
-      default: edge
-      type: string
-      description: Snap channel of the k8s snap


Nope, revisions are hard coded

addyess · 2024-04-24T15:23:16Z

charms/worker/k8s/src/charm.py

-    @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()


let's move all the logic about managing the snap revision to another module

addyess · 2024-04-24T15:26:24Z

charms/worker/k8s/src/snap.py

+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


This looks like all the arguments in snap_lib.install_local(...) All the names and type mimic those arguments intentionally.

two arguments name and install_type do not exist as arguments to that method -- so they are marked with exclude=True. When this.dict(...) is called -- those arguments won't show up in the call

addyess · 2024-04-24T15:26:57Z

charms/worker/k8s/src/snap.py

+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


This looks like all the arguments in snap_lib.Snap.ensure(...) All the names and types mimic those arguments intentionally.

Again, the two arguments name and install_type do not exist as arguments to that method -- so they are marked with exclude=True. When this.dict(...) is called -- those arguments won't show up in the call

addyess · 2024-04-24T15:27:54Z

charms/worker/k8s/src/snap.py

+SnapArgument = Annotated[
+    Union[SnapFileArgument, SnapStoreArgument], Field(discriminator="install_type")
+]


This is pydantic's fancy way of creating a type which can parse data as either SnapFileArgument or SnapStoreArgument

the Field(discriminator="install_type") indicates to the parser how to determine which type of object it should be parsed as.

addyess · 2024-04-24T15:30:20Z

charms/worker/k8s/src/snap.py

+    dpkg_arch = ["dpkg", "--print-architecture"]
+    arch = subprocess.check_output(dpkg_arch).decode("UTF-8").strip()


the snap_installation.yaml will have sections based on architecture. each architecture can specify a list of snaps to install (not just one). This lets us have the flexibility to add multiple snaps in the future if necessary. Maybe we want to add kubelet with its own set of args. Or maybe by default install k9s or ponysay.

addyess · 2024-04-24T15:32:18Z

charms/worker/k8s/src/snap.py

+        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]


so, for every arg in the architecture specification (arch_spec) we want to parse the dict object into either a SnapFileArgument or a SnapStoreArgument.

Since we made that nice annotated type we can use parse_obj_as(...) asking pydantic to figure out which object type it is.

addyess · 2024-04-24T15:35:27Z

charms/worker/k8s/src/snap.py

+    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))


This management() will get called on every reconcile hook. We need to parse the snap_installation.yaml, determine the args, and either locally install or run ensure with the given arguments. Pydantic has validated the arguments match what's in snap_lib, so we can just turn the args back into a dict, and call the methods with keyword arguments.

We don't want to keep installing the local file over and over and over, -- so this checks for the revision x1 but that might not be the cleanest way. This would limit the number of times you can do a install_local to just one revision. Maybe there's a TODO needed here

Definitely, let's add a TODO here

addyess · 2024-04-24T15:36:11Z

charms/worker/k8s/templates/snap_installation.yaml

+- name: k8s
+  install-type: store
+  channel: edge
+arm64:
+- name: k8s
+  install-type: store
+  channel: edge


by default, the main branch will continue installing from the edge channel. Other branches will use specific snap revisions

mateoflorido

Amazing work! LGTM

mateoflorido · 2024-04-24T14:20:40Z

charms/worker/k8s/src/snap.py

+SnapArgument = Annotated[
+    Union[SnapFileArgument, SnapStoreArgument], Field(discriminator="install_type")


Nice! I love the discriminator logic here.

this is what we have to work with in pydantic 1.x. This has been changed in pydantic 2.x a bit
https://docs.pydantic.dev/latest/migration/#introduction-of-typeadapter

replace snap channel config for hard coded revision config

ae5c67d

github-actions bot reviewed Apr 23, 2024

View reviewed changes

charms/worker/k8s/templates/snap_installation.yaml Show resolved Hide resolved

Add license to snap_installation.yaml

d90653c

addyess changed the title ~~replace snap channel config for hard coded revision config~~ [WIP] replace snap channel config for hard coded revision config Apr 24, 2024

addyess added 2 commits April 23, 2024 21:50

set install-type as store

b06e6b6

Merge branch 'main' into KU-698/encode-snap-revisions

c03ed9d

addyess marked this pull request as ready for review April 24, 2024 13:58

addyess requested a review from a team as a code owner April 24, 2024 13:58

addyess added the enhancement New feature or request label Apr 24, 2024

addyess changed the title ~~[WIP] replace snap channel config for hard coded revision config~~ Replace snap channel config for hard coded revision config Apr 24, 2024

addyess commented Apr 24, 2024

View reviewed changes

mateoflorido approved these changes Apr 24, 2024

View reviewed changes

addyess merged commit 81caac3 into main Apr 24, 2024
34 checks passed

addyess deleted the KU-698/encode-snap-revisions branch April 24, 2024 15:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace snap channel config for hard coded revision config #79

Replace snap channel config for hard coded revision config #79

addyess commented Apr 23, 2024 •

edited

Loading

github-actions bot left a comment

addyess Apr 24, 2024

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024

addyess Apr 24, 2024

addyess Apr 24, 2024

addyess Apr 24, 2024

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024

mateoflorido Apr 24, 2024

addyess Apr 24, 2024

mateoflorido left a comment

mateoflorido Apr 24, 2024

addyess Apr 24, 2024

		dpkg_arch = ["dpkg", "--print-architecture"]
		arch = subprocess.check_output(dpkg_arch).decode("UTF-8").strip()

		SnapArgument = Annotated[
		Union[SnapFileArgument, SnapStoreArgument], Field(discriminator="install_type")

Replace snap channel config for hard coded revision config #79

Replace snap channel config for hard coded revision config #79

Conversation

addyess commented Apr 23, 2024 • edited Loading

Overview

Rationale

Checklist

github-actions bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addyess Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addyess Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

addyess Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

addyess Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateoflorido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addyess commented Apr 23, 2024 •

edited

Loading

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024 •

edited

Loading

addyess Apr 24, 2024 •

edited

Loading