Skip to content

Repo type#58

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

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

Conversation

@briiians

Copy link
Copy Markdown
Collaborator

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
Collaborator Author

@audunmg and @gquintard

@audunmg

audunmg commented Dec 11, 2025

Copy link
Copy Markdown
Collaborator

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

@audunmg

audunmg commented Dec 12, 2025

Copy link
Copy Markdown
Collaborator

Now it's just missing tests.

@briiians

Copy link
Copy Markdown
Collaborator Author

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

@audunmg

audunmg commented Dec 22, 2025

Copy link
Copy Markdown
Collaborator

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

@audunmg

audunmg commented Dec 23, 2025

Copy link
Copy Markdown
Collaborator

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
Collaborator 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.

@audunmg audunmg left a comment

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 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