Skip to content
Closed
Changes from all commits
Commits
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
31 changes: 21 additions & 10 deletions ocp_resources/data_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from ocp_resources.constants import TIMEOUT_4MINUTES
from ocp_resources.persistent_volume_claim import PersistentVolumeClaim
from ocp_resources.resource import NamespacedResource
from ocp_resources.volume_snapshot import VolumeSnapshot


class DataSource(NamespacedResource):
Expand Down Expand Up @@ -39,19 +40,29 @@ def to_dict(self):
},
})

@property
def pvc(self):
data_source_pvc = self.instance.spec.source.pvc
pvc_name = data_source_pvc.name
pvc_namespace = data_source_pvc.namespace
def _get_boot_source(self, boot_source_type):
boot_source = self.instance.spec.source.get(boot_source_type)
if not boot_source:
return None
boot_source_name = boot_source.name
boot_source_namespace = boot_source.namespace
try:
return PersistentVolumeClaim(
boot_source_object = PersistentVolumeClaim if boot_source_type == "pvc" else VolumeSnapshot
return boot_source_object(
client=self.client,
name=pvc_name,
namespace=pvc_namespace,
name=boot_source_name,
namespace=boot_source_namespace,
)
except ResourceNotFoundError:
self.logger.warning(
f"dataSource {self.name} is pointing to a non-existing PVC, name:"
f" {pvc_name}, namespace: {pvc_namespace}"
f"dataSource {self.name} is pointing to a non-existing {boot_source_type}, name:"
f" {boot_source_name}, namespace: {boot_source_namespace}"
)

@property
def pvc(self):
return self._get_boot_source(boot_source_type="pvc")

@property
def snapshot(self):
return self._get_boot_source(boot_source_type="snapshot")
Comment on lines +62 to +68
Copy link
Collaborator

Choose a reason for hiding this comment

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

boot_source should be the property that returns either a PVC or snapshot

Copy link
Collaborator

Choose a reason for hiding this comment

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

To clarify - add boot_source and add deprecation on pvc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify:

You are suggesting that the only property should be boot source,
And that it should return PersisentVolumeClaim or VolumeSnapshot dynamically based on the the instance boot source type?

Copy link
Collaborator

Choose a reason for hiding this comment

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

right, and add deprecation warning on pvc that it will be deprecated in 4.16.

Copy link
Contributor Author

@hmeir hmeir Jan 22, 2024

Choose a reason for hiding this comment

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

Hi

I have this PR in downstream.
And I want to apply this DataSource change on it as well, because its accessing its snapshot/pvc references.
Its currently on main, but need to be backported to 4.15 & 4.14.

So If I change to boot source instead of pvc, the implementation will be different in 4.14,4.15.
So should I implement the downstream PR with boot_source and in 4.14&4.15 add the snapshot property like I did here?