allow port ranges in opaque ports environment variable#2080
Closed
hawkw wants to merge 2 commits intoeliza/rm-fixed-policyfrom
Closed
allow port ranges in opaque ports environment variable#2080hawkw wants to merge 2 commits intoeliza/rm-fixed-policyfrom
hawkw wants to merge 2 commits intoeliza/rm-fixed-policyfrom
Conversation
377db61 to
014c443
Compare
Depends on #2079 Currently, the `LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTION` environment variable accepts a comma-delimited list of individual port numbers. Each of these ports is then stored as a separate cache entry in the port policy cache to indicate that the port is opaque. Unfortunately, this does not work well when a very large number of ports are marked as opaque, because the proxy-injector will generate a massive value for that environment variable, listing every individual port. This can cause issues when this manifest becomes larger than the Kubernetes API can reasonably handle. In addition, huge numbers of ports will all be kept in memory as a separate cache entry by the proxy, increasing proxy memory usage. See linkerd/linkerd2#9803 for details. This branch changes the proxy so that the `LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTION` environment variable may contain a list of individual port numbers *and* port ranges, which are specified as `<low>-<high>`. Opaque ports are now stored using a `RangeInclusiveSet` from the `rangemap` crate, rather than by creating individual cache entries for each port. This means we are no longer storing a separate entry in the cache for every port in a range, reducing memory consumption when there are very large ranges of opaque ports. This is the proxy half of the work towards resolving linkerd/linkerd2#9803. Once this branch lands, we'll also have to change the proxy-injector so that it no longer handles opaque port ranges by generating a list of all the individual ports in the range, and simply forwards those ranges as they were specified when generating the environment variable.
0d54f71 to
51e1008
Compare
olix0r
reviewed
Dec 23, 2022
| Self::new(cache, default, Some(discover), opaque_ports) | ||
| } | ||
|
|
||
| fn new( |
Member
There was a problem hiding this comment.
Prefer to make it clear it spawns things.
Suggested change
| fn new( | |
| fn spawn( |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Depends on #2079
Currently, the
LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTIONenvironment variable accepts a comma-delimited list of individual port
numbers. Each of these ports is then stored as a separate cache entry in
the port policy cache to indicate that the port is opaque.
Unfortunately, this does not work well when a very large number of ports
are marked as opaque, because the proxy-injector will generate a massive
value for that environment variable, listing every individual port. This
can cause issues when this manifest becomes larger than the Kubernetes
API can reasonably handle. In addition, huge numbers of ports will all
be kept in memory as a separate cache entry by the proxy, increasing
proxy memory usage. See linkerd/linkerd2#9803 for details.
This branch changes the proxy so that the
LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTIONenvironmentvariable may contain a list of individual port numbers and port
ranges, which are specified as
<low>-<high>.Opaque ports are now stored using a
RangeInclusiveSetfrom therangemapcrate, rather than by creating individual cache entries foreach port. This means we are no longer storing a separate entry in the
cache for every port in a range, reducing memory consumption when there
are very large ranges of opaque ports.
This is the proxy half of the work towards resolving
linkerd/linkerd2#9803. Once this branch lands, we'll also have to change
the proxy-injector so that it no longer handles opaque port ranges by
generating a list of all the individual ports in the range, and simply
forwards those ranges as they were specified when generating the
environment variable.