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

xds: allow updating listener back to original state #12645

Merged
merged 5 commits into from
Aug 18, 2020
Merged

xds: allow updating listener back to original state #12645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25fcc8b
allow updating listener back to original state
Aug 14, 2020
bffc8cc
fix comment style in xds_verifier and xds_verifier_test
Aug 17, 2020
5e39e27
add unit test
Aug 17, 2020
0822291
fix style/spelling
Aug 17, 2020
a7e2520
Merge branch 'master' of github.com:envoyproxy/envoy into xds_update
Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions source/server/listener_manager_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,17 @@ bool ListenerManagerImpl::addOrUpdateListenerInternal(
auto existing_active_listener = getListenerByName(active_listeners_, name);
auto existing_warming_listener = getListenerByName(warming_listeners_, name);

// The listener should be updated back to its original state and the warming listener should be
// removed.
if (existing_warming_listener != warming_listeners_.end() &&
existing_active_listener != active_listeners_.end() &&
(*existing_active_listener)->blockUpdate(hash)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have to look into blockUpdate before I asked how to verify the active listener and the latest config is equivalent. Sigh...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blockUpdate() just checks that the hash of the two listeners is the same

envoy/source/server/listener_impl.h

Line 256 in 51551ae

bool blockUpdate(uint64_t new_hash) { return new_hash == hash_ || !added_via_api_; }

envoy/source/common/protobuf/utility.cc

Lines 264 to 277 in 51551ae

size_t MessageUtil::hash(const Protobuf::Message& message) {
std::string text_format;
{
Protobuf::TextFormat::Printer printer;
printer.SetExpandAny(true);
printer.SetUseFieldNumber(true);
printer.SetSingleLineMode(true);
printer.SetHideUnknownFields(true);
printer.PrintToString(message, &text_format);
}
return HashUtil::xxHash64(text_format);
}

It's used just below this snippet to ignore duplicate listeners:

envoy/source/server/listener_manager_impl.cc

Lines 409 to 415 in 25fcc8b

if ((existing_warming_listener != warming_listeners_.end() &&
(*existing_warming_listener)->blockUpdate(hash)) ||
(existing_active_listener != active_listeners_.end() &&
(*existing_active_listener)->blockUpdate(hash))) {
ENVOY_LOG(debug, "duplicate/locked listener '{}'. no add/update", name);
return false;
}

warming_listeners_.erase(existing_warming_listener);
updateWarmingActiveGauges();
stats_.listener_modified_.inc();
return true;
}

// Do a quick blocked update check before going further. This check needs to be done against both
// warming and active.
if ((existing_warming_listener != warming_listeners_.end() &&
Expand Down
118 changes: 118 additions & 0 deletions test/server/config_validation/xds_corpus/clusterfuzz-testcase-xds_fuzz_test-6589246463541248
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading