Skip to content

Conversation

@bcsgh
Copy link
Contributor

@bcsgh bcsgh commented Jan 3, 2026

Address part 2 of #226

  • Duplicate the values argument from helm_template into helm_template_test.
  • Update a test of helm_template_test to uses values and also so that it detects that the extra values files are in fact used.

@bcsgh bcsgh marked this pull request as ready for review January 4, 2026 00:35
Copy link
Collaborator

@abrisco abrisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@bcsgh
Copy link
Contributor Author

bcsgh commented Jan 5, 2026

I kinda suspect all the windows CI issues are a result of the paths not being normalized which might (based on where things fail) to be a preexisting issue. Unfortunately, I don't have a windows machine I can test on so this could be a pain for me to track down.

@bcsgh bcsgh force-pushed the bcsgh/226.2.1-helm_template_test.values branch 2 times, most recently from 8c0eeb1 to e8fd858 Compare January 5, 2026 03:31
@bcsgh
Copy link
Contributor Author

bcsgh commented Jan 5, 2026

I think the proper solution to the windows breakage is to wrap the values paths as rlocationpath(values, ctx.workspace_name) but that seems to break think on Linux where I can test them so I'm wondering is the rlocationpath is incorrect when the file is a local source file. (I've had some fun in a different project fighting with all the permutations and how they get munged.)

@abrisco Do you have direct access to a Windows environment? Any way I can talk you into taking a look at this?

@abrisco
Copy link
Collaborator

abrisco commented Jan 5, 2026

I think the proper solution to the windows breakage is to wrap the values paths as rlocationpath(values, ctx.workspace_name) but that seems to break think on Linux where I can test them so I'm wondering is the rlocationpath is incorrect when the file is a local source file. (I've had some fun in a different project fighting with all the permutations and how they get munged.)

@abrisco Do you have direct access to a Windows environment? Any way I can talk you into taking a look at this?

I do not have a windows environment. All windows development has been done with github actions and prayer. Though I don't think the issue is too mysterious

==================== Test output for //tests/template_with_values:template_with_values_template_test:
(03:33:37) INFO: From Testing //tests/template_with_values:template_with_values_template_test:
Error: open tests\template_with_values\template_values.yaml: The system cannot find the path specified.
2026/01/05 03:33:37 Template not found in the helm chart: template-with-values/templates/deployment.yaml

This seems like parseHelmOutput might need to sanitize windows paths to unix?

@bcsgh
Copy link
Contributor Author

bcsgh commented Jan 5, 2026

[...] I don't think the issue is too mysterious

==================== Test output for //tests/template_with_values:template_with_values_template_test:
(03:33:37) INFO: From Testing //tests/template_with_values:template_with_values_template_test:
Error: open tests\template_with_values\template_values.yaml: The system cannot find the path specified.
2026/01/05 03:33:37 Template not found in the helm chart: template-with-values/templates/deployment.yaml

This seems like parseHelmOutput might need to sanitize windows paths to unix?

The missing path isn't a template file but rather template_values.yaml which shouldn't show up in the helm output. I think the second error cascades from the first, likely due to the templated config ending up empty (which kind of suggests that the runner isn't detecting that helm failed). And from what I can tell, helm handles paths internally as always using / (I pushed several commits that tried variations on path munging but they didn't un-break anything).

@bcsgh bcsgh force-pushed the bcsgh/226.2.1-helm_template_test.values branch from e8fd858 to 65fa62e Compare January 10, 2026 22:24
@bcsgh bcsgh force-pushed the bcsgh/226.2.1-helm_template_test.values branch from 65fa62e to 6da49dd Compare January 18, 2026 15:12
@bcsgh
Copy link
Contributor Author

bcsgh commented Jan 18, 2026

PTAL.

I've duplicated the path munging in runner.go from the chart path to the values paths (and re-plumbed things so that the values paths aren't directly forwarded to helm).

@abrisco abrisco merged commit 89ab770 into periareon:main Jan 18, 2026
13 checks passed
@abrisco
Copy link
Collaborator

abrisco commented Jan 18, 2026

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants