helm: document missing bpf helm option #21650
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
kaworu
reviewed
Oct 11, 2022
There was a problem hiding this comment.
Hi @vincentmli and thanks for the PR.
The kindIs "invalid" Helm tests look a bit odd, any reason not to set the default values into Helm directly instead (e.g. masquerade: false)?
kaworu
added
kind/enhancement
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
sorry I should have maybe included previous PR discussion, it is suggested by @gandro here is the discussion #21195 (comment) and #21195 (comment) |
|
@kaworu I updated the commit message to give some context on the |
gandro
requested changes
Oct 11, 2022
gandro
reviewed
Oct 12, 2022
vincentmli
force-pushed
the
vli-doc
branch
3 times, most recently
from
0b6de80
to
ab6b40f
Compare
kaworu
approved these changes
Oct 12, 2022
add missing bpf.hostLegacyRouting, bpf.tproxy, bpf.vlanBypass in helm reference and helm documents. remove bpf.lbBypassFIBLookup, hostRouting when original commented helm option is uncommented, the helm template generated manifest will include the new uncommented helm option even helm template does not specify the new uncommented option, this behavior could affect unknown effect, see cilium#21195 (comment) For bool type helm option, gandro suggested (not (eq nil $value)) check, but it didn't work. see cilium#21195 (comment) Eventually gandro suggested the {{- if (not (kindIs "invalid" .Values.bpf.tproxy)) }} trick found in Masterminds/sprig#53 (comment) see: cilium#21195 (comment) cilium#21195 (comment) Signed-off-by: Vincent Li <v.li@f5.com>
squeed
approved these changes
Oct 18, 2022
squeed
added
the
ready-to-merge
|
This is a nice cleanup - thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add missing bpf.lbBypassFIBLookup, bpf.hostLegacyRouting bpf.tproxy, bpf.vlanBypass in helm reference and helm documents
Signed-off-by: Vincent Li v.li@f5.com
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXXline if the commit addresses a particularGitHub issue.
Fixes: <commit-id>tag, thenplease add the commit author[s] as reviewer[s] to this issue.
Fixes: #issue-number