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.

Sign up for GitHub

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

Jump to bottom

proxy: Do not change configured proxy ports #26343

Merged
merged 1 commit into from
Jun 22, 2023
Merged

proxy: Do not change configured proxy ports #26343

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 9 additions & 7 deletions pkg/proxy/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,17 +561,19 @@ func (p *Proxy) CreateOrUpdateRedirect(ctx context.Context, l4 policy.ProxyPolic

for nRetry := 0; nRetry < redirectCreationAttempts; nRetry++ {
if nRetry > 0 {
// an error occurred and we can retry
// an error occurred and we are retrying
scopedLog.WithError(err).Warningf("Unable to create %s proxy, retrying", ppName)
// Do not increment port for DNS when the port is set in config
if pp.proxyType != ProxyTypeDNS || option.Config.ToFQDNsProxyPort == 0 {
}
if !pp.configured {
Copy link
Member

Choose a reason for hiding this comment

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

Follow up to my question in the PR discussion:

Am i correct that this logic still increments the proxy port in case of an initial redirect creation if an Envoy startup error occurs in createEnvoyRedirect (e.g. memory pressure in case of embedded mode). In this case pp.reservePort() , which would set pp.configured=true, will not be called (only if no error occured).

if nRetry > 0 {
// Retry with a new proxy port in case there was a conflict with the
// previous one when the port has not been `configured` yet.
// The incremented port number here is just a hint to allocatePort()
// below, it will check if it is available for use.
pp.proxyPort++
}
}

if !pp.configured {
// Try allocate (the configured) port, but only if the proxy has not
// been already configured.
// Check if pp.proxyPort is available and find an another available proxy port if not.
pp.proxyPort, err = allocatePort(pp.proxyPort, p.rangeMin, p.rangeMax)
if err != nil {
return 0, err, nil, nil
Expand Down