bjw-s / helm-charts

A collection of Helm charts
https://bjw-s.github.io/helm-charts/
Apache License 2.0
524 stars 98 forks source link

[v3.0.3] Probe with no service causes template error #293

Closed gabe565 closed 3 months ago

gabe565 commented 3 months ago

Details

What steps did you take and what happened:

After the v3.0.3 release, enabling probes causes an error. I'm still debugging to find the source of the issue, but I thought I'd go ahead and open this.

Error: template: app-template/templates/common.yaml:14:3: executing "app-template/templates/common.yaml" at <include "bjw-s.common.loader.generate" .>: error calling include: template: app-template/charts/common/templates/loader/_generate.tpl:11:6: executing "bjw-s.common.loader.generate" at <include "bjw-s.common.render.controllers" .>: error calling include: template: app-template/charts/common/templates/render/_controllers.tpl:25:12: executing "bjw-s.common.render.controllers" at <include "bjw-s.common.class.deployment" (dict "rootContext" $ "object" $deploymentObject)>: error calling include: template: app-template/charts/common/templates/classes/_deployment.tpl:63:13: executing "bjw-s.common.class.deployment" at <include "bjw-s.common.lib.pod.spec" (dict "rootContext" $rootContext "controllerObject" $deploymentObject)>: error calling include: template: app-template/charts/common/templates/lib/pod/_spec.tpl:61:12: executing "bjw-s.common.lib.pod.spec" at <include "bjw-s.common.lib.pod.field.containers" (dict "ctx" $ctx)>: error calling include: template: app-template/charts/common/templates/lib/pod/fields/_containers.tpl:24:30: executing "bjw-s.common.lib.pod.field.containers" at <include "bjw-s.common.lib.container.spec" (dict "rootContext" $rootContext "controllerObject" $controllerObject "containerObject" $containerObject)>: error calling include: template: app-template/charts/common/templates/lib/container/_spec.tpl:45:12: executing "bjw-s.common.lib.container.spec" at <include "bjw-s.common.lib.container.field.probes" (dict "ctx" $ctx)>: error calling include: template: app-template/charts/common/templates/lib/container/fields/_probes.tpl:32:43: executing "bjw-s.common.lib.container.field.probes" at <include "bjw-s.common.lib.service.primaryPort" (dict "rootContext" $rootContext "serviceObject" $primaryService)>: error calling include: template: app-template/charts/common/templates/lib/service/_primary_port.tpl:21:36: executing "bjw-s.common.lib.service.primaryPort" at <$firstPortKey>: invalid value; expected string

What did you expect to happen:

Anything else you would like to add:

Additional Information:

bjw-s commented 3 months ago

That's odd, as all the unit tests ran succesfully 🤔 I will probably need to add some guards somewhere in the chart persistence check

Can you perhaps share the values that you are using that run into this error?

gabe565 commented 3 months ago

Yeah I was thinking the same thing! My repro values in #289 causes the error. So far, I'm thinking the $firstPortKey should be checked before the get call:

diff --git a/charts/library/common/templates/lib/service/_primary_port.tpl b/charts/library/common/templates/lib/service/_primary_port.tpl
index d921be1..9716847 100644
--- a/charts/library/common/templates/lib/service/_primary_port.tpl
+++ b/charts/library/common/templates/lib/service/_primary_port.tpl
@@ -18,7 +18,9 @@ Return the primary port for a given Service object.
   {{- /* Return the first port if none has been explicitly marked as primary */ -}}
   {{- if not $result -}}
     {{- $firstPortKey := keys $enabledPorts | first -}}
-    {{- $result = get $enabledPorts $firstPortKey -}}
+    {{- if $firstPortKey -}}
+      {{- $result = get $enabledPorts $firstPortKey -}}
+    {{- end -}}
   {{- end -}}

   {{- $result | toYaml -}}

After that, a prettier error is rendered:

Error: execution error at (app-template/templates/common.yaml:14:3): No enabled controller found with this identifier. (service: 'home-assistant', controller: 'main')

I'm still troubleshooting why the service isn't being found. Possibly because I don't have explicit enabled keys? I'll keep you posted.

Edit: Oh duh, my service controller was main instead of home-assistant, so I think that change should be enough. This will just be an error handling change more than anything.

bjw-s commented 3 months ago

The enabled keys shouldn't really be required though, since Services are set to auto-enabled by default. Thanks for the values ref, I'll try and see what's up as well

gabe565 commented 3 months ago

See my edit above. You're right about enabled not being the culprit. I just had a misconfiguration from changing controllers.main to controllers.home-assistant. The only "issue" is that $firstPortKey isn't checked before it gets used.

bjw-s commented 3 months ago

Nice catch once again :D I don't think I want to re-release 3.0.3 for this but I also don't want to do a 3.0.4 just for this either so I'll keep the fix in the 3.0.4 bank for a bit before releasing.

gabe565 commented 3 months ago

Haha thanks! I agree, makes total sense to keep this for a future release once there are more changes.