feast-dev / feast

The Open Source Feature Store for Machine Learning
https://feast.dev
Apache License 2.0
5.62k stars 1k forks source link

feat: Added support for setting env vars in feast services in feast controller #4739

Closed tmihalac closed 2 weeks ago

tmihalac commented 2 weeks ago

What this PR does / why we need it:

Allow users to add or override the container env vars for each deployed feast service using the new env list in the CR

Which issue(s) this PR fixes:

Relates to https://github.com/feast-dev/feast/issues/4561

For example:

let's say the operator sets the offline container env vars like this by default -

env:
  - name: FEATURE_STORE_YAML_BASE64
    value: <string>
  - name: VAR1
    value: test

If the user deployed the following CR -

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: sample
spec:
  feastProject: my_project
  services:
    offlineStore:
      env:
       - name: VAR1
         value: override
       - name: VAR2
         value: demo

The resulting offline container env vars would look like this -

      env:
        - name: FEATURE_STORE_YAML_BASE64
          value: <string>
        - name: VAR1
          value: override
        - name: VAR2
          value: demo
tchughesiv commented 2 weeks ago

@tmihalac looks like one of the unit tests are failing

tmihalac commented 2 weeks ago

@tchughesiv Yes the in deploy.Spec.Template.Spec.Containers[0].Env the APIVersion parameter is initialized with v1 and in the spec I do not set that parameter

result = {v1.EnvVar} 
 Name = {string} "fieldRefName"
 Value = {string} ""
 ValueFrom = {*v1.EnvVarSource | 0xc0004a6080} 
  FieldRef = {*v1.ObjectFieldSelector | 0xc0004a60a0} 
   APIVersion = {string} "v1" <-
   FieldPath = {string} "metadata.namespace"
  ResourceFieldRef = {*v1.ResourceFieldSelector | 0x0} nil
  ConfigMapKeyRef = {*v1.ConfigMapKeySelector | 0x0} nil
  SecretKeyRef = {*v1.SecretKeySelector | 0x0} nil

So they are not equal, I am thinking of reverting the deepEquals