Skip to content

Repo type#58

Open
briiians wants to merge 3 commits intovarnish:mainfrom
briiians:repo-type
Open

Repo type#58
briiians wants to merge 3 commits intovarnish:mainfrom
briiians:repo-type

Conversation

@briiians
Copy link
Copy Markdown
Contributor

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.

@briiians
Copy link
Copy Markdown
Contributor Author

@audunmg and @gquintard

@audunmg
Copy link
Copy Markdown
Collaborator

audunmg commented Dec 11, 2025

This needs a bit of cleaning up, some commits from another PR has snuck in here. (external traffic policy)

@audunmg
Copy link
Copy Markdown
Collaborator

audunmg commented Dec 12, 2025

Now it's just missing tests.

@briiians
Copy link
Copy Markdown
Contributor Author

Okay the tests have been added in repotype.bats as I did them using the new test method from @audunmg

@audunmg
Copy link
Copy Markdown
Collaborator

audunmg commented Dec 22, 2025

I think you have to rebase to main again, and delete the commits which are not related to this PR (externalTrafficPolicy, etc.)

@audunmg
Copy link
Copy Markdown
Collaborator

audunmg commented Dec 23, 2025

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.

@briiians
Copy link
Copy Markdown
Contributor Author

Hey @audunmg, I rebased again and believe I cleaned things up right. Can you take a look?

@briiians briiians force-pushed the repo-type branch 2 times, most recently from 75279d7 to 018c9b3 Compare December 23, 2025 19:02
@gquintard
Copy link
Copy Markdown
Collaborator

bump @audunmg

tee -a /dev/stderr)

[[ "${actual}" == *"'server.extraVolumeClaimTemplates' cannot be enabled"* ]]
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please keep trailing newlines.

Copy link
Copy Markdown
Collaborator

@audunmg audunmg left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +38 to +49
{{- 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 }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please keep trailing newlines.

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.

3 participants