https://flyte.org logo
#flyte-deployment
Title
# flyte-deployment
y

Yini Gao

02/20/2024, 5:04 PM
Hey👋 I’m trying to upgrade to flyte 1.10.7 using helm chart, and got:
Copy code
Error: template: flyte-core/charts/flyteagent/templates/agent/deployment.yaml:17:17: executing "flyte-core/charts/flyteagent/templates/agent/deployment.yaml" at <include "flyteagent.podLabels" .>: error calling include: template: flyte-core/templates/_helpers.tpl:122:16: executing "flyteagent.podLabels" at <.Values.flyteagent.podLabels>: nil pointer evaluating interface {}.podLabels
The context is that in the
value.yaml
, I set
Copy code
flyteagent:
  enabled: true
While if I remove the
flyteagent
section in
value.yaml
, it got successfully deployed. I’m wondering if it’s relevant to the
podLabels
is not configured in the default values here: https://github.com/flyteorg/flyte/blob/master/charts/flyte-core/values.yaml#L274-L281
d

David Espejo (he/him)

02/20/2024, 5:42 PM
@Yini Gao you're right It seems that we missed it during a recent change. cc @Ethan Brown I guess we can wrap it into a conditional in
helpers.tpl
?
e

Ethan Brown

02/20/2024, 8:46 PM
Ah shoot -- sorry about that! We can either set a default empty
podLabels
value like all the other pods or wrap it in a conditional. I would probably lean towards the former.
Do you expect folks to set
enabled:true
directly in the other sample values.yaml files (control plane, data plane, etc)? If so, it might be better to take your suggestion and address this in the flyte-agent chart to be a little more defensive
d

David Espejo (he/him)

02/20/2024, 9:22 PM
If
values
is for defaults, empty
podLabels
is probably the most common pattern. Users should also be able to override them so adding the logic to
helpers
would be necessary too IMO
e

Ethan Brown

02/20/2024, 9:25 PM
👍
I think for upgrades we're all set too because Helm will merge the latest
values.yaml
defaults from the chart with the values already written / captured in the cluster secret for the (previous) release
d

David Espejo (he/him)

02/20/2024, 9:32 PM
the problem are the other example values files I guess
y

Yini Gao

02/26/2024, 1:41 PM
Hey guys👋I’m wondering if there is any update on this issue.
d

David Espejo (he/him)

02/26/2024, 1:56 PM
@Yini Gao I just merged #4922 as a first step that should prevent this from happening
y

Yini Gao

02/26/2024, 1:59 PM
Awesome🎉 Then when will this fix be released?
d

David Espejo (he/him)

02/26/2024, 2:08 PM
I think the next release should carry this over (1-2 weeks from now) In the meantime you can add the change proposed there to your values file and test
y

Yini Gao

02/26/2024, 2:14 PM
Thanks for the info! While I’ve tried it before asking here😂 I don’t think it would work by adding
podLabels: {}
to value file on my end, since it is the default value that is missing, and overwriting it on my end seems not helpful, am i right?
d

David Espejo (he/him)

02/26/2024, 2:18 PM
right the conditional needs to be added to the chart that expects that field
I'll send a PR later
Or you could if you want 🙂 I think it's a matter of wrapping this line into a conditional so it doesn't break with empty podLabels https://github.com/flyteorg/flyte/blob/d751d0ba83a5c4596c3fa4d3d7de16478de30058/charts/flyteagent/templates/agent/deployment.yaml#L17
e

Ethan Brown

02/26/2024, 2:33 PM
Hmmm I don't think I'm following. Why won't a values override work?
y

Yini Gao

02/26/2024, 2:36 PM
Since override only works when default value is given, and the error I got during rendering is due to no podLabels param found.
@David Espejo (he/him) Given the fix is already merged, is it still necessary to make it conditional? My understanding is all of these fixes are gonna to be available after the new release, right? If so, I’m OK to wait till the next release.
e

Ethan Brown

02/26/2024, 2:38 PM
Yeah, I don't think the conditional is necessary given the default is set in the next release
For some reason I thought user overrides would just merge into computed values, even without a default in values.yaml ... TIL!
y

Yini Gao

02/26/2024, 2:44 PM
Emmm interesting! Here is override I tried, am i missing sth?
Copy code
flyteagent:
  enabled: true
  podLabels: {}
e

Ethan Brown

02/26/2024, 2:45 PM
That looks right to me -- my understanding might just be wrong. If you run Helm with dry-run and verbose it should show you calculated values
d

David Espejo (he/him)

02/26/2024, 3:10 PM
The original error message has to do with the map being empty. If a user enables flyteagent and leaves the podLabels map empty, AFAICT, a conditional would prevent failure
Or is the error message interpreted more about the map not being present at all? After 4 days of travel my mind is fuzzy today 😅
e

Ethan Brown

02/27/2024, 5:14 PM
Yeah, the error is about the map being undefined (nil interface pointer)... which is why setting a default should fix it