Hey:wave: I’m trying to upgrade to flyte 1.10.7 us...
# flyte-deployment
y
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
@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
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
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
👍
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
the problem are the other example values files I guess
y
Hey guys👋I’m wondering if there is any update on this issue.
d
@Yini Gao I just merged #4922 as a first step that should prevent this from happening
y
Awesome🎉 Then when will this fix be released?
d
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
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
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
Hmmm I don't think I'm following. Why won't a values override work?
y
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
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
Emmm interesting! Here is override I tried, am i missing sth?
Copy code
flyteagent:
  enabled: true
  podLabels: {}
e
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
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
Yeah, the error is about the map being undefined (nil interface pointer)... which is why setting a default should fix it