istio-cni - use templated env from cm, simplify config
#51050
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
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
istio-policy-bot
added
area/ambient
istio-testing
added
the
size/XL
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
bleggett
force-pushed
the
bleggett/cni-config-cleanup
branch
from
1269a6c
to
bc501fc
Compare
bleggett
added
area/networking/cni
howardjohn
reviewed
May 15, 2024
bleggett
force-pushed
the
bleggett/cni-config-cleanup
branch
from
0549f54
to
09909a3
Compare
bleggett
force-pushed
the
bleggett/cni-config-cleanup
branch
2 times, most recently
from
55c977c
to
d2e18f8
Compare
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
bleggett
force-pushed
the
bleggett/cni-config-cleanup
branch
from
d2e18f8
to
0cf6736
Compare
ilrudie
reviewed
May 17, 2024
|
|
||
| b.WriteString("LogLevel: " + c.LogLevel + "\n") | ||
| b.WriteString("KubeconfigFilename: " + c.KubeconfigFilename + "\n") | ||
| b.WriteString("KubeconfigMode: " + fmt.Sprintf("%#o", c.KubeconfigMode) + "\n") | ||
| b.WriteString("KubeCAFile: " + c.KubeCAFile + "\n") | ||
| b.WriteString("SkipTLSVerify: " + fmt.Sprint(c.SkipTLSVerify) + "\n") | ||
|
|
||
| b.WriteString("ExcludeNamespaces: " + fmt.Sprint(c.ExcludeNamespaces) + "\n") |
There was a problem hiding this comment.
Is K8sServiceAccountPath omitted on purpose?
There was a problem hiding this comment.
Sorta. It's only override-able via unit tests so echoing it would just be noise.
| // disable monitoring & repair | ||
| viper.Set(constants.MonitoringPort, 0) | ||
| viper.Set(constants.RepairEnabled, false) | ||
|
|
||
| // Don't set the CNI conf file env var if preConfFile is not set |
There was a problem hiding this comment.
Suggested change
| // Don't set the CNI conf file env var if preConfFile is not set |
There was a problem hiding this comment.
Yeah it's still set but not an env var anymore.
howardjohn
approved these changes
May 17, 2024
|
this broke old k8s versions (1.23 Nd 1.24) https://prow.istio.io/view/gs/istio-prow/logs/integ-k8s-123_istio_postsubmit/1791587628840128512 |
#51135 should fix. |
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.
Please provide a description of this PR:
Our handling of
istio-cniconfig is rather backwards - this attempts to decruftify it a bit.envFromcnilibraries.