From a73514cd4f7fecbc89679566e0f8a0af16b6b06d Mon Sep 17 00:00:00 2001 From: Daniel Dowler <12484302+dandawg@users.noreply.github.com> Date: Thu, 19 Dec 2024 20:31:58 -0500 Subject: [PATCH] feat: Added pvc accessModes support (#4851) * added pvc accessModes support Signed-off-by: dandawg <12484302+dandawg@users.noreply.github.com> * made accessModes doc line more clear for users Signed-off-by: dandawg <12484302+dandawg@users.noreply.github.com> * Added multiple accessModes to PVC accessModes test Signed-off-by: dandawg <12484302+dandawg@users.noreply.github.com> * function for pvc.Create Signed-off-by: dandawg <12484302+dandawg@users.noreply.github.com> --------- Signed-off-by: dandawg <12484302+dandawg@users.noreply.github.com> --- .../api/v1alpha1/featurestore_types.go | 2 + .../api/v1alpha1/zz_generated.deepcopy.go | 5 +++ .../crd/bases/feast.dev_featurestores.yaml | 37 +++++++++++++++++++ infra/feast-operator/dist/install.yaml | 37 +++++++++++++++++++ .../featurestore_controller_pvc_test.go | 8 ++++ .../internal/controller/services/services.go | 2 +- .../controller/services/services_types.go | 10 +++-- .../internal/controller/services/util.go | 30 ++++++++++----- 8 files changed, 117 insertions(+), 14 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 84b4d8e841..2eb9ec8554 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -242,6 +242,8 @@ type PvcConfig struct { // The PVC name is the same as the associated deployment name. // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="PvcCreate is immutable" type PvcCreate struct { + // AccessModes k8s persistent volume access modes. Defaults to ["ReadWriteOnce"]. + AccessModes []corev1.PersistentVolumeAccessMode `json:"accessModes,omitempty"` // StorageClassName is the name of an existing StorageClass to which this persistent volume belongs. Empty value // means that this volume does not belong to any StorageClass and the cluster default will be used. StorageClassName *string `json:"storageClassName,omitempty"` diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 6cba8e5923..3241dff775 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -524,6 +524,11 @@ func (in *PvcConfig) DeepCopy() *PvcConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PvcCreate) DeepCopyInto(out *PvcCreate) { *out = *in + if in.AccessModes != nil { + in, out := &in.AccessModes, &out.AccessModes + *out = make([]v1.PersistentVolumeAccessMode, len(*in)) + copy(*out, *in) + } if in.StorageClassName != nil { in, out := &in.StorageClassName, &out.StorageClassName *out = new(string) diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml index 7fbd38ed31..fd8861cef1 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -254,6 +254,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent volume + access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -622,6 +628,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent volume + access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -1006,6 +1018,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -1506,6 +1524,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -1879,6 +1903,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -2272,6 +2302,13 @@ spec: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to + ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 73abc3717b..435be789e5 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -262,6 +262,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent volume + access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -630,6 +636,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent volume + access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -1014,6 +1026,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -1514,6 +1532,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -1887,6 +1911,12 @@ spec: create: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. @@ -2280,6 +2310,13 @@ spec: description: Settings for creating a new PVC properties: + accessModes: + description: AccessModes k8s persistent + volume access modes. Defaults to + ["ReadWriteOnce"]. + items: + type: string + type: array resources: description: |- Resources describes the storage resource requirements for a volume. diff --git a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go index fe0caa38e6..e64e5cd624 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -69,6 +69,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { onlineStoreMountPath := "/online" registryMountPath := "/registry" + accessModes := []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce, corev1.ReadWriteMany} storageClassName := "test" onlineStoreMountedPath := path.Join(onlineStoreMountPath, onlineStorePath) @@ -85,6 +86,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Type: offlineType, PvcConfig: &feastdevv1alpha1.PvcConfig{ Create: &feastdevv1alpha1.PvcCreate{ + AccessModes: accessModes, StorageClassName: &storageClassName, }, MountPath: offlineStoreMountPath, @@ -162,6 +164,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.Type).To(Equal(offlineType)) Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig).NotTo(BeNil()) Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.AccessModes).To(Equal(accessModes)) Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.StorageClassName).To(Equal(&storageClassName)) expectedResources := corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ @@ -179,6 +182,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(onlineStorePath)) Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig).NotTo(BeNil()) Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.AccessModes).To(Equal(services.DefaultPVCAccessModes)) Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.StorageClassName).To(BeNil()) expectedResources = corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ @@ -198,6 +202,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(registryPath)) Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig).NotTo(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.AccessModes).To(Equal(services.DefaultPVCAccessModes)) Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.StorageClassName).To(BeNil()) expectedResources = corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ @@ -283,6 +288,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) Expect(pvc.Spec.StorageClassName).To(Equal(&storageClassName)) + Expect(pvc.Spec.AccessModes).To(Equal(accessModes)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOfflineStorageRequest)) Expect(pvc.DeletionTimestamp).To(BeNil()) @@ -313,6 +319,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { pvc) Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) + Expect(pvc.Spec.AccessModes).To(Equal(services.DefaultPVCAccessModes)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOnlineStorageRequest)) Expect(pvc.DeletionTimestamp).To(BeNil()) @@ -343,6 +350,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { pvc) Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) + Expect(pvc.Spec.AccessModes).To(Equal(services.DefaultPVCAccessModes)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultRegistryStorageRequest)) Expect(pvc.DeletionTimestamp).To(BeNil()) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index f85597e648..232e3e5874 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -450,7 +450,7 @@ func (feast *FeastServices) createNewPVC(pvcCreate *feastdevv1alpha1.PvcCreate, pvc := feast.initPVC(feastType) pvc.Spec = corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, + AccessModes: pvcCreate.AccessModes, Resources: pvcCreate.Resources, } if pvcCreate.StorageClassName != nil { diff --git a/infra/feast-operator/internal/controller/services/services_types.go b/infra/feast-operator/internal/controller/services/services_types.go index b7c0f5f048..b9e1a9d9d7 100644 --- a/infra/feast-operator/internal/controller/services/services_types.go +++ b/infra/feast-operator/internal/controller/services/services_types.go @@ -20,6 +20,7 @@ import ( "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" handler "github.com/feast-dev/feast/infra/feast-operator/internal/controller/handler" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -80,10 +81,11 @@ const ( ) var ( - DefaultImage = "feastdev/feature-server:" + feastversion.FeastVersion - DefaultReplicas = int32(1) - NameLabelKey = feastdevv1alpha1.GroupVersion.Group + "/name" - ServiceTypeLabelKey = feastdevv1alpha1.GroupVersion.Group + "/service-type" + DefaultImage = "feastdev/feature-server:" + feastversion.FeastVersion + DefaultReplicas = int32(1) + DefaultPVCAccessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} + NameLabelKey = feastdevv1alpha1.GroupVersion.Group + "/name" + ServiceTypeLabelKey = feastdevv1alpha1.GroupVersion.Group + "/service-type" FeastServiceConstants = map[FeastServiceType]deploymentSettings{ OfflineFeastType: { diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index 631709d6ba..92ee2b5752 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -92,9 +92,7 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { if services.Registry.Local.Persistence.FilePersistence.PvcConfig != nil { pvc := services.Registry.Local.Persistence.FilePersistence.PvcConfig - if pvc.Create != nil { - ensureRequestedStorage(&pvc.Create.Resources, DefaultRegistryStorageRequest) - } + ensurePVCDefaults(pvc, RegistryFeastType) } } @@ -116,9 +114,7 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { if services.OfflineStore.Persistence.FilePersistence.PvcConfig != nil { pvc := services.OfflineStore.Persistence.FilePersistence.PvcConfig - if pvc.Create != nil { - ensureRequestedStorage(&pvc.Create.Resources, DefaultOfflineStorageRequest) - } + ensurePVCDefaults(pvc, OfflineFeastType) } } @@ -141,9 +137,7 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { if services.OnlineStore.Persistence.FilePersistence.PvcConfig != nil { pvc := services.OnlineStore.Persistence.FilePersistence.PvcConfig - if pvc.Create != nil { - ensureRequestedStorage(&pvc.Create.Resources, DefaultOnlineStorageRequest) - } + ensurePVCDefaults(pvc, OnlineFeastType) } } @@ -182,6 +176,24 @@ func ensureRequestedStorage(resources *v1.VolumeResourceRequirements, requestedS } } +func ensurePVCDefaults(pvc *feastdevv1alpha1.PvcConfig, feastType FeastServiceType) { + var storageRequest string + switch feastType { + case OnlineFeastType: + storageRequest = DefaultOnlineStorageRequest + case OfflineFeastType: + storageRequest = DefaultOfflineStorageRequest + case RegistryFeastType: + storageRequest = DefaultRegistryStorageRequest + } + if pvc.Create != nil { + ensureRequestedStorage(&pvc.Create.Resources, storageRequest) + if pvc.Create.AccessModes == nil { + pvc.Create.AccessModes = DefaultPVCAccessModes + } + } +} + func defaultOnlineStorePath(persistence *feastdevv1alpha1.OnlineStoreFilePersistence) string { if persistence.PvcConfig == nil { return DefaultOnlineStoreEphemeralPath