envoyproxy/envoy

Align logic for reverting a warming cluster and reverting a warming listener

Open

#16,350 opened on May 5, 2021

View on GitHub
 (6 comments) (0 reactions) (0 assignees)C++ (5,373 forks)batch import
area/listenerhelp wantedtech debt

Repository metrics

Stars
 (27,997 stars)
PR merge metrics
 (Avg merge 8d) (378 merged PRs in 30d)

Description

Title: Align logic for reverting a warming cluster and reverting a warming listener

Description: #15269 fixed blocking a revert of a warming cluster by always accepting a new cluster when it's different from the existing warming cluster. This turns out to be a different approach from #12645 which instead removes the warming listener.

It seems they could both use the same logic for this situation and so probably should. My reasoning for #15269 was that it avoids needing a concept of "reverting" config and it settles at the latest version_info given. While #15269 seems simpler, are there advantages to #12645 other than being a performance optimization?

Implementation note: When I tried to adapt the listener manager to use the same logic as #15269 and adjust the unit test UpdateActiveToWarmAndBack, expectListenerOverridden can't handle this case because buildFilterChain and thus createNetworkFilterFactoryList aren't called (the existing filter chain is used instead)...

Relevant Links: See https://github.com/envoyproxy/envoy/pull/15269/files#r592046281 for discussion

cc @htuch @adisuissa @dmitri-d

Contributor guide

Research direction
Investigate the logic in PR #15269 and #12645 for reverting warming clusters and listeners. Understand the different approaches and their trade offs. Check the unit test 'UpdateActiveToWarmAndBack' and see why 'expectListenerOverridden' fails. Determine if the logic can be unified without needing a concept of 'reverting' config.
Tech stack
cpp
Domain
infrastructure
Issue type
Bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
C++Envoy
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
30