Repo type#58
Conversation
|
@audunmg and @gquintard |
|
This needs a bit of cleaning up, some commits from another PR has snuck in here. (external traffic policy) |
|
Now it's just missing tests. |
|
Okay the tests have been added in repotype.bats as I did them using the new test method from @audunmg |
|
I think you have to rebase to main again, and delete the commits which are not related to this PR (externalTrafficPolicy, etc.) |
|
The public repo image is using a different entrypoint with different env variabels, so I think this could be a part of a series of PRs to unify the behavior. |
|
Hey @audunmg, I rebased again and believe I cleaned things up right. Can you take a look? |
75279d7 to
018c9b3
Compare
|
bump @audunmg |
| tee -a /dev/stderr) | ||
|
|
||
| [[ "${actual}" == *"'server.extraVolumeClaimTemplates' cannot be enabled"* ]] | ||
| } |
There was a problem hiding this comment.
Please keep trailing newlines.
audunmg
left a comment
There was a problem hiding this comment.
This still needs work.
Even if i fix the newline in the image, it will not be possible to use on an actual cluster because of how the listening ports and other parameters are configured differently in the different images.
Because this touch on much more than just changing the image, we should probably talk more about how to proceed with this.
| {{- define "varnish-enterprise.image" -}} | ||
| {{- $base := .base | default dict -}} | ||
| {{- $image := .image | default dict -}} | ||
|
|
||
| {{- $repoType := .Values.global.repoType | default "" -}} | ||
| {{- $calculatedRepo := $base.repository -}} | ||
|
|
||
| {{- if eq $repoType "public-enterprise" }} | ||
| {{- $calculatedRepo = "varnish/varnish-enterprise" }} | ||
| {{- else if eq $repoType "private-enterprise" }} | ||
| {{- $calculatedRepo = "quay.io/varnish-software/varnish-plus" }} | ||
| {{ end }} |
There was a problem hiding this comment.
This doesn't work, you're introducing a newline between the image and the tag.
The reason it was all written on one line was probably to avoid introducing newlines.
It wasn't pretty and I prefer to not do it that way.
If you want to keep it on several lines, please indent the code between {{ define }} and {{ end }} to make it more readable.
There was a problem hiding this comment.
Please move these tests to the unit_common test file and change back to the original test format and let's agree on a test style before we change to something like this.
Since the above code broke the varnishncsa image, and not the varnish image please test for that too.
|
|
||
| [[ "${actual}" == *"'server.extraVolumeClaimTemplates' cannot be enabled"* ]] | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Please keep trailing newlines.
Added a feature for defining a repo type that uses the global variable repoType to allow someone to choose "public-enterprise" for "varnish/enterprise" or "private-enterprise" for "quay.io/varnish-software/varnish-plus". I still need to add tests and docs, but I wanted to push this before I go on PTO until Monday.