From ec7c8295ba60137db79276938b16dab012390684 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 8 Nov 2024 18:05:59 +0100 Subject: [PATCH 01/12] PVC configuration and impl Signed-off-by: Daniele Martinoli --- .../api/v1alpha1/featurestore_types.go | 42 +- .../api/v1alpha1/zz_generated.deepcopy.go | 67 +- .../crd/bases/feast.dev_featurestores.yaml | 485 ++++++++++++++ ...a1_featurestore_ephemeral_persistence.yaml | 3 + ...v1alpha1_featurestore_pvc_persistence.yaml | 45 ++ infra/feast-operator/dist/install.yaml | 485 ++++++++++++++ .../featurestore_controller_ephemeral_test.go | 482 ++++++++++++++ .../featurestore_controller_pvc_test.go | 612 ++++++++++++++++++ .../featurestore_controller_test.go | 455 +------------ .../controller/services/repo_config.go | 37 +- .../controller/services/repo_config_test.go | 2 +- .../internal/controller/services/services.go | 92 +++ .../controller/services/services_types.go | 20 +- .../internal/controller/services/util.go | 68 +- .../test/api/featurestore_types_test.go | 299 +++++++++ infra/feast-operator/test/api/suite_test.go | 89 +++ 16 files changed, 2807 insertions(+), 476 deletions(-) create mode 100644 infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml create mode 100644 infra/feast-operator/internal/controller/featurestore_controller_ephemeral_test.go create mode 100644 infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go create mode 100644 infra/feast-operator/test/api/featurestore_types_test.go create mode 100644 infra/feast-operator/test/api/suite_test.go diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index a8448571c04..ee491c2e592 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -82,7 +82,8 @@ type OfflineStorePersistence struct { // OfflineStorePersistence configures the file-based persistence for the offline store service type OfflineStoreFilePersistence struct { // +kubebuilder:validation:Enum=dask;duckdb - Type string `json:"type,omitempty"` + Type string `json:"type,omitempty"` + PvcConfig *PvcConfig `json:"pvc,omitempty"` } var ValidOfflineStoreFilePersistenceTypes = []string{ @@ -102,8 +103,11 @@ type OnlineStorePersistence struct { } // OnlineStoreFilePersistence configures the file-based persistence for the offline store service +// +kubebuilder:validation:XValidation:rule="(!has(self.pvc) && has(self.path)) ? self.path.startsWith('/') : true",message="Ephemeral stores must have absolute paths." +// +kubebuilder:validation:XValidation:rule="(has(self.pvc) && has(self.path)) ? !self.path.startsWith('/') : true",message="PVC path must be a file name only, with no slashes." type OnlineStoreFilePersistence struct { - Path string `json:"path,omitempty"` + Path string `json:"path,omitempty"` + PvcConfig *PvcConfig `json:"pvc,omitempty"` } // LocalRegistryConfig configures the deployed registry service @@ -118,8 +122,40 @@ type RegistryPersistence struct { } // RegistryFilePersistence configures the file-based persistence for the registry service +// +kubebuilder:validation:XValidation:rule="(!has(self.pvc) && has(self.path)) ? self.path.startsWith('/') : true",message="Ephemeral stores must have absolute paths." +// +kubebuilder:validation:XValidation:rule="(has(self.pvc) && has(self.path)) ? !self.path.startsWith('/') : true",message="PVC path must be a file name only, with no slashes." type RegistryFilePersistence struct { - Path string `json:"path,omitempty"` + Path string `json:"path,omitempty"` + PvcConfig *PvcConfig `json:"pvc,omitempty"` +} + +// PvcConfig defines the settings for a persistent file store based on PVCs. +// We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. +// +kubebuilder:validation:XValidation:rule="[has(self.ref), has(self.create)].exists_one(c, c)",message="One selection is required between ref and create." +// +kubebuilder:validation:XValidation:rule="self.mountPath.matches('^/[^:]*$')",message="Mount path must start with '/' and must not contain ':'" +type PvcConfig struct { + // Reference to an existing field + Ref *corev1.LocalObjectReference `json:"ref,omitempty"` + // Settings for creating a new PVC + Create *PvcCreate `json:"create,omitempty"` + // MountPath within the container at which the volume should be mounted. + // Must start by "/" and cannot contain ':'. + MountPath string `json:"mountPath,omitempty"` +} + +// PvcCreate defines the immutable settings to create a new PVC mounted at the given path. +// The PVC name is the same as the associated deployment name. +// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="PvcCreate is immutable" +type PvcCreate struct { + // 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"` + // Resources describes the storage resource requirements for a volume. + // Default requested storage size depends on the associated service: + // - 10Gi for offline store + // - 5Gi for online store + // - 5Gi for registry + Resources corev1.VolumeResourceRequirements `json:"resources,omitempty"` } // Registry configures the registry service. One selection is required. Local is the default setting. diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 6ff9957bb84..59f44c406cf 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -239,6 +239,11 @@ func (in *OfflineStore) DeepCopy() *OfflineStore { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OfflineStoreFilePersistence) DeepCopyInto(out *OfflineStoreFilePersistence) { *out = *in + if in.PvcConfig != nil { + in, out := &in.PvcConfig, &out.PvcConfig + *out = new(PvcConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OfflineStoreFilePersistence. @@ -257,7 +262,7 @@ func (in *OfflineStorePersistence) DeepCopyInto(out *OfflineStorePersistence) { if in.FilePersistence != nil { in, out := &in.FilePersistence, &out.FilePersistence *out = new(OfflineStoreFilePersistence) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -295,6 +300,11 @@ func (in *OnlineStore) DeepCopy() *OnlineStore { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OnlineStoreFilePersistence) DeepCopyInto(out *OnlineStoreFilePersistence) { *out = *in + if in.PvcConfig != nil { + in, out := &in.PvcConfig, &out.PvcConfig + *out = new(PvcConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OnlineStoreFilePersistence. @@ -313,7 +323,7 @@ func (in *OnlineStorePersistence) DeepCopyInto(out *OnlineStorePersistence) { if in.FilePersistence != nil { in, out := &in.FilePersistence, &out.FilePersistence *out = new(OnlineStoreFilePersistence) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -363,6 +373,52 @@ func (in *OptionalConfigs) DeepCopy() *OptionalConfigs { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PvcConfig) DeepCopyInto(out *PvcConfig) { + *out = *in + if in.Ref != nil { + in, out := &in.Ref, &out.Ref + *out = new(v1.LocalObjectReference) + **out = **in + } + if in.Create != nil { + in, out := &in.Create, &out.Create + *out = new(PvcCreate) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PvcConfig. +func (in *PvcConfig) DeepCopy() *PvcConfig { + if in == nil { + return nil + } + out := new(PvcConfig) + in.DeepCopyInto(out) + return out +} + +// 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.StorageClassName != nil { + in, out := &in.StorageClassName, &out.StorageClassName + *out = new(string) + **out = **in + } + in.Resources.DeepCopyInto(&out.Resources) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PvcCreate. +func (in *PvcCreate) DeepCopy() *PvcCreate { + if in == nil { + return nil + } + out := new(PvcCreate) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Registry) DeepCopyInto(out *Registry) { *out = *in @@ -391,6 +447,11 @@ func (in *Registry) DeepCopy() *Registry { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RegistryFilePersistence) DeepCopyInto(out *RegistryFilePersistence) { *out = *in + if in.PvcConfig != nil { + in, out := &in.PvcConfig, &out.PvcConfig + *out = new(PvcConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryFilePersistence. @@ -409,7 +470,7 @@ func (in *RegistryPersistence) DeepCopyInto(out *RegistryPersistence) { if in.FilePersistence != nil { in, out := &in.FilePersistence, &out.FilePersistence *out = new(RegistryFilePersistence) - **out = **in + (*in).DeepCopyInto(*out) } } 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 a6d4551da5c..5e470296a9e 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -189,6 +189,81 @@ spec: description: OfflineStorePersistence configures the file-based persistence for the offline store service properties: + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref and + create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and must + not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: enum: - dask @@ -386,7 +461,90 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref and + create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and must + not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute paths. + rule: '(!has(self.pvc) && has(self.path)) ? self.path.startsWith(''/'') + : true' + - message: PVC path must be a file name only, with no + slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute resource @@ -583,7 +741,90 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref + and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and + must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute paths. + rule: '(!has(self.pvc) && has(self.path)) ? self.path.startsWith(''/'') + : true' + - message: PVC path must be a file name only, with + no slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute @@ -827,6 +1068,81 @@ spec: description: OfflineStorePersistence configures the file-based persistence for the offline store service properties: + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref + and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and + must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: enum: - dask @@ -1027,7 +1343,90 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref + and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and + must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute paths. + rule: '(!has(self.pvc) && has(self.path)) ? self.path.startsWith(''/'') + : true' + - message: PVC path must be a file name only, with + no slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute @@ -1229,7 +1628,93 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new + PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing + field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between + ref and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' + and must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute + paths. + rule: '(!has(self.pvc) && has(self.path)) ? + self.path.startsWith(''/'') : true' + - message: PVC path must be a file name only, + with no slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml index 512fed9d4c0..8449ec1bf51 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml @@ -6,15 +6,18 @@ spec: feastProject: my_project services: onlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/online_store.db offlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: type: dask registry: local: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/registry.db diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml new file mode 100644 index 00000000000..d457a748822 --- /dev/null +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml @@ -0,0 +1,45 @@ +apiVersion: feast.dev/v1alpha1 +kind: FeatureStore +metadata: + name: sample-pvc-persistence +spec: + feastProject: my_project + services: + onlineStore: + persistence: + file: + path: online_store.db + pvc: + ref: + name: online-pvc + mountPath: /data/online + offlineStore: + persistence: + file: + type: duckdb + pvc: + create: + storageClassName: standard + resources: + requests: + storage: 5Gi + mountPath: /data/offline + registry: + local: + persistence: + file: + path: registry.db + pvc: + create: {} + mountPath: /data/registryma +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: online-pvc +spec: + accessModes: + - ReadWriteMany + resources: + requests: + storage: 5Gi diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index ceb6b34e5c4..69d3df45342 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -197,6 +197,81 @@ spec: description: OfflineStorePersistence configures the file-based persistence for the offline store service properties: + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref and + create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and must + not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: enum: - dask @@ -394,7 +469,90 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref and + create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and must + not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute paths. + rule: '(!has(self.pvc) && has(self.path)) ? self.path.startsWith(''/'') + : true' + - message: PVC path must be a file name only, with no + slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute resource @@ -591,7 +749,90 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref + and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and + must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute paths. + rule: '(!has(self.pvc) && has(self.path)) ? self.path.startsWith(''/'') + : true' + - message: PVC path must be a file name only, with + no slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute @@ -835,6 +1076,81 @@ spec: description: OfflineStorePersistence configures the file-based persistence for the offline store service properties: + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref + and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and + must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: enum: - dask @@ -1035,7 +1351,90 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between ref + and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' and + must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute paths. + rule: '(!has(self.pvc) && has(self.path)) ? self.path.startsWith(''/'') + : true' + - message: PVC path must be a file name only, with + no slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute @@ -1237,7 +1636,93 @@ spec: properties: path: type: string + pvc: + description: |- + PvcConfig defines the settings for a persistent file store based on PVCs. + We can refer to an existing PVC using the `Ref` field, or create a new one using the `Create` field. + properties: + create: + description: Settings for creating a new + PVC + properties: + resources: + description: |- + Resources describes the storage resource requirements for a volume. + Default requested storage size depends on the associated service: + - 10Gi for offline store + - 5Gi for online store + - 5Gi for registry + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + storageClassName: + description: |- + 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. + type: string + type: object + x-kubernetes-validations: + - message: PvcCreate is immutable + rule: self == oldSelf + mountPath: + description: |- + MountPath within the container at which the volume should be mounted. + Must start by "/" and cannot contain ':'. + type: string + ref: + description: Reference to an existing + field + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: One selection is required between + ref and create. + rule: '[has(self.ref), has(self.create)].exists_one(c, + c)' + - message: Mount path must start with '/' + and must not contain ':' + rule: self.mountPath.matches('^/[^:]*$') type: object + x-kubernetes-validations: + - message: Ephemeral stores must have absolute + paths. + rule: '(!has(self.pvc) && has(self.path)) ? + self.path.startsWith(''/'') : true' + - message: PVC path must be a file name only, + with no slashes. + rule: '(has(self.pvc) && has(self.path)) ? !self.path.startsWith(''/'') + : true' type: object resources: description: ResourceRequirements describes the compute diff --git a/infra/feast-operator/internal/controller/featurestore_controller_ephemeral_test.go b/infra/feast-operator/internal/controller/featurestore_controller_ephemeral_test.go new file mode 100644 index 00000000000..bd597f987ca --- /dev/null +++ b/infra/feast-operator/internal/controller/featurestore_controller_ephemeral_test.go @@ -0,0 +1,482 @@ +/* +Copyright 2024 Feast Community. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/base64" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "gopkg.in/yaml.v3" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" +) + +var _ = Describe("FeatureStore Controller-Ephemeral services", func() { + Context("When deploying a resource with all ephemeral services", func() { + const resourceName = "services-ephemeral" + var pullPolicy = corev1.PullAlways + var testEnvVarName = "testEnvVarName" + var testEnvVarValue = "testEnvVarValue" + + ctx := context.Background() + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", + } + featurestore := &feastdevv1alpha1.FeatureStore{} + onlineStorePath := "/data/online.db" + registryPath := "/data/registry.db" + offlineType := "duckdb" + + BeforeEach(func() { + By("creating the custom resource for the Kind FeatureStore") + err := k8sClient.Get(ctx, typeNamespacedName, featurestore) + if err != nil && errors.IsNotFound(err) { + resource := createFeatureStoreResource(resourceName, image, pullPolicy, &[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, + {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}}) + resource.Spec.Services.OfflineStore.Persistence = &feastdevv1alpha1.OfflineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ + Type: offlineType, + }, + } + resource.Spec.Services.OnlineStore.Persistence = &feastdevv1alpha1.OnlineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OnlineStoreFilePersistence{ + Path: onlineStorePath, + }, + } + resource.Spec.Services.Registry = &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Persistence: &feastdevv1alpha1.RegistryPersistence{ + FilePersistence: &feastdevv1alpha1.RegistryFilePersistence{ + Path: registryPath, + }, + }, + }, + } + + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + }) + AfterEach(func() { + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance FeatureStore") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + }) + + It("should successfully reconcile the resource", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + feast := services.FeastServices{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + } + Expect(resource.Status).NotTo(BeNil()) + Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion)) + Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType))) + Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject)) + Expect(resource.Status.Applied.Services).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.Type).To(Equal(offlineType)) + Expect(resource.Status.Applied.Services.OfflineStore.ImagePullPolicy).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Resources).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Image).To(Equal(&services.DefaultImage)) + Expect(resource.Status.Applied.Services.OnlineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(onlineStorePath)) + Expect(resource.Status.Applied.Services.OnlineStore.Env).To(Equal(&[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}})) + Expect(resource.Status.Applied.Services.OnlineStore.ImagePullPolicy).To(Equal(&pullPolicy)) + Expect(resource.Status.Applied.Services.OnlineStore.Resources).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Image).To(Equal(&image)) + Expect(resource.Status.Applied.Services.Registry).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(registryPath)) + Expect(resource.Status.Applied.Services.Registry.Local.ImagePullPolicy).To(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Resources).To(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Image).To(Equal(&services.DefaultImage)) + + Expect(resource.Status.ServiceHostnames.OfflineStore).To(Equal(feast.GetFeastServiceName(services.OfflineFeastType) + "." + resource.Namespace + domain)) + Expect(resource.Status.ServiceHostnames.OnlineStore).To(Equal(feast.GetFeastServiceName(services.OnlineFeastType) + "." + resource.Namespace + domain)) + Expect(resource.Status.ServiceHostnames.Registry).To(Equal(feast.GetFeastServiceName(services.RegistryFeastType) + "." + resource.Namespace + domain)) + + Expect(resource.Status.Conditions).NotTo(BeEmpty()) + cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ClientReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ClientReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ClientReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OfflineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OfflineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OfflineStoreReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OnlineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OnlineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OnlineStoreReadyMessage)) + + Expect(resource.Status.Phase).To(Equal(feastdevv1alpha1.ReadyPhase)) + + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + + svc := &corev1.Service{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + svc) + Expect(err).NotTo(HaveOccurred()) + Expect(controllerutil.HasControllerReference(svc)).To(BeTrue()) + Expect(svc.Spec.Ports[0].TargetPort).To(Equal(intstr.FromInt(int(services.FeastServiceConstants[services.RegistryFeastType].TargetPort)))) + }) + + It("should properly encode a feature_store.yaml config", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + req, err := labels.NewRequirement(services.NameLabelKey, selection.Equals, []string{resource.Name}) + Expect(err).NotTo(HaveOccurred()) + labelSelector := labels.NewSelector().Add(*req) + listOpts := &client.ListOptions{Namespace: resource.Namespace, LabelSelector: labelSelector} + deployList := appsv1.DeploymentList{} + err = k8sClient.List(ctx, &deployList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(deployList.Items).To(HaveLen(3)) + + svcList := corev1.ServiceList{} + err = k8sClient.List(ctx, &svcList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(svcList.Items).To(HaveLen(3)) + + cmList := corev1.ConfigMapList{} + err = k8sClient.List(ctx, &cmList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(cmList.Items).To(HaveLen(1)) + + feast := services.FeastServices{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + } + + // check registry config + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(1)) + env := getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err := feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err := base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + testConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + Registry: services.RegistryConfig{ + RegistryType: services.RegistryFileConfigType, + Path: registryPath, + }, + } + Expect(repoConfig).To(Equal(testConfig)) + + // check offline config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(1)) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfigOffline := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOffline) + Expect(err).NotTo(HaveOccurred()) + regRemote := services.RegistryConfig{ + RegistryType: services.RegistryRemoteConfigType, + Path: fmt.Sprintf("feast-%s-registry.default.svc.cluster.local:80", resourceName), + } + offlineConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: services.OfflineStoreConfig{ + Type: services.OfflineDuckDbConfigType, + }, + Registry: regRemote, + } + Expect(repoConfigOffline).To(Equal(offlineConfig)) + + // check online config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(3)) + Expect(deploy.Spec.Template.Spec.Containers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfigOnline := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOnline) + Expect(err).NotTo(HaveOccurred()) + offlineRemote := services.OfflineStoreConfig{ + Host: fmt.Sprintf("feast-%s-offline.default.svc.cluster.local", resourceName), + Type: services.OfflineRemoteConfigType, + Port: services.HttpPort, + } + onlineConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: offlineRemote, + OnlineStore: services.OnlineStoreConfig{ + Path: onlineStorePath, + Type: services.OnlineSqliteConfigType, + }, + Registry: regRemote, + } + Expect(repoConfigOnline).To(Equal(onlineConfig)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(3)) + + // check client config + cm := &corev1.ConfigMap{} + name := feast.GetFeastServiceName(services.ClientFeastType) + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: name, + Namespace: resource.Namespace, + }, + cm) + Expect(err).NotTo(HaveOccurred()) + repoConfigClient := &services.RepoConfig{} + err = yaml.Unmarshal([]byte(cm.Data[services.FeatureStoreYamlCmKey]), repoConfigClient) + Expect(err).NotTo(HaveOccurred()) + clientConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: offlineRemote, + OnlineStore: services.OnlineStoreConfig{ + Path: fmt.Sprintf("http://feast-%s-online.default.svc.cluster.local:80", resourceName), + Type: services.OnlineRemoteConfigType, + }, + Registry: regRemote, + } + Expect(repoConfigClient).To(Equal(clientConfig)) + + // change paths and reconcile + resourceNew := resource.DeepCopy() + newOnlineStorePath := "/data/new_online.db" + newRegistryPath := "/data/new_registry.db" + resourceNew.Spec.Services.OnlineStore.Persistence.FilePersistence.Path = newOnlineStorePath + resourceNew.Spec.Services.Registry.Local.Persistence.FilePersistence.Path = newRegistryPath + err = k8sClient.Update(ctx, resourceNew) + Expect(err).NotTo(HaveOccurred()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource = &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + feast.FeatureStore = resource + + // check registry config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + testConfig.Registry.Path = newRegistryPath + Expect(repoConfig).To(Equal(testConfig)) + + // check offline config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfigOffline = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOffline) + Expect(err).NotTo(HaveOccurred()) + Expect(repoConfigOffline).To(Equal(offlineConfig)) + + // check online config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + + repoConfigOnline = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOnline) + Expect(err).NotTo(HaveOccurred()) + onlineConfig.OnlineStore.Path = newOnlineStorePath + Expect(repoConfigOnline).To(Equal(onlineConfig)) + }) + }) +}) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go new file mode 100644 index 00000000000..edd0d8fffff --- /dev/null +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -0,0 +1,612 @@ +/* +Copyright 2024 Feast Community. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/base64" + "fmt" + "path" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "gopkg.in/yaml.v3" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" +) + +var _ = Describe("FeatureStore Controller-Ephemeral services", func() { + Context("When deploying a resource with all ephemeral services", func() { + const resourceName = "services-ephemeral" + var pullPolicy = corev1.PullAlways + var testEnvVarName = "testEnvVarName" + var testEnvVarValue = "testEnvVarValue" + + ctx := context.Background() + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", + } + featurestore := &feastdevv1alpha1.FeatureStore{} + onlineStorePath := "online.db" + registryPath := "registry.db" + offlineType := "duckdb" + + offlineStoreMountPath := "/offline" + onlineStoreMountPath := "/online" + registryMountPath := "/registry" + + onlineStoreMountedPath := path.Join(onlineStoreMountPath, onlineStorePath) + registryMountedPath := path.Join(registryMountPath, registryPath) + + BeforeEach(func() { + By("creating the custom resource for the Kind FeatureStore") + err := k8sClient.Get(ctx, typeNamespacedName, featurestore) + if err != nil && errors.IsNotFound(err) { + resource := createFeatureStoreResource(resourceName, image, pullPolicy, &[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, + {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}}) + resource.Spec.Services.OfflineStore.Persistence = &feastdevv1alpha1.OfflineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ + Type: offlineType, + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Create: &feastdevv1alpha1.PvcCreate{}, + MountPath: offlineStoreMountPath, + }, + }, + } + resource.Spec.Services.OnlineStore.Persistence = &feastdevv1alpha1.OnlineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OnlineStoreFilePersistence{ + Path: onlineStorePath, + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Create: &feastdevv1alpha1.PvcCreate{}, + MountPath: onlineStoreMountPath, + }, + }, + } + resource.Spec.Services.Registry = &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Persistence: &feastdevv1alpha1.RegistryPersistence{ + FilePersistence: &feastdevv1alpha1.RegistryFilePersistence{ + Path: registryPath, + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Create: &feastdevv1alpha1.PvcCreate{}, + MountPath: registryMountPath, + }, + }, + }, + }, + } + + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + }) + AfterEach(func() { + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance FeatureStore") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + }) + + It("should successfully reconcile the resource", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + feast := services.FeastServices{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + } + Expect(resource.Status).NotTo(BeNil()) + Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion)) + Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType))) + Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject)) + Expect(resource.Status.Applied.Services).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.Type).To(Equal(offlineType)) + Expect(resource.Status.Applied.Services.OfflineStore.ImagePullPolicy).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Resources).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Image).To(Equal(&services.DefaultImage)) + Expect(resource.Status.Applied.Services.OnlineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(onlineStorePath)) + Expect(resource.Status.Applied.Services.OnlineStore.Env).To(Equal(&[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}})) + Expect(resource.Status.Applied.Services.OnlineStore.ImagePullPolicy).To(Equal(&pullPolicy)) + Expect(resource.Status.Applied.Services.OnlineStore.Resources).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Image).To(Equal(&image)) + Expect(resource.Status.Applied.Services.Registry).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(registryPath)) + Expect(resource.Status.Applied.Services.Registry.Local.ImagePullPolicy).To(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Resources).To(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Image).To(Equal(&services.DefaultImage)) + + Expect(resource.Status.ServiceHostnames.OfflineStore).To(Equal(feast.GetFeastServiceName(services.OfflineFeastType) + "." + resource.Namespace + domain)) + Expect(resource.Status.ServiceHostnames.OnlineStore).To(Equal(feast.GetFeastServiceName(services.OnlineFeastType) + "." + resource.Namespace + domain)) + Expect(resource.Status.ServiceHostnames.Registry).To(Equal(feast.GetFeastServiceName(services.RegistryFeastType) + "." + resource.Namespace + domain)) + + Expect(resource.Status.Conditions).NotTo(BeEmpty()) + cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ClientReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ClientReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ClientReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OfflineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OfflineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OfflineStoreReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OnlineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OnlineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OnlineStoreReadyMessage)) + + Expect(resource.Status.Phase).To(Equal(feastdevv1alpha1.ReadyPhase)) + + // check offline deployment + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes[0].Name).To(Equal(deploy.Name)) + Expect(deploy.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim.ClaimName).To(Equal(deploy.Name)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath).To(Equal(offlineStoreMountPath)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name).To(Equal(deploy.Name)) + + // check offline pvc + pvc := &corev1.PersistentVolumeClaim{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: deploy.Name, + Namespace: resource.Namespace, + }, + pvc) + Expect(err).NotTo(HaveOccurred()) + Expect(pvc.Name).To(Equal(deploy.Name)) + Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOfflineStorageRequest)) + + // check online deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes[0].Name).To(Equal(deploy.Name)) + Expect(deploy.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim.ClaimName).To(Equal(deploy.Name)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath).To(Equal(onlineStoreMountPath)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name).To(Equal(deploy.Name)) + + // check online pvc + pvc = &corev1.PersistentVolumeClaim{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: deploy.Name, + Namespace: resource.Namespace, + }, + pvc) + Expect(err).NotTo(HaveOccurred()) + Expect(pvc.Name).To(Equal(deploy.Name)) + Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOnlineStorageRequest)) + + // check registry deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes[0].Name).To(Equal(deploy.Name)) + Expect(deploy.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim.ClaimName).To(Equal(deploy.Name)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath).To(Equal(registryMountPath)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name).To(Equal(deploy.Name)) + + // check registry pvc + pvc = &corev1.PersistentVolumeClaim{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: deploy.Name, + Namespace: resource.Namespace, + }, + pvc) + Expect(err).NotTo(HaveOccurred()) + Expect(pvc.Name).To(Equal(deploy.Name)) + Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultRegistryStorageRequest)) + + // remove online PVC and reconcile + resourceNew := resource.DeepCopy() + newOnlineStorePath := "/tmp/new_online.db" + resourceNew.Spec.Services.OnlineStore.Persistence.FilePersistence.Path = newOnlineStorePath + resourceNew.Spec.Services.OnlineStore.Persistence.FilePersistence.PvcConfig = nil + err = k8sClient.Update(ctx, resourceNew) + Expect(err).NotTo(HaveOccurred()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource = &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + feast.FeatureStore = resource + + // check online deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(0)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(0)) + + // check online pvc is deleted + pvc = &corev1.PersistentVolumeClaim{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: deploy.Name, + Namespace: resource.Namespace, + }, + pvc) + print(err) + // Expect(err).To(HaveOccurred()) + // Expect(errors.IsNotFound(err)).To(BeTrue()) + }) + + It("should properly encode a feature_store.yaml config", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + req, err := labels.NewRequirement(services.NameLabelKey, selection.Equals, []string{resource.Name}) + Expect(err).NotTo(HaveOccurred()) + labelSelector := labels.NewSelector().Add(*req) + listOpts := &client.ListOptions{Namespace: resource.Namespace, LabelSelector: labelSelector} + deployList := appsv1.DeploymentList{} + err = k8sClient.List(ctx, &deployList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(deployList.Items).To(HaveLen(3)) + + svcList := corev1.ServiceList{} + err = k8sClient.List(ctx, &svcList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(svcList.Items).To(HaveLen(3)) + + cmList := corev1.ConfigMapList{} + err = k8sClient.List(ctx, &cmList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(cmList.Items).To(HaveLen(1)) + + feast := services.FeastServices{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + } + + // check registry deployment + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(1)) + env := getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + // check registry config + fsYamlStr, err := feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err := base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + testConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + Registry: services.RegistryConfig{ + RegistryType: services.RegistryFileConfigType, + Path: registryMountedPath, + }, + } + Expect(repoConfig).To(Equal(testConfig)) + + // check offline deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(1)) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + // check offline config + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfigOffline := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOffline) + Expect(err).NotTo(HaveOccurred()) + regRemote := services.RegistryConfig{ + RegistryType: services.RegistryRemoteConfigType, + Path: fmt.Sprintf("feast-%s-registry.default.svc.cluster.local:80", resourceName), + } + offlineConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: services.OfflineStoreConfig{ + Type: services.OfflineDuckDbConfigType, + }, + Registry: regRemote, + } + Expect(repoConfigOffline).To(Equal(offlineConfig)) + + // check online config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(3)) + Expect(deploy.Spec.Template.Spec.Containers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfigOnline := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOnline) + Expect(err).NotTo(HaveOccurred()) + offlineRemote := services.OfflineStoreConfig{ + Host: fmt.Sprintf("feast-%s-offline.default.svc.cluster.local", resourceName), + Type: services.OfflineRemoteConfigType, + Port: services.HttpPort, + } + onlineConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: offlineRemote, + OnlineStore: services.OnlineStoreConfig{ + Path: onlineStoreMountedPath, + Type: services.OnlineSqliteConfigType, + }, + Registry: regRemote, + } + Expect(repoConfigOnline).To(Equal(onlineConfig)) + Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(3)) + + // check client config + cm := &corev1.ConfigMap{} + name := feast.GetFeastServiceName(services.ClientFeastType) + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: name, + Namespace: resource.Namespace, + }, + cm) + Expect(err).NotTo(HaveOccurred()) + repoConfigClient := &services.RepoConfig{} + err = yaml.Unmarshal([]byte(cm.Data[services.FeatureStoreYamlCmKey]), repoConfigClient) + Expect(err).NotTo(HaveOccurred()) + clientConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: offlineRemote, + OnlineStore: services.OnlineStoreConfig{ + Path: fmt.Sprintf("http://feast-%s-online.default.svc.cluster.local:80", resourceName), + Type: services.OnlineRemoteConfigType, + }, + Registry: regRemote, + } + Expect(repoConfigClient).To(Equal(clientConfig)) + + // change paths and reconcile + resourceNew := resource.DeepCopy() + newOnlineStorePath := "new_online.db" + newRegistryPath := "new_registry.db" + + newOnlineStoreMountedPath := path.Join(onlineStoreMountPath, newOnlineStorePath) + newRegistryMountedPath := path.Join(registryMountPath, newRegistryPath) + + resourceNew.Spec.Services.OnlineStore.Persistence.FilePersistence.Path = newOnlineStorePath + resourceNew.Spec.Services.Registry.Local.Persistence.FilePersistence.Path = newRegistryPath + err = k8sClient.Update(ctx, resourceNew) + Expect(err).NotTo(HaveOccurred()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource = &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + feast.FeatureStore = resource + + // check registry config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + testConfig.Registry.Path = newRegistryMountedPath + Expect(repoConfig).To(Equal(testConfig)) + + // check offline config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfigOffline = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOffline) + Expect(err).NotTo(HaveOccurred()) + Expect(repoConfigOffline).To(Equal(offlineConfig)) + + // check online config + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + + repoConfigOnline = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfigOnline) + Expect(err).NotTo(HaveOccurred()) + onlineConfig.OnlineStore.Path = newOnlineStoreMountedPath + Expect(repoConfigOnline).To(Equal(onlineConfig)) + }) + }) +}) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index aab260d9991..14cac19600d 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -19,7 +19,6 @@ package controller import ( "context" "encoding/base64" - "fmt" "reflect" . "github.com/onsi/ginkgo/v2" @@ -44,6 +43,9 @@ import ( ) const feastProject = "test_project" +const domain = ".svc.cluster.local:80" + +var image = "test:latest" var _ = Describe("FeatureStore Controller", func() { Context("When reconciling a resource", func() { @@ -234,7 +236,7 @@ var _ = Describe("FeatureStore Controller", func() { EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, Registry: services.RegistryConfig{ RegistryType: services.RegistryFileConfigType, - Path: services.DefaultRegistryPath, + Path: services.DefaultRegistryEphemeralPath, }, } Expect(repoConfig).To(Equal(testConfig)) @@ -383,7 +385,6 @@ var _ = Describe("FeatureStore Controller", func() { Context("When reconciling a resource with all services enabled", func() { const resourceName = "services" - image := "test:latest" var pullPolicy = corev1.PullAlways var testEnvVarName = "testEnvVarName" var testEnvVarValue = "testEnvVarValue" @@ -451,7 +452,7 @@ var _ = Describe("FeatureStore Controller", func() { Expect(resource.Status.Applied.Services.OnlineStore).NotTo(BeNil()) Expect(resource.Status.Applied.Services.OnlineStore.Persistence).NotTo(BeNil()) Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(services.DefaultOnlinePath)) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(services.DefaultOnlineStoreEphemeralPath)) Expect(resource.Status.Applied.Services.OnlineStore.Env).To(Equal(&[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}})) Expect(resource.Status.Applied.Services.OnlineStore.ImagePullPolicy).To(Equal(&pullPolicy)) Expect(resource.Status.Applied.Services.OnlineStore.Resources).NotTo(BeNil()) @@ -460,12 +461,11 @@ var _ = Describe("FeatureStore Controller", func() { Expect(resource.Status.Applied.Services.Registry.Local).NotTo(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Persistence).NotTo(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(services.DefaultRegistryPath)) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(services.DefaultRegistryEphemeralPath)) Expect(resource.Status.Applied.Services.Registry.Local.ImagePullPolicy).To(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Resources).To(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Image).To(Equal(&services.DefaultImage)) - domain := ".svc.cluster.local:80" Expect(resource.Status.ServiceHostnames.OfflineStore).To(Equal(feast.GetFeastServiceName(services.OfflineFeastType) + "." + resource.Namespace + domain)) Expect(resource.Status.ServiceHostnames.OnlineStore).To(Equal(feast.GetFeastServiceName(services.OnlineFeastType) + "." + resource.Namespace + domain)) Expect(resource.Status.ServiceHostnames.Registry).To(Equal(feast.GetFeastServiceName(services.RegistryFeastType) + "." + resource.Namespace + domain)) @@ -600,7 +600,7 @@ var _ = Describe("FeatureStore Controller", func() { EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, Registry: services.RegistryConfig{ RegistryType: services.RegistryFileConfigType, - Path: services.DefaultRegistryPath, + Path: services.DefaultRegistryEphemeralPath, }, } Expect(repoConfig).To(Equal(testConfig)) @@ -676,7 +676,7 @@ var _ = Describe("FeatureStore Controller", func() { EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, OfflineStore: offlineRemote, OnlineStore: services.OnlineStoreConfig{ - Path: services.DefaultOnlinePath, + Path: services.DefaultOnlineStoreEphemeralPath, Type: services.OnlineSqliteConfigType, }, Registry: regRemote, @@ -1197,445 +1197,6 @@ var _ = Describe("FeatureStore Controller", func() { Expect(err).NotTo(HaveOccurred()) }) }) - - Context("When deploying a resource with all ephemeral services", func() { - const resourceName = "services-ephemeral" - image := "test:latest" - var pullPolicy = corev1.PullAlways - var testEnvVarName = "testEnvVarName" - var testEnvVarValue = "testEnvVarValue" - - ctx := context.Background() - - typeNamespacedName := types.NamespacedName{ - Name: resourceName, - Namespace: "default", - } - featurestore := &feastdevv1alpha1.FeatureStore{} - onlineStorePath := "/data/online.db" - registryPath := "/data/registry.db" - offlineType := "duckdb" - - BeforeEach(func() { - By("creating the custom resource for the Kind FeatureStore") - err := k8sClient.Get(ctx, typeNamespacedName, featurestore) - if err != nil && errors.IsNotFound(err) { - resource := createFeatureStoreResource(resourceName, image, pullPolicy, &[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, - {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}}) - resource.Spec.Services.OfflineStore.Persistence = &feastdevv1alpha1.OfflineStorePersistence{ - FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ - Type: offlineType, - }, - } - resource.Spec.Services.OnlineStore.Persistence = &feastdevv1alpha1.OnlineStorePersistence{ - FilePersistence: &feastdevv1alpha1.OnlineStoreFilePersistence{ - Path: onlineStorePath, - }, - } - resource.Spec.Services.Registry = &feastdevv1alpha1.Registry{ - Local: &feastdevv1alpha1.LocalRegistryConfig{ - Persistence: &feastdevv1alpha1.RegistryPersistence{ - FilePersistence: &feastdevv1alpha1.RegistryFilePersistence{ - Path: registryPath, - }, - }, - }, - } - - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - } - }) - AfterEach(func() { - resource := &feastdevv1alpha1.FeatureStore{} - err := k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - - By("Cleanup the specific resource instance FeatureStore") - Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) - }) - - It("should successfully reconcile the resource", func() { - By("Reconciling the created resource") - controllerReconciler := &FeatureStoreReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - } - - _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: typeNamespacedName, - }) - Expect(err).NotTo(HaveOccurred()) - - resource := &feastdevv1alpha1.FeatureStore{} - err = k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - - feast := services.FeastServices{ - Client: controllerReconciler.Client, - Context: ctx, - Scheme: controllerReconciler.Scheme, - FeatureStore: resource, - } - Expect(resource.Status).NotTo(BeNil()) - Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion)) - Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType))) - Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject)) - Expect(resource.Status.Applied.Services).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.Type).To(Equal(offlineType)) - Expect(resource.Status.Applied.Services.OfflineStore.ImagePullPolicy).To(BeNil()) - Expect(resource.Status.Applied.Services.OfflineStore.Resources).To(BeNil()) - Expect(resource.Status.Applied.Services.OfflineStore.Image).To(Equal(&services.DefaultImage)) - Expect(resource.Status.Applied.Services.OnlineStore).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OnlineStore.Persistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(onlineStorePath)) - Expect(resource.Status.Applied.Services.OnlineStore.Env).To(Equal(&[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}})) - Expect(resource.Status.Applied.Services.OnlineStore.ImagePullPolicy).To(Equal(&pullPolicy)) - Expect(resource.Status.Applied.Services.OnlineStore.Resources).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.OnlineStore.Image).To(Equal(&image)) - Expect(resource.Status.Applied.Services.Registry).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local.Persistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence).NotTo(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(registryPath)) - Expect(resource.Status.Applied.Services.Registry.Local.ImagePullPolicy).To(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local.Resources).To(BeNil()) - Expect(resource.Status.Applied.Services.Registry.Local.Image).To(Equal(&services.DefaultImage)) - - domain := ".svc.cluster.local:80" - Expect(resource.Status.ServiceHostnames.OfflineStore).To(Equal(feast.GetFeastServiceName(services.OfflineFeastType) + "." + resource.Namespace + domain)) - Expect(resource.Status.ServiceHostnames.OnlineStore).To(Equal(feast.GetFeastServiceName(services.OnlineFeastType) + "." + resource.Namespace + domain)) - Expect(resource.Status.ServiceHostnames.Registry).To(Equal(feast.GetFeastServiceName(services.RegistryFeastType) + "." + resource.Namespace + domain)) - - Expect(resource.Status.Conditions).NotTo(BeEmpty()) - cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ReadyType) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) - Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType)) - Expect(cond.Message).To(Equal(feastdevv1alpha1.ReadyMessage)) - - cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) - Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType)) - Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage)) - - cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ClientReadyType) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) - Expect(cond.Type).To(Equal(feastdevv1alpha1.ClientReadyType)) - Expect(cond.Message).To(Equal(feastdevv1alpha1.ClientReadyMessage)) - - cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OfflineStoreReadyType) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) - Expect(cond.Type).To(Equal(feastdevv1alpha1.OfflineStoreReadyType)) - Expect(cond.Message).To(Equal(feastdevv1alpha1.OfflineStoreReadyMessage)) - - cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OnlineStoreReadyType) - Expect(cond).ToNot(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionTrue)) - Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) - Expect(cond.Type).To(Equal(feastdevv1alpha1.OnlineStoreReadyType)) - Expect(cond.Message).To(Equal(feastdevv1alpha1.OnlineStoreReadyMessage)) - - Expect(resource.Status.Phase).To(Equal(feastdevv1alpha1.ReadyPhase)) - - deploy := &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.RegistryFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) - Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) - Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) - - svc := &corev1.Service{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.RegistryFeastType), - Namespace: resource.Namespace, - }, - svc) - Expect(err).NotTo(HaveOccurred()) - Expect(controllerutil.HasControllerReference(svc)).To(BeTrue()) - Expect(svc.Spec.Ports[0].TargetPort).To(Equal(intstr.FromInt(int(services.FeastServiceConstants[services.RegistryFeastType].TargetPort)))) - }) - - It("should properly encode a feature_store.yaml config", func() { - By("Reconciling the created resource") - controllerReconciler := &FeatureStoreReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - } - - _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: typeNamespacedName, - }) - Expect(err).NotTo(HaveOccurred()) - - resource := &feastdevv1alpha1.FeatureStore{} - err = k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - - req, err := labels.NewRequirement(services.NameLabelKey, selection.Equals, []string{resource.Name}) - Expect(err).NotTo(HaveOccurred()) - labelSelector := labels.NewSelector().Add(*req) - listOpts := &client.ListOptions{Namespace: resource.Namespace, LabelSelector: labelSelector} - deployList := appsv1.DeploymentList{} - err = k8sClient.List(ctx, &deployList, listOpts) - Expect(err).NotTo(HaveOccurred()) - Expect(deployList.Items).To(HaveLen(3)) - - svcList := corev1.ServiceList{} - err = k8sClient.List(ctx, &svcList, listOpts) - Expect(err).NotTo(HaveOccurred()) - Expect(svcList.Items).To(HaveLen(3)) - - cmList := corev1.ConfigMapList{} - err = k8sClient.List(ctx, &cmList, listOpts) - Expect(err).NotTo(HaveOccurred()) - Expect(cmList.Items).To(HaveLen(1)) - - feast := services.FeastServices{ - Client: controllerReconciler.Client, - Context: ctx, - Scheme: controllerReconciler.Scheme, - FeatureStore: resource, - } - - // check registry config - deploy := &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.RegistryFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) - Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(1)) - env := getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) - Expect(env).NotTo(BeNil()) - - fsYamlStr, err := feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) - Expect(err).NotTo(HaveOccurred()) - Expect(fsYamlStr).To(Equal(env.Value)) - - envByte, err := base64.StdEncoding.DecodeString(env.Value) - Expect(err).NotTo(HaveOccurred()) - repoConfig := &services.RepoConfig{} - err = yaml.Unmarshal(envByte, repoConfig) - Expect(err).NotTo(HaveOccurred()) - testConfig := &services.RepoConfig{ - Project: feastProject, - Provider: services.LocalProviderType, - EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, - Registry: services.RegistryConfig{ - RegistryType: services.RegistryFileConfigType, - Path: registryPath, - }, - } - Expect(repoConfig).To(Equal(testConfig)) - - // check offline config - deploy = &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.OfflineFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) - Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(1)) - env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) - Expect(env).NotTo(BeNil()) - - fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) - Expect(err).NotTo(HaveOccurred()) - Expect(fsYamlStr).To(Equal(env.Value)) - - envByte, err = base64.StdEncoding.DecodeString(env.Value) - Expect(err).NotTo(HaveOccurred()) - repoConfigOffline := &services.RepoConfig{} - err = yaml.Unmarshal(envByte, repoConfigOffline) - Expect(err).NotTo(HaveOccurred()) - regRemote := services.RegistryConfig{ - RegistryType: services.RegistryRemoteConfigType, - Path: fmt.Sprintf("feast-%s-registry.default.svc.cluster.local:80", resourceName), - } - offlineConfig := &services.RepoConfig{ - Project: feastProject, - Provider: services.LocalProviderType, - EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, - OfflineStore: services.OfflineStoreConfig{ - Type: services.OfflineDuckDbConfigType, - }, - Registry: regRemote, - } - Expect(repoConfigOffline).To(Equal(offlineConfig)) - - // check online config - deploy = &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.OnlineFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) - Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(3)) - Expect(deploy.Spec.Template.Spec.Containers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) - env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) - Expect(env).NotTo(BeNil()) - - fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) - Expect(err).NotTo(HaveOccurred()) - Expect(fsYamlStr).To(Equal(env.Value)) - - envByte, err = base64.StdEncoding.DecodeString(env.Value) - Expect(err).NotTo(HaveOccurred()) - repoConfigOnline := &services.RepoConfig{} - err = yaml.Unmarshal(envByte, repoConfigOnline) - Expect(err).NotTo(HaveOccurred()) - offlineRemote := services.OfflineStoreConfig{ - Host: fmt.Sprintf("feast-%s-offline.default.svc.cluster.local", resourceName), - Type: services.OfflineRemoteConfigType, - Port: services.HttpPort, - } - onlineConfig := &services.RepoConfig{ - Project: feastProject, - Provider: services.LocalProviderType, - EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, - OfflineStore: offlineRemote, - OnlineStore: services.OnlineStoreConfig{ - Path: onlineStorePath, - Type: services.OnlineSqliteConfigType, - }, - Registry: regRemote, - } - Expect(repoConfigOnline).To(Equal(onlineConfig)) - Expect(deploy.Spec.Template.Spec.Containers[0].Env).To(HaveLen(3)) - - // check client config - cm := &corev1.ConfigMap{} - name := feast.GetFeastServiceName(services.ClientFeastType) - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: name, - Namespace: resource.Namespace, - }, - cm) - Expect(err).NotTo(HaveOccurred()) - repoConfigClient := &services.RepoConfig{} - err = yaml.Unmarshal([]byte(cm.Data[services.FeatureStoreYamlCmKey]), repoConfigClient) - Expect(err).NotTo(HaveOccurred()) - clientConfig := &services.RepoConfig{ - Project: feastProject, - Provider: services.LocalProviderType, - EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, - OfflineStore: offlineRemote, - OnlineStore: services.OnlineStoreConfig{ - Path: fmt.Sprintf("http://feast-%s-online.default.svc.cluster.local:80", resourceName), - Type: services.OnlineRemoteConfigType, - }, - Registry: regRemote, - } - Expect(repoConfigClient).To(Equal(clientConfig)) - - // change paths and reconcile - resourceNew := resource.DeepCopy() - newOnlineStorePath := "/data/new_online.db" - newRegistryPath := "/data/new_registry.db" - resourceNew.Spec.Services.OnlineStore.Persistence.FilePersistence.Path = newOnlineStorePath - resourceNew.Spec.Services.Registry.Local.Persistence.FilePersistence.Path = newRegistryPath - err = k8sClient.Update(ctx, resourceNew) - Expect(err).NotTo(HaveOccurred()) - _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: typeNamespacedName, - }) - Expect(err).NotTo(HaveOccurred()) - - resource = &feastdevv1alpha1.FeatureStore{} - err = k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - feast.FeatureStore = resource - - // check registry config - deploy = &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.RegistryFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) - Expect(env).NotTo(BeNil()) - fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) - Expect(err).NotTo(HaveOccurred()) - Expect(fsYamlStr).To(Equal(env.Value)) - - envByte, err = base64.StdEncoding.DecodeString(env.Value) - Expect(err).NotTo(HaveOccurred()) - repoConfig = &services.RepoConfig{} - err = yaml.Unmarshal(envByte, repoConfig) - Expect(err).NotTo(HaveOccurred()) - testConfig.Registry.Path = newRegistryPath - Expect(repoConfig).To(Equal(testConfig)) - - // check offline config - deploy = &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.OfflineFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) - Expect(env).NotTo(BeNil()) - - fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) - Expect(err).NotTo(HaveOccurred()) - Expect(fsYamlStr).To(Equal(env.Value)) - - envByte, err = base64.StdEncoding.DecodeString(env.Value) - Expect(err).NotTo(HaveOccurred()) - repoConfigOffline = &services.RepoConfig{} - err = yaml.Unmarshal(envByte, repoConfigOffline) - Expect(err).NotTo(HaveOccurred()) - Expect(repoConfigOffline).To(Equal(offlineConfig)) - - // check online config - deploy = &appsv1.Deployment{} - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: feast.GetFeastServiceName(services.OnlineFeastType), - Namespace: resource.Namespace, - }, - deploy) - Expect(err).NotTo(HaveOccurred()) - env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) - Expect(env).NotTo(BeNil()) - - fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) - Expect(err).NotTo(HaveOccurred()) - Expect(fsYamlStr).To(Equal(env.Value)) - - envByte, err = base64.StdEncoding.DecodeString(env.Value) - Expect(err).NotTo(HaveOccurred()) - - repoConfigOnline = &services.RepoConfig{} - err = yaml.Unmarshal(envByte, repoConfigOnline) - Expect(err).NotTo(HaveOccurred()) - onlineConfig.OnlineStore.Path = newOnlineStorePath - Expect(repoConfigOnline).To(Equal(onlineConfig)) - }) - }) }) func createFeatureStoreResource(resourceName string, image string, pullPolicy corev1.PullPolicy, envVars *[]corev1.EnvVar) *feastdevv1alpha1.FeatureStore { diff --git a/infra/feast-operator/internal/controller/services/repo_config.go b/infra/feast-operator/internal/controller/services/repo_config.go index e087038ba52..e56f311a127 100644 --- a/infra/feast-operator/internal/controller/services/repo_config.go +++ b/infra/feast-operator/internal/controller/services/repo_config.go @@ -18,6 +18,7 @@ package services import ( "encoding/base64" + "path" "strings" feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" @@ -52,13 +53,14 @@ func getServiceRepoConfig(feastType FeastServiceType, featureStore *feastdevv1al repoConfig := getClientRepoConfig(featureStore) isLocalRegistry := isLocalRegistry(featureStore) if appliedSpec.Services != nil { + services := appliedSpec.Services // Offline server has an `offline_store` section and a remote `registry` - if feastType == OfflineFeastType && appliedSpec.Services.OfflineStore != nil { + if feastType == OfflineFeastType && services.OfflineStore != nil { fileType := string(OfflineDaskConfigType) - if appliedSpec.Services.OfflineStore.Persistence != nil && - appliedSpec.Services.OfflineStore.Persistence.FilePersistence != nil && - len(appliedSpec.Services.OfflineStore.Persistence.FilePersistence.Type) > 0 { - fileType = appliedSpec.Services.OfflineStore.Persistence.FilePersistence.Type + if services.OfflineStore.Persistence != nil && + services.OfflineStore.Persistence.FilePersistence != nil && + len(services.OfflineStore.Persistence.FilePersistence.Type) > 0 { + fileType = services.OfflineStore.Persistence.FilePersistence.Type } repoConfig.OfflineStore = OfflineStoreConfig{ @@ -67,10 +69,11 @@ func getServiceRepoConfig(feastType FeastServiceType, featureStore *feastdevv1al repoConfig.OnlineStore = OnlineStoreConfig{} } // Online server has an `online_store` section, a remote `registry` and a remote `offline_store` - if feastType == OnlineFeastType && appliedSpec.Services.OnlineStore != nil { - path := DefaultOnlinePath - if appliedSpec.Services.OnlineStore.Persistence != nil && appliedSpec.Services.OnlineStore.Persistence.FilePersistence != nil { - path = appliedSpec.Services.OnlineStore.Persistence.FilePersistence.Path + if feastType == OnlineFeastType && services.OnlineStore != nil { + path := DefaultOnlineStoreEphemeralPath + if services.OnlineStore.Persistence != nil && services.OnlineStore.Persistence.FilePersistence != nil { + filePersistence := services.OnlineStore.Persistence.FilePersistence + path = getActualPath(filePersistence.Path, filePersistence.PvcConfig) } repoConfig.OnlineStore = OnlineStoreConfig{ @@ -80,10 +83,11 @@ func getServiceRepoConfig(feastType FeastServiceType, featureStore *feastdevv1al } // Registry server only has a `registry` section if feastType == RegistryFeastType && isLocalRegistry { - path := DefaultRegistryPath - if appliedSpec.Services != nil && appliedSpec.Services.Registry != nil && appliedSpec.Services.Registry.Local != nil && - appliedSpec.Services.Registry.Local.Persistence != nil && appliedSpec.Services.Registry.Local.Persistence.FilePersistence != nil { - path = appliedSpec.Services.Registry.Local.Persistence.FilePersistence.Path + path := DefaultRegistryEphemeralPath + if services != nil && services.Registry != nil && services.Registry.Local != nil && + services.Registry.Local.Persistence != nil && services.Registry.Local.Persistence.FilePersistence != nil { + filePersistence := services.Registry.Local.Persistence.FilePersistence + path = getActualPath(filePersistence.Path, filePersistence.PvcConfig) } repoConfig.Registry = RegistryConfig{ RegistryType: RegistryFileConfigType, @@ -129,3 +133,10 @@ func getClientRepoConfig(featureStore *feastdevv1alpha1.FeatureStore) RepoConfig } return clientRepoConfig } + +func getActualPath(filePath string, pvcConfig *feastdevv1alpha1.PvcConfig) string { + if pvcConfig == nil { + return filePath + } + return path.Join(pvcConfig.MountPath, filePath) +} diff --git a/infra/feast-operator/internal/controller/services/repo_config_test.go b/infra/feast-operator/internal/controller/services/repo_config_test.go index 285fc05c566..1868bf437b5 100644 --- a/infra/feast-operator/internal/controller/services/repo_config_test.go +++ b/infra/feast-operator/internal/controller/services/repo_config_test.go @@ -51,7 +51,7 @@ var _ = Describe("Repo Config", func() { Expect(repoConfig.OnlineStore).To(Equal(emptyOnlineStoreConfig())) expectedRegistryConfig := RegistryConfig{ RegistryType: "file", - Path: DefaultRegistryPath, + Path: DefaultRegistryEphemeralPath, } Expect(repoConfig.Registry).To(Equal(expectedRegistryConfig)) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index a205c838ea1..4a71a139f23 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -53,6 +53,11 @@ func (feast *FeastServices) Deploy() error { if err := feast.deployFeastServiceByType(OfflineFeastType); err != nil { return err } + if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, OfflineFeastType); !shouldCreate { + if err := feast.deleteOwnedFeastObj(feast.initPVC(OfflineFeastType)); err != nil && !apierrors.IsNotFound(err) { + return err + } + } } else { if err := feast.removeFeastServiceByType(OfflineFeastType); err != nil { return err @@ -63,6 +68,11 @@ func (feast *FeastServices) Deploy() error { if err := feast.deployFeastServiceByType(OnlineFeastType); err != nil { return err } + if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, OnlineFeastType); !shouldCreate { + if err := feast.deleteOwnedFeastObj(feast.initPVC(OnlineFeastType)); err != nil && !apierrors.IsNotFound(err) { + return err + } + } } else { if err := feast.removeFeastServiceByType(OnlineFeastType); err != nil { return err @@ -73,6 +83,11 @@ func (feast *FeastServices) Deploy() error { if err := feast.deployFeastServiceByType(RegistryFeastType); err != nil { return err } + if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, RegistryFeastType); !shouldCreate { + if err := feast.deleteOwnedFeastObj(feast.initPVC(RegistryFeastType)); err != nil && !apierrors.IsNotFound(err) { + return err + } + } } else { if err := feast.removeFeastServiceByType(RegistryFeastType); err != nil { return err @@ -94,6 +109,11 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) if err := feast.createDeployment(feastType); err != nil { return feast.setFeastServiceCondition(err, feastType) } + if pvcCreate, shouldCreate := shouldCreatePvc(feast.FeatureStore, feastType); shouldCreate { + if err := feast.createPVC(pvcCreate, feastType); err != nil { + return feast.setFeastServiceCondition(err, feastType) + } + } return feast.setFeastServiceCondition(nil, feastType) } @@ -104,6 +124,11 @@ func (feast *FeastServices) removeFeastServiceByType(feastType FeastServiceType) if err := feast.deleteOwnedFeastObj(feast.initFeastSvc(feastType)); err != nil { return err } + if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, feastType); shouldCreate { + if err := feast.deleteOwnedFeastObj(feast.initPVC(feastType)); err != nil { + return err + } + } apimeta.RemoveStatusCondition(&feast.FeatureStore.Status.Conditions, FeastServiceConditions[feastType][metav1.ConditionTrue].Type) return nil } @@ -135,6 +160,24 @@ func (feast *FeastServices) createDeployment(feastType FeastServiceType) error { return nil } +func (feast *FeastServices) createPVC(pvcCreate *feastdevv1alpha1.PvcCreate, feastType FeastServiceType) error { + logger := log.FromContext(feast.Context) + pvc := feast.initPVC(feastType) + err := feast.setPVC(pvc, pvcCreate, feastType) + if err != nil { + return err + } + if op, err := controllerutil.CreateOrUpdate(feast.Context, feast.Client, pvc, controllerutil.MutateFn(func() error { + return nil + })); err != nil { + return err + } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "PersistentVolumeClaim", pvc.Name, "operation", op) + } + + return nil +} + func (feast *FeastServices) setDeployment(deploy *appsv1.Deployment, feastType FeastServiceType) error { fsYamlB64, err := feast.GetServiceFeatureStoreYamlBase64(feastType) if err != nil { @@ -196,6 +239,9 @@ func (feast *FeastServices) setDeployment(deploy *appsv1.Deployment, feastType F // configs are applied here container := &deploy.Spec.Template.Spec.Containers[0] applyOptionalContainerConfigs(container, serviceConfigs.OptionalConfigs) + if pvcConfig, hasPvcConfig := hasPvcConfig(feast.FeatureStore, feastType); hasPvcConfig { + mountPvcConfig(&deploy.Spec.Template.Spec, pvcConfig, deploy.Name) + } return controllerutil.SetControllerReference(feast.FeatureStore, deploy, feast.Scheme) } @@ -220,6 +266,17 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi return controllerutil.SetControllerReference(feast.FeatureStore, svc, feast.Scheme) } +func (feast *FeastServices) setPVC(pvc *corev1.PersistentVolumeClaim, pvcCreate *feastdevv1alpha1.PvcCreate, feastType FeastServiceType) error { + pvc.Labels = feast.getLabels(feastType) + + pvc.Spec = corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, + Resources: pvcCreate.Resources, + } + + return controllerutil.SetControllerReference(feast.FeatureStore, pvc, feast.Scheme) +} + func (feast *FeastServices) getServiceConfigs(feastType FeastServiceType) feastdevv1alpha1.ServiceConfigs { appliedSpec := feast.FeatureStore.Status.Applied if feastType == OfflineFeastType && appliedSpec.Services.OfflineStore != nil { @@ -376,6 +433,14 @@ func (feast *FeastServices) initFeastSvc(feastType FeastServiceType) *corev1.Ser return svc } +func (feast *FeastServices) initPVC(feastType FeastServiceType) *corev1.PersistentVolumeClaim { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: feast.GetObjectMeta(feastType), + } + pvc.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim")) + return pvc +} + // delete an object if the FeatureStore is set as the object's controller/owner func (feast *FeastServices) deleteOwnedFeastObj(obj client.Object) error { if err := feast.Client.Get(feast.Context, client.ObjectKeyFromObject(obj), obj); err != nil { @@ -427,3 +492,30 @@ func mergeEnvVarsArrays(envVars1 []corev1.EnvVar, envVars2 *[]corev1.EnvVar) []c return result } + +func mountPvcConfig(podSpec *corev1.PodSpec, pvcConfig *feastdevv1alpha1.PvcConfig, deployName string) { + container := &podSpec.Containers[0] + var pvcName string + if pvcConfig.Create != nil { + pvcName = deployName + } else { + pvcName = pvcConfig.Ref.Name + } + + podSpec.Volumes = []corev1.Volume{ + { + Name: pvcName, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: pvcName, + }, + }, + }, + } + container.VolumeMounts = []corev1.VolumeMount{ + { + Name: pvcName, + MountPath: pvcConfig.MountPath, + }, + } +} diff --git a/infra/feast-operator/internal/controller/services/services_types.go b/infra/feast-operator/internal/controller/services/services_types.go index 222bf62dc0e..e32c5cf6cae 100644 --- a/infra/feast-operator/internal/controller/services/services_types.go +++ b/infra/feast-operator/internal/controller/services/services_types.go @@ -27,13 +27,19 @@ import ( ) const ( - FeastPrefix = "feast-" - FeatureStoreYamlEnvVar = "FEATURE_STORE_YAML_BASE64" - FeatureStoreYamlCmKey = "feature_store.yaml" - DefaultRegistryPath = "/tmp/registry.db" - DefaultOnlinePath = "/tmp/online_store.db" - svcDomain = ".svc.cluster.local" - HttpPort = 80 + FeastPrefix = "feast-" + FeatureStoreYamlEnvVar = "FEATURE_STORE_YAML_BASE64" + FeatureStoreYamlCmKey = "feature_store.yaml" + DefaultRegistryEphemeralPath = "/tmp/registry.db" + DefaultRegistryPvcPath = "registry.db" + DefaultOnlineStoreEphemeralPath = "/tmp/online_store.db" + DefaultOnlineStorePvcPath = "online_store.db" + svcDomain = ".svc.cluster.local" + HttpPort = 80 + + DefaultOfflineStorageRequest = "20Gi" + DefaultOnlineStorageRequest = "5Gi" + DefaultRegistryStorageRequest = "5Gi" OfflineFeastType FeastServiceType = "offline" OnlineFeastType FeastServiceType = "online" diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index c2b29361a84..72ea70d8fdb 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -6,6 +6,8 @@ import ( "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" ) func isLocalRegistry(featureStore *feastdevv1alpha1.FeatureStore) bool { @@ -13,6 +15,28 @@ func isLocalRegistry(featureStore *feastdevv1alpha1.FeatureStore) bool { return appliedServices != nil && appliedServices.Registry != nil && appliedServices.Registry.Local != nil } +func hasPvcConfig(featureStore *feastdevv1alpha1.FeatureStore, feastType FeastServiceType) (*feastdevv1alpha1.PvcConfig, bool) { + services := featureStore.Status.Applied.Services + var pvcConfig *feastdevv1alpha1.PvcConfig = nil + if feastType == OnlineFeastType && services.OnlineStore != nil && services.OnlineStore.Persistence.FilePersistence != nil { + pvcConfig = services.OnlineStore.Persistence.FilePersistence.PvcConfig + } + if feastType == OfflineFeastType && services.OfflineStore != nil && services.OfflineStore.Persistence.FilePersistence != nil { + pvcConfig = services.OfflineStore.Persistence.FilePersistence.PvcConfig + } + if feastType == RegistryFeastType && isLocalRegistry(featureStore) && services.Registry.Local.Persistence.FilePersistence != nil { + pvcConfig = services.Registry.Local.Persistence.FilePersistence.PvcConfig + } + return pvcConfig, pvcConfig != nil +} + +func shouldCreatePvc(featureStore *feastdevv1alpha1.FeatureStore, feastType FeastServiceType) (*feastdevv1alpha1.PvcCreate, bool) { + if pvcConfig, ok := hasPvcConfig(featureStore, feastType); ok { + return pvcConfig.Create, pvcConfig.Create != nil + } + return nil, false +} + func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { cr.Status.FeastVersion = feastversion.FeastVersion applied := cr.Spec.DeepCopy() @@ -38,7 +62,13 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { services.Registry.Local.Persistence.FilePersistence = &feastdevv1alpha1.RegistryFilePersistence{} } if len(services.Registry.Local.Persistence.FilePersistence.Path) == 0 { - services.Registry.Local.Persistence.FilePersistence.Path = DefaultRegistryPath + services.Registry.Local.Persistence.FilePersistence.Path = defaultRegistryPath(services.Registry.Local.Persistence.FilePersistence) + } + if services.Registry.Local.Persistence.FilePersistence.PvcConfig != nil { + pvc := services.Registry.Local.Persistence.FilePersistence.PvcConfig + if pvc.Create != nil { + ensureRequestedStorage(&pvc.Create.Resources, DefaultRegistryStorageRequest) + } } setServiceDefaultConfigs(&services.Registry.Local.ServiceConfigs.DefaultConfigs) } @@ -53,6 +83,12 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { if len(services.OfflineStore.Persistence.FilePersistence.Type) == 0 { services.OfflineStore.Persistence.FilePersistence.Type = string(OfflineDaskConfigType) } + if services.OfflineStore.Persistence.FilePersistence.PvcConfig != nil { + pvc := services.OfflineStore.Persistence.FilePersistence.PvcConfig + if pvc.Create != nil { + ensureRequestedStorage(&pvc.Create.Resources, DefaultOfflineStorageRequest) + } + } } if services.OnlineStore != nil { setServiceDefaultConfigs(&services.OnlineStore.ServiceConfigs.DefaultConfigs) @@ -63,7 +99,13 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { services.OnlineStore.Persistence.FilePersistence = &feastdevv1alpha1.OnlineStoreFilePersistence{} } if len(services.OnlineStore.Persistence.FilePersistence.Path) == 0 { - services.OnlineStore.Persistence.FilePersistence.Path = DefaultOnlinePath + services.OnlineStore.Persistence.FilePersistence.Path = defaultOnlineStorePath(services.OnlineStore.Persistence.FilePersistence) + } + if services.OnlineStore.Persistence.FilePersistence.PvcConfig != nil { + pvc := services.OnlineStore.Persistence.FilePersistence.PvcConfig + if pvc.Create != nil { + ensureRequestedStorage(&pvc.Create.Resources, DefaultOnlineStorageRequest) + } } } // overwrite status.applied with every reconcile @@ -82,3 +124,25 @@ func checkOfflineStoreFilePersistenceType(value string) error { } return fmt.Errorf("invalid file type %s for offline store", value) } + +func ensureRequestedStorage(resources *v1.VolumeResourceRequirements, requestedStorage string) { + if resources.Requests == nil { + resources.Requests = v1.ResourceList{} + } + if _, ok := resources.Requests[v1.ResourceStorage]; !ok { + resources.Requests[v1.ResourceStorage] = resource.MustParse(requestedStorage) + } +} + +func defaultOnlineStorePath(persistence *feastdevv1alpha1.OnlineStoreFilePersistence) string { + if persistence.PvcConfig == nil { + return DefaultOnlineStoreEphemeralPath + } + return DefaultOnlineStorePvcPath +} +func defaultRegistryPath(persistence *feastdevv1alpha1.RegistryFilePersistence) string { + if persistence.PvcConfig == nil { + return DefaultRegistryEphemeralPath + } + return DefaultRegistryPvcPath +} diff --git a/infra/feast-operator/test/api/featurestore_types_test.go b/infra/feast-operator/test/api/featurestore_types_test.go new file mode 100644 index 00000000000..7e8a448f199 --- /dev/null +++ b/infra/feast-operator/test/api/featurestore_types_test.go @@ -0,0 +1,299 @@ +package api + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/log" + + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Function to create invalid OnlineStore resource +func createFeatureStore() *feastdevv1alpha1.FeatureStore { + return &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: namespaceName, + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: "test_project", + }, + } +} + +func attemptInvalidCreationAndAsserts(ctx context.Context, featurestore *feastdevv1alpha1.FeatureStore, matcher string) { + By("Creating the resource") + logger := log.FromContext(ctx) + logger.Info("Creating", "FeatureStore", featurestore) + err := k8sClient.Create(ctx, featurestore) + logger.Info("Got", "err", err) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).Should(ContainSubstring(matcher)) +} + +func onlineStoreWithAbsolutePathForPvc(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + OnlineStore: &feastdevv1alpha1.OnlineStore{ + Persistence: &feastdevv1alpha1.OnlineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OnlineStoreFilePersistence{ + Path: "/data/online_store.db", + PvcConfig: &feastdevv1alpha1.PvcConfig{}, + }, + }, + }, + } + return copy +} +func onlineStoreWithRelativePathForEphemeral(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + OnlineStore: &feastdevv1alpha1.OnlineStore{ + Persistence: &feastdevv1alpha1.OnlineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OnlineStoreFilePersistence{ + Path: "data/online_store.db", + }, + }, + }, + } + return copy +} + +func offlineStoreWithUnmanagedFileType(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + OfflineStore: &feastdevv1alpha1.OfflineStore{ + Persistence: &feastdevv1alpha1.OfflineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ + Type: "unmanaged", + }, + }, + }, + } + return copy +} + +func registryWithAbsolutePathForPvc(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Persistence: &feastdevv1alpha1.RegistryPersistence{ + FilePersistence: &feastdevv1alpha1.RegistryFilePersistence{ + Path: "/data/registry.db", + PvcConfig: &feastdevv1alpha1.PvcConfig{}, + }}, + }, + }, + } + return copy +} +func registryWithRelativePathForEphemeral(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Persistence: &feastdevv1alpha1.RegistryPersistence{ + FilePersistence: &feastdevv1alpha1.RegistryFilePersistence{ + Path: "data/online_store.db", + }, + }, + }, + }, + } + return copy +} +func pvcConfigWithNeitherRefNorCreate(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + OfflineStore: &feastdevv1alpha1.OfflineStore{ + Persistence: &feastdevv1alpha1.OfflineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ + PvcConfig: &feastdevv1alpha1.PvcConfig{}, + }, + }, + }, + } + return copy +} +func pvcConfigWithBothRefAndCreate(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + OfflineStore: &feastdevv1alpha1.OfflineStore{ + Persistence: &feastdevv1alpha1.OfflineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Ref: &corev1.LocalObjectReference{ + Name: "pvc", + }, + Create: &feastdevv1alpha1.PvcCreate{}, + }, + }, + }, + }, + } + return copy +} + +func pvcConfigWithNoResources(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := featureStore.DeepCopy() + copy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + OfflineStore: &feastdevv1alpha1.OfflineStore{ + Persistence: &feastdevv1alpha1.OfflineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Create: &feastdevv1alpha1.PvcCreate{}, + MountPath: "/data/offline", + }, + }, + }, + }, + OnlineStore: &feastdevv1alpha1.OnlineStore{ + Persistence: &feastdevv1alpha1.OnlineStorePersistence{ + FilePersistence: &feastdevv1alpha1.OnlineStoreFilePersistence{ + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Create: &feastdevv1alpha1.PvcCreate{}, + MountPath: "/data/online", + }, + }, + }, + }, + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Persistence: &feastdevv1alpha1.RegistryPersistence{ + FilePersistence: &feastdevv1alpha1.RegistryFilePersistence{ + PvcConfig: &feastdevv1alpha1.PvcConfig{ + Create: &feastdevv1alpha1.PvcCreate{}, + MountPath: "/data/registry", + }, + }, + }, + }, + }, + } + return copy +} + +func pvcConfigWithResources(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + copy := pvcConfigWithNoResources(featureStore) + copy.Spec.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.Resources = corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + } + copy.Spec.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.Resources = corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + } + copy.Spec.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.Resources = corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("500Mi"), + }, + } + return copy +} + +const resourceName = "test-resource" +const namespaceName = "default" + +var typeNamespacedName = types.NamespacedName{ + Name: resourceName, + Namespace: "default", +} + +func initContext() (context.Context, *feastdevv1alpha1.FeatureStore) { + ctx := context.Background() + + featurestore := createFeatureStore() + + BeforeEach(func() { + By("verifying the custom resource FeatureStore is not there") + err := k8sClient.Get(ctx, typeNamespacedName, featurestore) + Expect(err != nil && errors.IsNotFound(err)) + }) + AfterEach(func() { + By("verifying the custom resource FeatureStore is not there") + err := k8sClient.Get(ctx, typeNamespacedName, featurestore) + Expect(err != nil && errors.IsNotFound(err)) + }) + + return ctx, featurestore +} + +var _ = Describe("FeatureStore API", func() { + Context("When creating an invalid Online Store", func() { + ctx, featurestore := initContext() + + It("should fail when PVC persistence has absolute path", func() { + attemptInvalidCreationAndAsserts(ctx, onlineStoreWithAbsolutePathForPvc(featurestore), "PVC path must be a file name only") + }) + It("should fail when ephemeral persistence has relative path", func() { + attemptInvalidCreationAndAsserts(ctx, onlineStoreWithRelativePathForEphemeral(featurestore), "Ephemeral stores must have absolute paths") + }) + }) + + Context("When creating an invalid Offline Store", func() { + ctx, featurestore := initContext() + + It("should fail when PVC persistence has absolute path", func() { + attemptInvalidCreationAndAsserts(ctx, offlineStoreWithUnmanagedFileType(featurestore), "Unsupported value") + }) + }) + + Context("When creating an invalid Registry", func() { + ctx, featurestore := initContext() + + It("should fail when PVC persistence has absolute path", func() { + attemptInvalidCreationAndAsserts(ctx, registryWithAbsolutePathForPvc(featurestore), "PVC path must be a file name only") + }) + It("should fail when ephemeral persistence has relative path", func() { + attemptInvalidCreationAndAsserts(ctx, registryWithRelativePathForEphemeral(featurestore), "Ephemeral stores must have absolute paths") + }) + }) + + Context("When creating an invalid PvcConfig", func() { + ctx, featurestore := initContext() + + It("should fail when neither ref nor create settings are given", func() { + attemptInvalidCreationAndAsserts(ctx, pvcConfigWithNeitherRefNorCreate(featurestore), "One selection is required") + }) + It("should fail when both ref and create settings are given", func() { + attemptInvalidCreationAndAsserts(ctx, pvcConfigWithBothRefAndCreate(featurestore), "One selection is required") + }) + }) + + Context("When creating a valid PvcConfig", func() { + _, featurestore := initContext() + + It("should set the expected defaults", func() { + resource := pvcConfigWithNoResources(featurestore) + services.ApplyDefaultsToStatus(resource) + + storage := resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() + Expect(storage).To(Equal("20Gi")) + storage = resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() + Expect(storage).To(Equal("5Gi")) + storage = resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() + Expect(storage).To(Equal("5Gi")) + }) + It("should not override the configured resources", func() { + resource := pvcConfigWithResources(featurestore) + services.ApplyDefaultsToStatus(resource) + storage := resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() + Expect(storage).To(Equal("10Gi")) + storage = resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() + Expect(storage).To(Equal("1Gi")) + storage = resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() + Expect(storage).To(Equal("500Mi")) + }) + }) +}) diff --git a/infra/feast-operator/test/api/suite_test.go b/infra/feast-operator/test/api/suite_test.go new file mode 100644 index 00000000000..270742760e7 --- /dev/null +++ b/infra/feast-operator/test/api/suite_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2024 Feast Community. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "fmt" + "path/filepath" + "runtime" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + //+kubebuilder:scaffold:imports +) + +// These tests use Ginkgo (BDD-style Go testing framework). Refer to +// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. + +var cfg *rest.Config +var k8sClient client.Client +var testEnv *envtest.Environment + +func TestApis(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecs(t, "Api Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + By("bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + ErrorIfCRDPathMissing: true, + + // The BinaryAssetsDirectory is only required if you want to run the tests directly + // without call the makefile target test. If not informed it will look for the + // default path defined in controller-runtime which is /usr/local/kubebuilder/. + // Note that you must have the required binaries setup under the bin directory to perform + // the tests directly. When we run make test it will be setup and used automatically. + BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s", + fmt.Sprintf("1.29.0-%s-%s", runtime.GOOS, runtime.GOARCH)), + } + + var err error + // cfg is defined in this file globally. + cfg, err = testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg).NotTo(BeNil()) + + err = feastdevv1alpha1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + //+kubebuilder:scaffold:scheme + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient).NotTo(BeNil()) + +}) + +var _ = AfterSuite(func() { + By("tearing down the test environment") + err := testEnv.Stop() + Expect(err).NotTo(HaveOccurred()) +}) From 5a4f95429c41057a7ef2f94a3ac67847729d686c Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 8 Nov 2024 18:23:51 +0100 Subject: [PATCH 02/12] typo in sample manifest Signed-off-by: Daniele Martinoli --- .../config/samples/v1alpha1_featurestore_pvc_persistence.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml index d457a748822..b7c7412c0f0 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml @@ -31,7 +31,7 @@ spec: path: registry.db pvc: create: {} - mountPath: /data/registryma + mountPath: /data/registry --- apiVersion: v1 kind: PersistentVolumeClaim From 4ea2284c87b8250f703deab0d561ad93e467e29b Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 8 Nov 2024 20:45:34 +0100 Subject: [PATCH 03/12] removing private images Signed-off-by: Daniele Martinoli --- .../samples/v1alpha1_featurestore_ephemeral_persistence.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml index 8449ec1bf51..512fed9d4c0 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml @@ -6,18 +6,15 @@ spec: feastProject: my_project services: onlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/online_store.db offlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: type: dask registry: local: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/registry.db From 62a6669f683baec972de32a871980f1cd9462bb1 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 8 Nov 2024 21:30:17 +0100 Subject: [PATCH 04/12] More logs for PVC deletion and changing create strategy Signed-off-by: Daniele Martinoli --- .../featurestore_controller_pvc_test.go | 9 ++++---- .../internal/controller/services/services.go | 21 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) 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 edd0d8fffff..6b66aab307b 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" @@ -44,7 +45,7 @@ import ( var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Context("When deploying a resource with all ephemeral services", func() { - const resourceName = "services-ephemeral" + const resourceName = "services-pvc" var pullPolicy = corev1.PullAlways var testEnvVarName = "testEnvVarName" var testEnvVarValue = "testEnvVarValue" @@ -326,15 +327,15 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(0)) // check online pvc is deleted + log.FromContext(feast.Context).Info("Checking deletion of", "PersistentVolumeClaim", deploy.Name) pvc = &corev1.PersistentVolumeClaim{} err = k8sClient.Get(ctx, types.NamespacedName{ Name: deploy.Name, Namespace: resource.Namespace, }, pvc) - print(err) - // Expect(err).To(HaveOccurred()) - // Expect(errors.IsNotFound(err)).To(BeTrue()) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) }) It("should properly encode a feature_store.yaml config", func() { diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 4a71a139f23..0eec761fcf6 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -163,16 +163,14 @@ func (feast *FeastServices) createDeployment(feastType FeastServiceType) error { func (feast *FeastServices) createPVC(pvcCreate *feastdevv1alpha1.PvcCreate, feastType FeastServiceType) error { logger := log.FromContext(feast.Context) pvc := feast.initPVC(feastType) - err := feast.setPVC(pvc, pvcCreate, feastType) - if err != nil { - return err - } - if op, err := controllerutil.CreateOrUpdate(feast.Context, feast.Client, pvc, controllerutil.MutateFn(func() error { - return nil - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "PersistentVolumeClaim", pvc.Name, "operation", op) + feast.setPVC(pvc, pvcCreate, feastType) + err := feast.Client.Get(feast.Context, client.ObjectKeyFromObject(pvc), pvc) + if err != nil && apierrors.IsNotFound(err) { + err = feast.Client.Create(feast.Context, pvc) + if err != nil { + return err + } + logger.Info("Successfully created", "PersistentVolumeClaim", pvc.Name) } return nil @@ -443,6 +441,8 @@ func (feast *FeastServices) initPVC(feastType FeastServiceType) *corev1.Persiste // delete an object if the FeatureStore is set as the object's controller/owner func (feast *FeastServices) deleteOwnedFeastObj(obj client.Object) error { + name := obj.GetName() + kind := obj.GetObjectKind().GroupVersionKind().Kind if err := feast.Client.Get(feast.Context, client.ObjectKeyFromObject(obj), obj); err != nil { if apierrors.IsNotFound(err) { return nil @@ -454,6 +454,7 @@ func (feast *FeastServices) deleteOwnedFeastObj(obj client.Object) error { if err := feast.Client.Delete(feast.Context, obj); err != nil { return err } + log.FromContext(feast.Context).Info("Successfully deleted", kind, name) } } return nil From 8426e7b8f1738e2000abc76ebf2706d326202616 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 8 Nov 2024 21:41:42 +0100 Subject: [PATCH 05/12] using createNewPVC Signed-off-by: Daniele Martinoli --- .../internal/controller/services/services.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 0eec761fcf6..f75ba87b13d 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -162,9 +162,12 @@ func (feast *FeastServices) createDeployment(feastType FeastServiceType) error { func (feast *FeastServices) createPVC(pvcCreate *feastdevv1alpha1.PvcCreate, feastType FeastServiceType) error { logger := log.FromContext(feast.Context) - pvc := feast.initPVC(feastType) - feast.setPVC(pvc, pvcCreate, feastType) - err := feast.Client.Get(feast.Context, client.ObjectKeyFromObject(pvc), pvc) + pvc, err := feast.createNewPVC(pvcCreate, feastType) + if err != nil { + return err + } + + err = feast.Client.Get(feast.Context, client.ObjectKeyFromObject(pvc), pvc) if err != nil && apierrors.IsNotFound(err) { err = feast.Client.Create(feast.Context, pvc) if err != nil { @@ -264,15 +267,14 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi return controllerutil.SetControllerReference(feast.FeatureStore, svc, feast.Scheme) } -func (feast *FeastServices) setPVC(pvc *corev1.PersistentVolumeClaim, pvcCreate *feastdevv1alpha1.PvcCreate, feastType FeastServiceType) error { - pvc.Labels = feast.getLabels(feastType) +func (feast *FeastServices) createNewPVC(pvcCreate *feastdevv1alpha1.PvcCreate, feastType FeastServiceType) (*corev1.PersistentVolumeClaim, error) { + pvc := feast.initPVC(feastType) pvc.Spec = corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: pvcCreate.Resources, } - - return controllerutil.SetControllerReference(feast.FeatureStore, pvc, feast.Scheme) + return pvc, controllerutil.SetControllerReference(feast.FeatureStore, pvc, feast.Scheme) } func (feast *FeastServices) getServiceConfigs(feastType FeastServiceType) feastdevv1alpha1.ServiceConfigs { From cf3027a0c2375e853511f412e0a8f0f166343f4d Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 11 Nov 2024 16:31:53 +0100 Subject: [PATCH 06/12] fixed broken test Signed-off-by: Daniele Martinoli --- .../controller/featurestore_controller_pvc_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 6b66aab307b..9ba4011b94a 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -239,6 +239,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOfflineStorageRequest)) + Expect(pvc.DeletionTimestamp).To(BeNil()) // check online deployment deploy = &appsv1.Deployment{} @@ -268,6 +269,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOnlineStorageRequest)) + Expect(pvc.DeletionTimestamp).To(BeNil()) // check registry deployment deploy = &appsv1.Deployment{} @@ -297,6 +299,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultRegistryStorageRequest)) + Expect(pvc.DeletionTimestamp).To(BeNil()) // remove online PVC and reconcile resourceNew := resource.DeepCopy() @@ -334,8 +337,8 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Namespace: resource.Namespace, }, pvc) - Expect(err).To(HaveOccurred()) - Expect(errors.IsNotFound(err)).To(BeTrue()) + Expect(err).NotTo(HaveOccurred()) + Expect(pvc.DeletionTimestamp).NotTo(BeNil()) }) It("should properly encode a feature_store.yaml config", func() { From 32d47ad718cf1d66702c20eb00fb59f0327a3657 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 11 Nov 2024 16:53:42 +0100 Subject: [PATCH 07/12] validating PVC config in applied status Signed-off-by: Daniele Martinoli --- .../featurestore_controller_pvc_test.go | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) 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 9ba4011b94a..56efe8dfdd6 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -22,6 +22,8 @@ import ( "fmt" "path" + apiresource "k8s.io/apimachinery/pkg/api/resource" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "gopkg.in/yaml.v3" @@ -149,6 +151,16 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil()) Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence).NotTo(BeNil()) 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.StorageClassName).To(BeNil()) + expectedResources := corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: apiresource.MustParse("20Gi"), + }, + } + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.Resources).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.Resources).To(Equal(expectedResources)) Expect(resource.Status.Applied.Services.OfflineStore.ImagePullPolicy).To(BeNil()) Expect(resource.Status.Applied.Services.OfflineStore.Resources).To(BeNil()) Expect(resource.Status.Applied.Services.OfflineStore.Image).To(Equal(&services.DefaultImage)) @@ -156,6 +168,16 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(resource.Status.Applied.Services.OnlineStore.Persistence).NotTo(BeNil()) Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence).NotTo(BeNil()) 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.StorageClassName).To(BeNil()) + expectedResources = corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: apiresource.MustParse("5Gi"), + }, + } + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.Resources).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig.Create.Resources).To(Equal(expectedResources)) Expect(resource.Status.Applied.Services.OnlineStore.Env).To(Equal(&[]corev1.EnvVar{{Name: testEnvVarName, Value: testEnvVarValue}, {Name: "fieldRefName", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}})) Expect(resource.Status.Applied.Services.OnlineStore.ImagePullPolicy).To(Equal(&pullPolicy)) Expect(resource.Status.Applied.Services.OnlineStore.Resources).NotTo(BeNil()) @@ -165,6 +187,16 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Expect(resource.Status.Applied.Services.Registry.Local.Persistence).NotTo(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence).NotTo(BeNil()) 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.StorageClassName).To(BeNil()) + expectedResources = corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: apiresource.MustParse("5Gi"), + }, + } + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.Resources).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.Resources).To(Equal(expectedResources)) Expect(resource.Status.Applied.Services.Registry.Local.ImagePullPolicy).To(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Resources).To(BeNil()) Expect(resource.Status.Applied.Services.Registry.Local.Image).To(Equal(&services.DefaultImage)) @@ -317,6 +349,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { err = k8sClient.Get(ctx, typeNamespacedName, resource) Expect(err).NotTo(HaveOccurred()) feast.FeatureStore = resource + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.PvcConfig).To(BeNil()) // check online deployment deploy = &appsv1.Deployment{} @@ -337,8 +370,11 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { Namespace: resource.Namespace, }, pvc) - Expect(err).NotTo(HaveOccurred()) - Expect(pvc.DeletionTimestamp).NotTo(BeNil()) + if err != nil { + Expect(errors.IsNotFound(err)).To(BeTrue()) + } else { + Expect(pvc.DeletionTimestamp).NotTo(BeNil()) + } }) It("should properly encode a feature_store.yaml config", func() { From 398e51dc805d9a41e3bfcc9f0ab8edb6fda31124 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 11 Nov 2024 19:01:09 +0100 Subject: [PATCH 08/12] simplified PVC deletion logic Signed-off-by: Daniele Martinoli --- .../internal/controller/services/services.go | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index f75ba87b13d..219fe58ae07 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -53,11 +53,6 @@ func (feast *FeastServices) Deploy() error { if err := feast.deployFeastServiceByType(OfflineFeastType); err != nil { return err } - if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, OfflineFeastType); !shouldCreate { - if err := feast.deleteOwnedFeastObj(feast.initPVC(OfflineFeastType)); err != nil && !apierrors.IsNotFound(err) { - return err - } - } } else { if err := feast.removeFeastServiceByType(OfflineFeastType); err != nil { return err @@ -68,11 +63,6 @@ func (feast *FeastServices) Deploy() error { if err := feast.deployFeastServiceByType(OnlineFeastType); err != nil { return err } - if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, OnlineFeastType); !shouldCreate { - if err := feast.deleteOwnedFeastObj(feast.initPVC(OnlineFeastType)); err != nil && !apierrors.IsNotFound(err) { - return err - } - } } else { if err := feast.removeFeastServiceByType(OnlineFeastType); err != nil { return err @@ -83,11 +73,6 @@ func (feast *FeastServices) Deploy() error { if err := feast.deployFeastServiceByType(RegistryFeastType); err != nil { return err } - if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, RegistryFeastType); !shouldCreate { - if err := feast.deleteOwnedFeastObj(feast.initPVC(RegistryFeastType)); err != nil && !apierrors.IsNotFound(err) { - return err - } - } } else { if err := feast.removeFeastServiceByType(RegistryFeastType); err != nil { return err @@ -113,6 +98,10 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) if err := feast.createPVC(pvcCreate, feastType); err != nil { return feast.setFeastServiceCondition(err, feastType) } + } else { + if err := feast.deleteOwnedFeastObj(feast.initPVC(feastType)); err != nil && !apierrors.IsNotFound(err) { + return err + } } return feast.setFeastServiceCondition(nil, feastType) } @@ -124,10 +113,8 @@ func (feast *FeastServices) removeFeastServiceByType(feastType FeastServiceType) if err := feast.deleteOwnedFeastObj(feast.initFeastSvc(feastType)); err != nil { return err } - if _, shouldCreate := shouldCreatePvc(feast.FeatureStore, feastType); shouldCreate { - if err := feast.deleteOwnedFeastObj(feast.initPVC(feastType)); err != nil { - return err - } + if err := feast.deleteOwnedFeastObj(feast.initPVC(feastType)); err != nil { + return err } apimeta.RemoveStatusCondition(&feast.FeatureStore.Status.Conditions, FeastServiceConditions[feastType][metav1.ConditionTrue].Type) return nil From 66448533ab6ea62c3d4888c5ce24a709cd45dad2 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 11 Nov 2024 20:56:35 +0100 Subject: [PATCH 09/12] ignoring err when PVC is deleted Signed-off-by: Daniele Martinoli --- infra/feast-operator/internal/controller/services/services.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 219fe58ae07..2819853a314 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -99,9 +99,7 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) return feast.setFeastServiceCondition(err, feastType) } } else { - if err := feast.deleteOwnedFeastObj(feast.initPVC(feastType)); err != nil && !apierrors.IsNotFound(err) { - return err - } + _ = feast.deleteOwnedFeastObj(feast.initPVC(feastType)) } return feast.setFeastServiceCondition(nil, feastType) } From ab395da0d784e9f40d1a28a2660f9fd6febb3f10 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Tue, 12 Nov 2024 22:26:02 +0100 Subject: [PATCH 10/12] integrating latest comments Signed-off-by: Daniele Martinoli --- infra/feast-operator/config/rbac/role.yaml | 1 + ...v1alpha1_featurestore_pvc_persistence.yaml | 6 +- .../controller/featurestore_controller.go | 3 +- .../featurestore_controller_pvc_test.go | 9 ++- .../controller/services/repo_config.go | 81 ++++++++++--------- .../internal/controller/services/services.go | 26 +++--- .../internal/controller/services/util.go | 21 +++-- 7 files changed, 88 insertions(+), 59 deletions(-) diff --git a/infra/feast-operator/config/rbac/role.yaml b/infra/feast-operator/config/rbac/role.yaml index 5ee64d47051..4b19baa663a 100644 --- a/infra/feast-operator/config/rbac/role.yaml +++ b/infra/feast-operator/config/rbac/role.yaml @@ -19,6 +19,7 @@ rules: - "" resources: - configmaps + - persistentvolumeclaims - services verbs: - create diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml index b7c7412c0f0..3e26fa9a021 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml @@ -30,7 +30,11 @@ spec: file: path: registry.db pvc: - create: {} + create: + storageClassName: standard + resources: + requests: + storage: 1Gi mountPath: /data/registry --- apiVersion: v1 diff --git a/infra/feast-operator/internal/controller/featurestore_controller.go b/infra/feast-operator/internal/controller/featurestore_controller.go index 5da9bad3aa6..fc74fcf4121 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller.go +++ b/infra/feast-operator/internal/controller/featurestore_controller.go @@ -53,7 +53,7 @@ type FeatureStoreReconciler struct { //+kubebuilder:rbac:groups=feast.dev,resources=featurestores/status,verbs=get;update;patch //+kubebuilder:rbac:groups=feast.dev,resources=featurestores/finalizers,verbs=update //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;create;update;watch;delete -//+kubebuilder:rbac:groups=core,resources=services;configmaps,verbs=get;list;create;update;watch;delete +//+kubebuilder:rbac:groups=core,resources=services;configmaps;persistentvolumeclaims,verbs=get;list;create;update;watch;delete // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -142,6 +142,7 @@ func (r *FeatureStoreReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.ConfigMap{}). Owns(&appsv1.Deployment{}). Owns(&corev1.Service{}). + Owns(&corev1.PersistentVolumeClaim{}). Watches(&feastdevv1alpha1.FeatureStore{}, handler.EnqueueRequestsFromMapFunc(r.mapFeastRefsToFeastRequests)). Complete(r) } 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 56efe8dfdd6..d469e516c55 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -67,6 +67,8 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { onlineStoreMountPath := "/online" registryMountPath := "/registry" + storageClassName := "default" + onlineStoreMountedPath := path.Join(onlineStoreMountPath, onlineStorePath) registryMountedPath := path.Join(registryMountPath, registryPath) @@ -80,7 +82,9 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{ Type: offlineType, PvcConfig: &feastdevv1alpha1.PvcConfig{ - Create: &feastdevv1alpha1.PvcCreate{}, + Create: &feastdevv1alpha1.PvcCreate{ + StorageClassName: &storageClassName, + }, MountPath: offlineStoreMountPath, }, }, @@ -153,7 +157,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.StorageClassName).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.PvcConfig.Create.StorageClassName).To(Equal(&storageClassName)) expectedResources := corev1.VolumeResourceRequirements{ Requests: corev1.ResourceList{ corev1.ResourceStorage: apiresource.MustParse("20Gi"), @@ -270,6 +274,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { pvc) Expect(err).NotTo(HaveOccurred()) Expect(pvc.Name).To(Equal(deploy.Name)) + Expect(pvc.Spec.StorageClassName).To(Equal(&storageClassName)) Expect(pvc.Spec.Resources.Requests.Storage().String()).To(Equal(services.DefaultOfflineStorageRequest)) Expect(pvc.DeletionTimestamp).To(BeNil()) diff --git a/infra/feast-operator/internal/controller/services/repo_config.go b/infra/feast-operator/internal/controller/services/repo_config.go index e56f311a127..7341271c673 100644 --- a/infra/feast-operator/internal/controller/services/repo_config.go +++ b/infra/feast-operator/internal/controller/services/repo_config.go @@ -54,47 +54,52 @@ func getServiceRepoConfig(feastType FeastServiceType, featureStore *feastdevv1al isLocalRegistry := isLocalRegistry(featureStore) if appliedSpec.Services != nil { services := appliedSpec.Services - // Offline server has an `offline_store` section and a remote `registry` - if feastType == OfflineFeastType && services.OfflineStore != nil { - fileType := string(OfflineDaskConfigType) - if services.OfflineStore.Persistence != nil && - services.OfflineStore.Persistence.FilePersistence != nil && - len(services.OfflineStore.Persistence.FilePersistence.Type) > 0 { - fileType = services.OfflineStore.Persistence.FilePersistence.Type + switch feastType { + case OfflineFeastType: + // Offline server has an `offline_store` section and a remote `registry` + if services.OfflineStore != nil { + fileType := string(OfflineDaskConfigType) + if services.OfflineStore.Persistence != nil && + services.OfflineStore.Persistence.FilePersistence != nil && + len(services.OfflineStore.Persistence.FilePersistence.Type) > 0 { + fileType = services.OfflineStore.Persistence.FilePersistence.Type + } + + repoConfig.OfflineStore = OfflineStoreConfig{ + Type: OfflineConfigType(fileType), + } + repoConfig.OnlineStore = OnlineStoreConfig{} } - - repoConfig.OfflineStore = OfflineStoreConfig{ - Type: OfflineConfigType(fileType), - } - repoConfig.OnlineStore = OnlineStoreConfig{} - } - // Online server has an `online_store` section, a remote `registry` and a remote `offline_store` - if feastType == OnlineFeastType && services.OnlineStore != nil { - path := DefaultOnlineStoreEphemeralPath - if services.OnlineStore.Persistence != nil && services.OnlineStore.Persistence.FilePersistence != nil { - filePersistence := services.OnlineStore.Persistence.FilePersistence - path = getActualPath(filePersistence.Path, filePersistence.PvcConfig) - } - - repoConfig.OnlineStore = OnlineStoreConfig{ - Type: OnlineSqliteConfigType, - Path: path, - } - } - // Registry server only has a `registry` section - if feastType == RegistryFeastType && isLocalRegistry { - path := DefaultRegistryEphemeralPath - if services != nil && services.Registry != nil && services.Registry.Local != nil && - services.Registry.Local.Persistence != nil && services.Registry.Local.Persistence.FilePersistence != nil { - filePersistence := services.Registry.Local.Persistence.FilePersistence - path = getActualPath(filePersistence.Path, filePersistence.PvcConfig) + case OnlineFeastType: + // Online server has an `online_store` section, a remote `registry` and a remote `offline_store` + if services.OnlineStore != nil { + path := DefaultOnlineStoreEphemeralPath + if services.OnlineStore.Persistence != nil && services.OnlineStore.Persistence.FilePersistence != nil { + filePersistence := services.OnlineStore.Persistence.FilePersistence + path = getActualPath(filePersistence.Path, filePersistence.PvcConfig) + } + + repoConfig.OnlineStore = OnlineStoreConfig{ + Type: OnlineSqliteConfigType, + Path: path, + } } - repoConfig.Registry = RegistryConfig{ - RegistryType: RegistryFileConfigType, - Path: path, + case RegistryFeastType: + // Registry server only has a `registry` section + if isLocalRegistry { + path := DefaultRegistryEphemeralPath + if services != nil && services.Registry != nil && services.Registry.Local != nil && + services.Registry.Local.Persistence != nil && services.Registry.Local.Persistence.FilePersistence != nil { + filePersistence := services.Registry.Local.Persistence.FilePersistence + path = getActualPath(filePersistence.Path, filePersistence.PvcConfig) + } + repoConfig.Registry = RegistryConfig{ + RegistryType: RegistryFileConfigType, + Path: path, + } + repoConfig.OfflineStore = OfflineStoreConfig{} + repoConfig.OnlineStore = OnlineStoreConfig{} } - repoConfig.OfflineStore = OfflineStoreConfig{} - repoConfig.OnlineStore = OnlineStoreConfig{} } } diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 2819853a314..665556b57b2 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -259,20 +259,28 @@ func (feast *FeastServices) createNewPVC(pvcCreate *feastdevv1alpha1.PvcCreate, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, Resources: pvcCreate.Resources, } + if pvcCreate.StorageClassName != nil { + pvc.Spec.StorageClassName = pvcCreate.StorageClassName + } return pvc, controllerutil.SetControllerReference(feast.FeatureStore, pvc, feast.Scheme) } func (feast *FeastServices) getServiceConfigs(feastType FeastServiceType) feastdevv1alpha1.ServiceConfigs { appliedSpec := feast.FeatureStore.Status.Applied - if feastType == OfflineFeastType && appliedSpec.Services.OfflineStore != nil { - return appliedSpec.Services.OfflineStore.ServiceConfigs - } - if feastType == OnlineFeastType && appliedSpec.Services.OnlineStore != nil { - return appliedSpec.Services.OnlineStore.ServiceConfigs - } - if feastType == RegistryFeastType && appliedSpec.Services.Registry != nil { - if appliedSpec.Services.Registry.Local != nil { - return appliedSpec.Services.Registry.Local.ServiceConfigs + switch feastType { + case OfflineFeastType: + if appliedSpec.Services.OfflineStore != nil { + return appliedSpec.Services.OfflineStore.ServiceConfigs + } + case OnlineFeastType: + if appliedSpec.Services.OnlineStore != nil { + return appliedSpec.Services.OnlineStore.ServiceConfigs + } + case RegistryFeastType: + if appliedSpec.Services.Registry != nil { + if appliedSpec.Services.Registry.Local != nil { + return appliedSpec.Services.Registry.Local.ServiceConfigs + } } } return feastdevv1alpha1.ServiceConfigs{} diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index 72ea70d8fdb..b68bf7916c3 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -18,14 +18,19 @@ func isLocalRegistry(featureStore *feastdevv1alpha1.FeatureStore) bool { func hasPvcConfig(featureStore *feastdevv1alpha1.FeatureStore, feastType FeastServiceType) (*feastdevv1alpha1.PvcConfig, bool) { services := featureStore.Status.Applied.Services var pvcConfig *feastdevv1alpha1.PvcConfig = nil - if feastType == OnlineFeastType && services.OnlineStore != nil && services.OnlineStore.Persistence.FilePersistence != nil { - pvcConfig = services.OnlineStore.Persistence.FilePersistence.PvcConfig - } - if feastType == OfflineFeastType && services.OfflineStore != nil && services.OfflineStore.Persistence.FilePersistence != nil { - pvcConfig = services.OfflineStore.Persistence.FilePersistence.PvcConfig - } - if feastType == RegistryFeastType && isLocalRegistry(featureStore) && services.Registry.Local.Persistence.FilePersistence != nil { - pvcConfig = services.Registry.Local.Persistence.FilePersistence.PvcConfig + switch feastType { + case OnlineFeastType: + if services.OnlineStore != nil && services.OnlineStore.Persistence.FilePersistence != nil { + pvcConfig = services.OnlineStore.Persistence.FilePersistence.PvcConfig + } + case OfflineFeastType: + if services.OfflineStore != nil && services.OfflineStore.Persistence.FilePersistence != nil { + pvcConfig = services.OfflineStore.Persistence.FilePersistence.PvcConfig + } + case RegistryFeastType: + if isLocalRegistry(featureStore) && services.Registry.Local.Persistence.FilePersistence != nil { + pvcConfig = services.Registry.Local.Persistence.FilePersistence.PvcConfig + } } return pvcConfig, pvcConfig != nil } From 2ae01610d35e0ad7357e34dad764efaa9b9b688d Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Tue, 12 Nov 2024 22:33:35 +0100 Subject: [PATCH 11/12] reverting sample Signed-off-by: Daniele Martinoli --- .../samples/v1alpha1_featurestore_pvc_persistence.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml index 3e26fa9a021..b7c7412c0f0 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml @@ -30,11 +30,7 @@ spec: file: path: registry.db pvc: - create: - storageClassName: standard - resources: - requests: - storage: 1Gi + create: {} mountPath: /data/registry --- apiVersion: v1 From e881aaffae9a7881d3b775b5e8732aa80de80196 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Wed, 13 Nov 2024 07:41:20 +0100 Subject: [PATCH 12/12] renamed storage class to "test" Signed-off-by: Daniele Martinoli --- infra/feast-operator/dist/install.yaml | 1 + .../internal/controller/featurestore_controller_pvc_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 69d3df45342..1be34a9e088 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -2034,6 +2034,7 @@ rules: - "" resources: - configmaps + - persistentvolumeclaims - services verbs: - create 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 d469e516c55..33fdce38fef 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go @@ -67,7 +67,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() { onlineStoreMountPath := "/online" registryMountPath := "/registry" - storageClassName := "default" + storageClassName := "test" onlineStoreMountedPath := path.Join(onlineStoreMountPath, onlineStorePath) registryMountedPath := path.Join(registryMountPath, registryPath)