Use new istio-iptables cleanup methodology in VMs #52918
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
|
Skipping CI for Draft Pull Request. |
istio-testing
added
the
do-not-merge/work-in-progress
istio-policy-bot
added
area/environments
area/networking
release-notes-none
istio-testing
added
size/XS
istio-testing
added
size/XS
istio-testing
added
size/XS
leosarra
changed the title
istio-testing
removed
the
do-not-merge/work-in-progress
leosarra
commented
Aug 30, 2024
| @@ -66,7 +66,7 @@ if [ "${ISTIO_CUSTOM_IP_TABLES}" != "true" ] ; then | |||
| if [[ ${1-} == "init" || ${1-} == "-p" ]] ; then | |||
There was a problem hiding this comment.
I can't find where this codepath is used but I adjusted it anyway. Perhaps it is something old we don't need to support anymore?
leosarra
commented
Aug 30, 2024
| @@ -77,7 +77,7 @@ if [ "${ISTIO_CUSTOM_IP_TABLES}" != "true" ] ; then | |||
| if [[ ${1-} != "run" ]] ; then | |||
| # clean the previous Istio iptables chains. This part is different from the init image mode, | |||
| # where the init container runs in a fresh environment and there cannot be previous Istio chains | |||
| "${ISTIO_BIN_BASE}/pilot-agent" istio-clean-iptables | |||
| "${ISTIO_BIN_BASE}/pilot-agent" istio-iptables --cleanup-only | |||
There was a problem hiding this comment.
This could have been simplified in one ""${ISTIO_BIN_BASE}/pilot-agent" istio-iptables --reconcile" cmd but I'm keeping the "preemptive cleanup"+apply step separate to be as consistent as possible with the old handling
howardjohn
approved these changes
Aug 30, 2024
2 tasks
luksa
pushed a commit
to luksa/istio
that referenced
this pull request
Oct 14, 2024
* upstream/master: Automator: update proxy@master in istio/istio@master (istio#52957) Add/Increase retry timeout due to delay in AWS config propagation (istio#52938) Apply local weight lb only when locality lb is enabled (istio#52898) Fix sending to external service entry from a VS (istio#52589) Automator: update proxy@master in istio/istio@master (istio#52946) Add option to exclude CRDs during install (istio#52947) Use new istio-iptables cleanup methodology in VMs (istio#52918)
16 tasks
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.
This update will change the current handling of start/stop/clean operations in VMs to rely on
istio-iptables --cleanup-onlyinstead of the oldistio-clean-iptablescommand.istio-iptables --cleanup-onlycomputes the rules that need to be deleted in a "dynamic" way based on what the command would have applied if the cleanup option was not used.On the other hand,
istio-clean-iptablesrelied on hardcoded logic which was separate from the one used to create rules in istio-iptables. This approach required constant maintenance to ensure that the two different tools did not diverge over time.This change removes the need to maintain
istio-clean-iptablesand prevents issues similar to #52835 and #41431 from occurring.I'm not sure whether the istio-clean-iptables source code in the repo should be removed right away or not. We could keep it around for 1-2 releases in case someone is using it outside of our scripts but that would entail that we still need to support it a bit more in case of changes in istio-iptables rules.