Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSSM-1430: CNI: Watch for modified files with a prefix #510

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

jwendell
Copy link
Member

Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names start with "v2-2". This avoid this race
condition and achieve the same end result.

Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
@jwendell
Copy link
Member Author

/retest

Copy link
Contributor

@dgn dgn left a comment

Choose a reason for hiding this comment

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

LGTM to unblock 2.2. For upstream, we'll have to submit a different PR though, as they don't have the CNIBinariesPrefix field in InstallConfig. We should file a follow-up ticket to do that, so we don't have to carry over this change indefinitely

// Creates a file watcher that watches for any changes to the directory
func CreateFileWatcher(dirs ...string) (watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why replace ...string with []string?

Copy link
Member Author

Choose a reason for hiding this comment

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

because if we have two arguments, first of type string and second of type ...string, go interprets it as a single one as of type ...string, and thus accepts only one argument as a result. https://go.dev/play/p/HuASlAWYUl2

@jwendell
Copy link
Member Author

LGTM to unblock 2.2. For upstream, we'll have to submit a different PR though, as they don't have the CNIBinariesPrefix field in InstallConfig. We should file a follow-up ticket to do that, so we don't have to carry over this change indefinitely

That's interesting. We already carry over this whole prefix thing. I was wondering if we should submit all these changes to upstream as an enhancement. I think it's a good addition even if they don't use it. The fact that we use and are willing to maintain it is a good argument. @luksa any thougths?

@maistra-bot maistra-bot merged commit 2e7c1f9 into maistra:maistra-2.2 Apr 28, 2022
@luksa
Copy link
Contributor

luksa commented Apr 28, 2022

I don't think upstream will want the prefix stuff, since they don't need it and it makes things quite more complex. But I'm not against pushing all this upstream.

@dgn
Copy link
Contributor

dgn commented Apr 29, 2022

I don't think upstream will want the prefix stuff, since they don't need it and it makes things quite more complex. But I'm not against pushing all this upstream.

+1, I'm skeptical they will merge unused/dead code - I wouldn't want it either. This fix however could be relevant, even though the issue technically only materializes when you have multiple CNI plugins "misbehaving" like this (ie copying everything into the folders on watch events even though nothing on the ground has actually changed)

jewertow pushed a commit to jewertow/istio that referenced this pull request Jul 4, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
maistra-bot pushed a commit that referenced this pull request Jul 5, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 24, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 25, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 25, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 25, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 29, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 29, 2022
Because our CNI pod contains more than one container, and they write to
the same directory, and they watch for changes on those directories,
changes made by one container trigger the watch on the other, which will
responde by copying the files to the directory, which will in turn
trigger the watcher of the other container in an endless loop.

This leads to high CPU usage on the node.

This PR changes the logic to only monitor for files that have the
desired prefix. Thus, for example, the 2.2 container will only react to
changes to files whose names  start with "v2-2". This avoid this race
condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 29, 2022
…Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 29, 2022
…Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.
maistra-bot pushed a commit that referenced this pull request Aug 30, 2022
…Istio CNI (#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.
bartoszmajsak pushed a commit to bartoszmajsak/istio that referenced this pull request Nov 18, 2022
…Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.
maistra-bot pushed a commit that referenced this pull request Nov 24, 2022
…Istio CNI (#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 31, 2023
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
jewertow pushed a commit to jewertow/istio that referenced this pull request Aug 31, 2023
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
openshift-merge-robot pushed a commit that referenced this pull request Sep 1, 2023
…Istio CNI (#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 21, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 22, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
yxun pushed a commit to yxun/istio that referenced this pull request May 22, 2024
…Istio CNI (maistra#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (maistra#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (maistra#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (maistra#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (maistra#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (maistra#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
openshift-merge-bot bot pushed a commit that referenced this pull request May 22, 2024
…Istio CNI (#688)

* [cni] MAISTRA-2132 Support deployment of multiple plugin versions in Istio CNI (#271)

Includes:

  * MAISTRA-2135 Add unit tests for our CNI binary-prefix work (#325)

  * MAISTRA-2137 Make network namespace setup executable name configurable (#273)

    To support the deployment of multiple CNI plugin versions, the name of the
    executable that is invoked to set up the network namespace must be configurable.

  * OSSM-1430: CNI: Watch for modified files with a prefix (#510)

    Because our CNI pod contains more than one container, and they write to
    the same directory, and they watch for changes on those directories,
    changes made by one container trigger the watch on the other, which will
    responde by copying the files to the directory, which will in turn
    trigger the watcher of the other container in an endless loop.

    This leads to high CPU usage on the node.

    This PR changes the logic to only monitor for files that have the
    desired prefix. Thus, for example, the 2.2 container will only react to
    changes to files whose names  start with "v2-2". This avoid this race
    condition and achieve the same end result.

* [cni] MAISTRA-2051 use correct UID/GID in istio-iptables

* OSSM-2082 CNI installer now creates the net.d directory if necessary (#638)

* fix(lint): replaces deprecated pkg io/ioutil

* fix: reverts back to t.TempDir() calls

Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: rcernich <rcernich@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants