Skip to content

Commit

Permalink
envoy: Keep track of proxy listeners separately
Browse files Browse the repository at this point in the history
Since the addition of Envoy prometheus listener it has been possible
to have non-proxy listeners configured with Envoy. Waiting for Envoy
N/ACKs must be disabled when no proxy listeners are configured, even
if a prometheus listener may still be configured.

Without this fix adding endpoints may fail due to not receiving N/ACKs
from Envoy after Envoy has been started due to an L7 network policy,
and this policy is removed, if the Cilium option
'--proxy-prometheus-port' is also configured.

Fixes: #12949
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
  • Loading branch information
jrajahalme authored and aditighag committed Jul 12, 2021
1 parent c2cbbf0 commit 099c34d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
32 changes: 26 additions & 6 deletions pkg/envoy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ type XDSServer struct {
// Value holds the number of redirects using the listener named by the key.
listeners map[string]*Listener

// proxyListeners is the count of redirection proxy listeners in 'listeners'.
// When this is zero, cilium should not wait for NACKs/ACKs from envoy.
// This value is different from len(listeners) due to non-proxy listeners
// (e.g., prometheus listener)
proxyListeners int

// networkPolicyCache publishes network policy configuration updates to
// Envoy proxies.
networkPolicyCache *xds.Cache
Expand Down Expand Up @@ -440,21 +446,24 @@ func (s *XDSServer) AddMetricsListener(port uint16, wg *completion.WaitGroup) {
if err != nil {
log.WithField(logfields.Port, port).WithError(err).Debug("Envoy: Adding metrics listener failed")
// Remove the added listener in case of a failure
s.RemoveListener(metricsListenerName, nil)
s.removeListener(metricsListenerName, nil, false)
} else {
log.WithField(logfields.Port, port).Info("Envoy: Listening for prometheus metrics")
}
})
}, false)
}

// addListener either reuses an existing listener with 'name', or creates a new one.
// 'listenerConf()' is only called if a new listener is being created.
func (s *XDSServer) addListener(name string, port uint16, listenerConf func() *envoy_config_listener.Listener, wg *completion.WaitGroup, cb func(err error)) {
func (s *XDSServer) addListener(name string, port uint16, listenerConf func() *envoy_config_listener.Listener, wg *completion.WaitGroup, cb func(err error), isProxyListener bool) {
s.mutex.Lock()
listener := s.listeners[name]
if listener == nil {
listener = &Listener{}
s.listeners[name] = listener
if isProxyListener {
s.proxyListeners++
}
}
listener.count++
listener.mutex.Lock() // needed for other than 'count'
Expand Down Expand Up @@ -585,11 +594,16 @@ func (s *XDSServer) AddListener(name string, kind policy.L7ParserType, port uint

s.addListener(name, port, func() *envoy_config_listener.Listener {
return s.getListenerConf(name, kind, port, isIngress, mayUseOriginalSourceAddr)
}, wg, nil)
}, wg, nil, true)
}

// RemoveListener removes an existing Envoy Listener.
func (s *XDSServer) RemoveListener(name string, wg *completion.WaitGroup) xds.AckingResourceMutatorRevertFunc {
return s.removeListener(name, wg, true)
}

// removeListener removes an existing Envoy Listener.
func (s *XDSServer) removeListener(name string, wg *completion.WaitGroup, isProxyListener bool) xds.AckingResourceMutatorRevertFunc {
log.Debugf("Envoy: RemoveListener %s", name)

var listenerRevertFunc xds.AckingResourceMutatorRevertFunc
Expand All @@ -599,6 +613,9 @@ func (s *XDSServer) RemoveListener(name string, wg *completion.WaitGroup) xds.Ac
if ok && listener != nil {
listener.count--
if listener.count == 0 {
if isProxyListener {
s.proxyListeners--
}
delete(s.listeners, name)
listenerRevertFunc = s.listenerMutator.Delete(ListenerTypeURL, name, []string{"127.0.0.1"}, wg, nil)
}
Expand All @@ -612,6 +629,9 @@ func (s *XDSServer) RemoveListener(name string, wg *completion.WaitGroup) xds.Ac
s.mutex.Lock()
if listenerRevertFunc != nil {
listenerRevertFunc(completion)
if isProxyListener {
s.proxyListeners++
}
}
listener.count++
s.listeners[name] = listener
Expand Down Expand Up @@ -1395,7 +1415,7 @@ func (s *XDSServer) UpdateNetworkPolicy(ep logger.EndpointUpdater, policy *polic
// query for network policies and therefore will never ACK them, and we'd
// wait forever.
if !ep.HasSidecarProxy() {
if len(s.listeners) == 0 {
if s.proxyListeners == 0 {
wg = nil
}
}
Expand Down Expand Up @@ -1453,7 +1473,7 @@ func (s *XDSServer) UseCurrentNetworkPolicy(ep logger.EndpointUpdater, policy *p
// If there are no listeners configured, the local node's Envoy proxy won't
// query for network policies and therefore will never ACK them, and we'd
// wait forever.
if !ep.HasSidecarProxy() && len(s.listeners) == 0 {
if !ep.HasSidecarProxy() && s.proxyListeners == 0 {
return
}

Expand Down
1 change: 1 addition & 0 deletions pkg/proxy/envoyproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func createEnvoyRedirect(r *Redirect, stateDir string, xdsServer *envoy.XDSServe
// Start Envoy on first invocation
envoyProxy = envoy.StartEnvoy(stateDir, option.Config.EnvoyLogPath, 0)

// Add Prometheus listener if the port is (properly) configured
if option.Config.ProxyPrometheusPort < 0 || option.Config.ProxyPrometheusPort > 65535 {
log.WithField(logfields.Port, option.Config.ProxyPrometheusPort).Error("Envoy: Invalid configured proxy-prometheus-port")
} else if option.Config.ProxyPrometheusPort != 0 {
Expand Down

0 comments on commit 099c34d

Please sign in to comment.