Skip to content
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.

Sign up for GitHub

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

fix: purge any cephfs storage classes installed by ops.manifest #38

Merged
merged 1 commit into from
Feb 11, 2025
Merged

fix: purge any cephfs storage classes installed by ops.manifest #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ops >= 1.2.0
ops.manifest >= 1.1.3, < 2.0
ops.manifest >= 1.5.0, < 2.0
jinja2
pyyaml

Expand Down
7 changes: 6 additions & 1 deletion src/manifests_cephfs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import logging
from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, cast

from lightkube.codecs import AnyResource
from lightkube.resources.core_v1 import Secret
Expand Down Expand Up @@ -174,6 +174,11 @@ def parameter_list(self) -> List[CephStorageClassParameters]:

def __call__(self) -> List[AnyResource]:
"""Craft the storage class object."""
if cast(SafeManifest, self.manifests).purgeable:
# If we are purging, we may not be able to create any storage classes
# Just return a fake storage class to satisfy delete_manifests method
# which will look up all storage classes installed by this app/manifest
return [StorageClass.from_dict(dict(metadata={}, provisioner=self.PROVISIONER))]
Comment on lines +177 to +181
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something I don't understand in this dunder method. Why do we need to create an empty sc when we are purging? Could there be any leftover sc, or is this just an edge case?

Copy link
Member Author

@addyess addyess Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manifest objects can support Manipulations. These are enacted by calling the Manipulation.

The types of manipulations are defined in the project

  • Subtraction
  • Patch
  • Addition

In this case. The CephStorageClass manipulation is an Addition. It is a set of objects added to the existing static manifests in the upstream yaml files. ops.manifest will call this manipulation to create the lightkube resource definition for the storage class.

Normally, for INSTALLING resources, this is straight forward to create these additions. ops.manifest knows the resources it should add. But in the purging situation, it's a bit different. We want to make sure that ops.manifest can delete all the objects this manifest may have added.

What's worse, is we don't REALLY know the name of the storage classes when we're purging. The state information necessary to generate the list of storage class names is gone (it was the cephfs filesystem listings).

But ops.manifest would have labelled each kubernetes resource with some metadata labels to indicate which manifest placed it into the cluster. In this case, if any storage class is created, it will have two labels associated with it:

- juju.io/application: ceph-csi-app-name # from the running application
- juju.io/manifest: cephfs  # from this module

What this __call__ is returning is a single list of StorageClasses with no names.

when ops.manifest builds its list of resources, it will query the cluster looking for all the StorageClasses with these matching labels and then delete those resources if they were found

return [self.create(class_param) for class_param in self.parameter_list()]


Expand Down
15 changes: 15 additions & 0 deletions tests/unit/test_manifests_cephfs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ def test_ceph_provisioner_adjustment_modelled(caplog):
def test_ceph_storage_class_modelled(caplog):
caplog.set_level(logging.INFO)
manifest = mock.MagicMock()
manifest.purgeable = False
csc = CephStorageClass(manifest)
alt_ns = "diff-ns"

Expand Down Expand Up @@ -140,6 +141,20 @@ def test_ceph_storage_class_modelled(caplog):
assert f"Modelling storage class sc='{TEST_CEPH_FS_ALT.name}'" in caplog.text


def test_ceph_storage_class_purgeable(caplog):
caplog.set_level(logging.INFO)
manifest = mock.MagicMock()
manifest.purgeable = True
csc = CephStorageClass(manifest)

caplog.clear()
expected = StorageClass(
metadata=ObjectMeta(),
provisioner=CephStorageClass.PROVISIONER,
)
assert csc() == [expected]


def test_manifest_evaluation(caplog):
caplog.set_level(logging.INFO)
charm = mock.MagicMock()
Expand Down