Skip to content

Commit a9363f5

Browse files
Adding the Volume and volumeMount code review comments.
Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com>
1 parent 6ef3efe commit a9363f5

2 files changed

Lines changed: 33 additions & 12 deletions

File tree

infra/feast-operator/api/v1alpha1/featurestore_types.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ type FeatureStoreServices struct {
7878
DeploymentStrategy *appsv1.DeploymentStrategy `json:"deploymentStrategy,omitempty"`
7979
// Disable the 'feast repo initialization' initContainer
8080
DisableInitContainers bool `json:"disableInitContainers,omitempty"`
81-
// Volumes specifies the list of volumes to mount in the FeatureStore deployment.
82-
// +optional
81+
// Volumes specifies the volumes to mount in the FeatureStore deployment. A corresponding `VolumeMount` should be added to whichever feast service(s) require access to said volume(s).
82+
//+optional
8383
Volumes []corev1.Volume `json:"volumes,omitempty"`
8484
}
8585

@@ -92,7 +92,11 @@ type OfflineStore struct {
9292
// Allowed values: "debug", "info", "warning", "error", "critical".
9393
// +kubebuilder:validation:Enum=debug;info;warning;error;critical
9494
LogLevel string `json:"logLevel,omitempty"`
95-
95+
// VolumeMounts defines the list of volumes that should be mounted into the container.
96+
// This allows attaching persistent storage, config files, secrets, or other resources
97+
// required by the Feast components. Ensure that each volume mount has a corresponding
98+
// volume definition in the Volumes field.
99+
//+optional
96100
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
97101
}
98102

@@ -148,6 +152,11 @@ type OnlineStore struct {
148152
// +kubebuilder:validation:Enum=debug;info;warning;error;critical
149153
LogLevel string `json:"logLevel,omitempty"`
150154

155+
// VolumeMounts defines the list of volumes that should be mounted into the container.
156+
// This allows attaching persistent storage, config files, secrets, or other resources
157+
// required by the Feast components. Ensure that each volume mount has a corresponding
158+
// volume definition in the Volumes field.
159+
//+optional
151160
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
152161
}
153162

@@ -205,6 +214,12 @@ type LocalRegistryConfig struct {
205214
// Allowed values: "debug", "info", "warning", "error", "critical".
206215
// +kubebuilder:validation:Enum=debug;info;warning;error;critical
207216
LogLevel string `json:"logLevel,omitempty"`
217+
// VolumeMounts defines the list of volumes that should be mounted into the container.
218+
// This allows attaching persistent storage, config files, secrets, or other resources
219+
// required by the Feast components. Ensure that each volume mount has a corresponding
220+
// volume definition in the Volumes field.
221+
//+optional
222+
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
208223
}
209224

210225
// RegistryPersistence configures the persistence settings for the registry service
@@ -274,9 +289,8 @@ type PvcCreate struct {
274289
// Registry configures the registry service. One selection is required. Local is the default setting.
275290
// +kubebuilder:validation:XValidation:rule="[has(self.local), has(self.remote)].exists_one(c, c)",message="One selection required."
276291
type Registry struct {
277-
Local *LocalRegistryConfig `json:"local,omitempty"`
278-
Remote *RemoteRegistryConfig `json:"remote,omitempty"`
279-
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
292+
Local *LocalRegistryConfig `json:"local,omitempty"`
293+
Remote *RemoteRegistryConfig `json:"remote,omitempty"`
280294
}
281295

282296
// RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR.

infra/feast-operator/internal/controller/services/services.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,13 @@ func (feast *FeastServices) createPVC(pvcCreate *feastdevv1alpha1.PvcCreate, fea
325325
}
326326

327327
func (feast *FeastServices) setDeployment(deploy *appsv1.Deployment) error {
328+
var volumes []corev1.Volume
329+
if feast.Handler.FeatureStore.Spec.Services != nil {
330+
volumes = feast.Handler.FeatureStore.Spec.Services.Volumes
331+
}
332+
if volumes == nil {
333+
volumes = []corev1.Volume{} // Ensure it's an empty slice instead of nil
334+
}
328335
deploy.Labels = feast.getLabels()
329336
deploy.Spec = appsv1.DeploymentSpec{
330337
Replicas: &DefaultReplicas,
@@ -336,7 +343,7 @@ func (feast *FeastServices) setDeployment(deploy *appsv1.Deployment) error {
336343
},
337344
Spec: corev1.PodSpec{
338345
ServiceAccountName: feast.initFeastSA().Name,
339-
Volumes: feast.Handler.FeatureStore.Spec.Services.Volumes,
346+
Volumes: volumes,
340347
},
341348
},
342349
}
@@ -432,19 +439,19 @@ func (feast *FeastServices) getVolumeMounts(feastType FeastServiceType) (volumeM
432439

433440
switch feastType {
434441
case OfflineFeastType:
435-
if feast.isOfflinStore() && appliedServices.OfflineStore != nil {
442+
if feast.isOfflinStore() {
436443
return appliedServices.OfflineStore.VolumeMounts
437444
}
438445
case OnlineFeastType:
439-
if feast.isOnlinStore() && appliedServices.OnlineStore != nil {
446+
if feast.isOnlinStore() {
440447
return appliedServices.OnlineStore.VolumeMounts
441448
}
442449
case RegistryFeastType:
443-
if feast.isLocalRegistry() && appliedServices.Registry != nil {
444-
return appliedServices.Registry.VolumeMounts
450+
if feast.isLocalRegistry() {
451+
return appliedServices.Registry.Local.VolumeMounts
445452
}
446453
case UIFeastType:
447-
if feast.isUI() && appliedServices.UI != nil {
454+
if feast.isUI() {
448455
return appliedServices.UI.VolumeMounts
449456
}
450457
}

0 commit comments

Comments
 (0)