Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
references:
envoy-build-image: &envoy-build-image
envoyproxy/envoy-build:1ef23d481a4701ad4a414d1ef98036bd2ed322e7
envoyproxy/envoy-build:e994c1c0b1cdc9a9470cff728311ff7c995685e6

version: 2
jobs:
Expand Down
4 changes: 2 additions & 2 deletions bazel/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ On Ubuntu, run the following commands:
apt-get install libtool
apt-get install cmake
apt-get install realpath
apt-get install clang-format-5.0
apt-get install clang-format-6.0
apt-get install automake
apt-get install ninja-build
apt-get install curl
Expand Down Expand Up @@ -231,7 +231,7 @@ bazel test -c dbg --config=asan //test/...

The ASAN failure stack traces include line numbers as a result of running ASAN with a `dbg` build above.

If you have clang-5.0, additional checks are provided with:
If you have clang-5.0 or newer, additional checks are provided with:

```
bazel test -c dbg --config=clang-asan //test/...
Expand Down
12 changes: 6 additions & 6 deletions ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Currently there are three build images:
* `envoyproxy/envoy-build-ubuntu` — based on Ubuntu 16.04 (Xenial) which uses the GCC 5.4 compiler.
* `envoyproxy/envoy-build-centos` — based on CentOS 7 which uses the GCC 5.3.1 compiler (devtoolset-4).

We also install and use the clang-5.0 compiler for some sanitizing runs.
We also install and use the clang-6.0 compiler for some sanitizing runs.

# Building and running tests as a developer

Expand Down Expand Up @@ -77,8 +77,8 @@ The build artifact can be found in `/tmp/envoy-docker-build/envoy/source/exe/env

The `./ci/run_envoy_docker.sh './ci/do_ci.sh <TARGET>'` targets are:

* `bazel.api` &mdash; build and run API tests under `-c fastbuild` with clang-5.0.
* `bazel.asan` &mdash; build and run tests under `-c dbg --config=clang-asan` with clang-5.0.
* `bazel.api` &mdash; build and run API tests under `-c fastbuild` with clang-6.0.
* `bazel.asan` &mdash; build and run tests under `-c dbg --config=clang-asan` with clang-6.0.
* `bazel.debug` &mdash; build Envoy static binary and run tests under `-c dbg`.
* `bazel.debug.server_only` &mdash; build Envoy static binary under `-c dbg`.
* `bazel.dev` &mdash; build Envoy static binary and run tests under `-c fastbuild` with gcc.
Expand All @@ -87,9 +87,9 @@ The `./ci/run_envoy_docker.sh './ci/do_ci.sh <TARGET>'` targets are:
* `bazel.release.server_only` &mdash; build Envoy static binary under `-c opt` with gcc.
* `bazel.coverage` &mdash; build and run tests under `-c dbg` with gcc, generating coverage information in `$ENVOY_DOCKER_BUILD_DIR/envoy/generated/coverage/coverage.html`.
* `bazel.coverity` &mdash; build Envoy static binary and run Coverity Scan static analysis.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang-5.0.
* `check_format`&mdash; run `clang-format` 5.0 and `buildifier` on entire source tree.
* `fix_format`&mdash; run and enforce `clang-format` 5.0 and `buildifier` on entire source tree.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang-6.0.
* `check_format`&mdash; run `clang-format-6.0` and `buildifier` on entire source tree.
* `fix_format`&mdash; run and enforce `clang-format-6.0` and `buildifier` on entire source tree.
* `docs`&mdash; build documentation tree in `generated/docs`.

# Testing changes to the build image as a developer
Expand Down
6 changes: 3 additions & 3 deletions ci/build_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ function setup_gcc_toolchain() {
}

function setup_clang_toolchain() {
Copy link
Member

Choose a reason for hiding this comment

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

Per @lizan might as well just start linking w/ lld for the cases where we use clang today? Or prefer to defer to a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Let's defer that to a separate PR, IIRC we hardcoded gold in some bzl files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely a separate PR. I believe that Bazel hardcodes -fuse-ld=gold somewhere as well, but maybe that's easy to override.

Copy link
Member

Choose a reason for hiding this comment

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

It's pretty trivial using bazelrc but agree let's do in a follow up.

Copy link
Member

Choose a reason for hiding this comment

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

@mattklein123 using bazelrc is a quick set up to speed up local building, I would like to also fix https://github.com/envoyproxy/envoy/blob/master/bazel/genrule_repository.bzl#L108 to land lld in master.

Copy link
Member

Choose a reason for hiding this comment

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

I see, yes good call.

export CC=clang-5.0
export CXX=clang++-5.0
export ASAN_SYMBOLIZER_PATH=/usr/lib/llvm-5.0/bin/llvm-symbolizer
export CC=clang-6.0
export CXX=clang++-6.0
export ASAN_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer
echo "$CC/$CXX toolchain configured"
}

Expand Down
2 changes: 1 addition & 1 deletion include/envoy/common/pure.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ namespace Envoy {
* Friendly name for a pure virtual routine.
*/
#define PURE = 0
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion include/envoy/stats/stats_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ namespace Envoy {
#define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "")
#define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "")
#define POOL_HISTOGRAM(POOL) POOL_HISTOGRAM_PREFIX(POOL, "")
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion include/envoy/upstream/host_description.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,5 @@ class HostDescription {

typedef std::shared_ptr<const HostDescription> HostDescriptionConstSharedPtr;

} // Upstream
} // namespace Upstream
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,4 @@ namespace Envoy {
// after a switch (some_enum) with all enum values included in the cases. The macro name includes
// "GCOVR_EXCL_LINE" to exclude the macro's usage from code coverage reports.
#define NOT_REACHED_GCOVR_EXCL_LINE PANIC("not reached")
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/compiler_requirements.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ namespace Envoy {
"ENVOY_IGNORE_GLIBCXX_USE_CXX11_ABI_ERROR=1 in your build."
#endif

} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,5 @@ void Registry::setLogFormat(const std::string& log_format) {
}
}

} // Logger
} // namespace Logger
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ template <Id id> class Loggable {
}
};

} // Logger
} // namespace Logger

// Convert the line macro to a string literal for concatenation in log macros.
#define DO_STRINGIZE(x) STRINGIZE(x)
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,4 @@ namespace Envoy {
#define FALLTHRU
#endif

} // Envoy
} // namespace Envoy
5 changes: 3 additions & 2 deletions source/common/config/filter_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,9 @@ void FilterJson::translateHttpConnectionManager(
json_filter->getString("name")));
JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type);

const std::string deprecated_config = "{\"deprecated_v1\": true, \"value\": " +
json_filter->getObject("config")->asJsonString() + "}";
const std::string deprecated_config =
"{\"deprecated_v1\": true, \"value\": " + json_filter->getObject("config")->asJsonString() +
"}";

const auto status =
Protobuf::util::JsonStringToMessage(deprecated_config, filter->mutable_config());
Expand Down
5 changes: 3 additions & 2 deletions source/common/config/lds_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ void LdsJson::translateListener(const Json::Object& json_listener,
json_filter->getString("name")));
JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type);

const std::string json_config = "{\"deprecated_v1\": true, \"value\": " +
json_filter->getObject("config")->asJsonString() + "}";
const std::string json_config =
"{\"deprecated_v1\": true, \"value\": " + json_filter->getObject("config")->asJsonString() +
"}";

const auto status = Protobuf::util::JsonStringToMessage(json_config, filter->mutable_config());
// JSON schema has already validated that this is a valid JSON object.
Expand Down
10 changes: 5 additions & 5 deletions source/common/filesystem/filesystem_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,10 @@ class FileImpl : public File {
Thread::CondVar flush_event_;
std::atomic<bool> flush_thread_exit_{};
std::atomic<bool> reopen_file_{};
Buffer::OwnedImpl flush_buffer_
GUARDED_BY(write_lock_); // This buffer is used by multiple threads. It gets filled and
// then flushed either when max size is reached or when a timer
// fires.
Buffer::OwnedImpl
flush_buffer_ GUARDED_BY(write_lock_); // This buffer is used by multiple threads. It gets
// filled and then flushed either when max size is
// reached or when a timer fires.
// TODO(jmarantz): this should be GUARDED_BY(flush_lock_) but the analysis cannot poke through
// the std::make_unique assignment. I do not believe it's possible to annotate this properly now
// due to limitations in the clang thread annotation analysis.
Expand All @@ -153,4 +153,4 @@ class FileImpl : public File {
};

} // namespace Filesystem
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/http/conn_manager_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,5 +293,5 @@ class ConnectionManagerConfig {
*/
virtual const Http::Http1Settings& http1Settings() const PURE;
};
}
} // namespace Http
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/http/header_map_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,5 +209,5 @@ class HeaderMapImpl : public HeaderMap, NonCopyable {

typedef std::unique_ptr<HeaderMapImpl> HeaderMapImplPtr;

} // Http
} // namespace Http
} // namespace Envoy
1 change: 0 additions & 1 deletion source/common/http/http2/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,6 @@ ConnectionImpl::Http2Callbacks::Http2Callbacks() {
callbacks_,
[](nghttp2_session*, const nghttp2_frame* frame, const uint8_t* raw_name, size_t name_length,
const uint8_t* raw_value, size_t value_length, uint8_t, void* user_data) -> int {

// TODO PERF: Can reference count here to avoid copies.
HeaderString name;
name.setCopy(reinterpret_cast<const char*>(raw_name), name_length);
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/user_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,4 @@ class UserAgent {
};

} // namespace Http
} // Envoy
} // namespace Envoy
16 changes: 8 additions & 8 deletions source/common/router/router.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,14 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
direct_response->rewritePathHeader(headers, !config_.suppress_envoy_headers_);
callbacks_->sendLocalReply(
direct_response->responseCode(), direct_response->responseBody(),
[ this, direct_response, &request_headers = headers ](Http::HeaderMap & response_headers)
->void {
const auto new_path = direct_response->newPath(request_headers);
if (!new_path.empty()) {
response_headers.addReferenceKey(Http::Headers::get().Location, new_path);
}
direct_response->finalizeResponseHeaders(response_headers, callbacks_->requestInfo());
});
[this, direct_response,
&request_headers = headers](Http::HeaderMap& response_headers) -> void {
const auto new_path = direct_response->newPath(request_headers);
if (!new_path.empty()) {
response_headers.addReferenceKey(Http::Headers::get().Location, new_path);
}
direct_response->finalizeResponseHeaders(response_headers, callbacks_->requestInfo());
});
return Http::FilterHeadersStatus::StopIteration;
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/runtime/runtime_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ std::unique_ptr<SnapshotImpl> LoaderImpl::createNewSnapshot() {

void LoaderImpl::loadNewSnapshot() {
ThreadLocal::ThreadLocalObjectSharedPtr ptr = createNewSnapshot();
tls_->set([ptr = std::move(ptr)](Event::Dispatcher&)->ThreadLocal::ThreadLocalObjectSharedPtr {
tls_->set([ptr = std::move(ptr)](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
return ptr;
});
}
Expand Down
2 changes: 1 addition & 1 deletion source/common/stats/thread_local_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void ThreadLocalStoreImpl::releaseScopeCrossThread(ScopeImpl* scope) {
// cache flush operation.
if (!shutting_down_ && main_thread_dispatcher_) {
main_thread_dispatcher_->post(
[ this, scope_id = scope->scope_id_ ]()->void { clearScopeFromCaches(scope_id); });
[this, scope_id = scope->scope_id_]() -> void { clearScopeFromCaches(scope_id); });
}
}

Expand Down
15 changes: 6 additions & 9 deletions source/common/tcp_proxy/tcp_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -466,17 +466,14 @@ void Filter::onUpstreamEvent(Network::ConnectionEvent event) {
// The idle_timer_ can be moved to a Drainer, so related callbacks call into
// the UpstreamCallbacks, which has the same lifetime as the timer, and can dispatch
// the call to either TcpProxy or to Drainer, depending on the current state.
idle_timer_ =
read_callbacks_->connection().dispatcher().createTimer([upstream_callbacks =
upstream_callbacks_]() {
upstream_callbacks->onIdleTimeout();
});
idle_timer_ = read_callbacks_->connection().dispatcher().createTimer(
[upstream_callbacks = upstream_callbacks_]() { upstream_callbacks->onIdleTimeout(); });
resetIdleTimer();
read_callbacks_->connection().addBytesSentCallback([this](uint64_t) { resetIdleTimer(); });
upstream_conn_data_->connection().addBytesSentCallback([upstream_callbacks =
upstream_callbacks_](uint64_t) {
upstream_callbacks->onBytesSent();
});
upstream_conn_data_->connection().addBytesSentCallback(
[upstream_callbacks = upstream_callbacks_](uint64_t) {
upstream_callbacks->onBytesSent();
});
}
}
}
Expand Down
57 changes: 26 additions & 31 deletions source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -521,28 +521,24 @@ bool ClusterManagerImpl::addOrUpdateCluster(const envoy::api::v2::Cluster& clust
}

void ClusterManagerImpl::createOrUpdateThreadLocalCluster(ClusterData& cluster) {
tls_->runOnAllThreads(
[
this, new_cluster = cluster.cluster_->info(),
thread_aware_lb_factory = cluster.loadBalancerFactory()
]()
->void {
ThreadLocalClusterManagerImpl& cluster_manager =
tls_->getTyped<ThreadLocalClusterManagerImpl>();

if (cluster_manager.thread_local_clusters_.count(new_cluster->name()) > 0) {
ENVOY_LOG(debug, "updating TLS cluster {}", new_cluster->name());
} else {
ENVOY_LOG(debug, "adding TLS cluster {}", new_cluster->name());
}

auto thread_local_cluster = new ThreadLocalClusterManagerImpl::ClusterEntry(
cluster_manager, new_cluster, thread_aware_lb_factory);
cluster_manager.thread_local_clusters_[new_cluster->name()].reset(thread_local_cluster);
for (auto& cb : cluster_manager.update_callbacks_) {
cb->onClusterAddOrUpdate(*thread_local_cluster);
}
});
tls_->runOnAllThreads([this, new_cluster = cluster.cluster_->info(),
thread_aware_lb_factory = cluster.loadBalancerFactory()]() -> void {
ThreadLocalClusterManagerImpl& cluster_manager =
tls_->getTyped<ThreadLocalClusterManagerImpl>();

if (cluster_manager.thread_local_clusters_.count(new_cluster->name()) > 0) {
ENVOY_LOG(debug, "updating TLS cluster {}", new_cluster->name());
} else {
ENVOY_LOG(debug, "adding TLS cluster {}", new_cluster->name());
}

auto thread_local_cluster = new ThreadLocalClusterManagerImpl::ClusterEntry(
cluster_manager, new_cluster, thread_aware_lb_factory);
cluster_manager.thread_local_clusters_[new_cluster->name()].reset(thread_local_cluster);
for (auto& cb : cluster_manager.update_callbacks_) {
cb->onClusterAddOrUpdate(*thread_local_cluster);
}
});
}

bool ClusterManagerImpl::removeCluster(const std::string& cluster_name) {
Expand Down Expand Up @@ -693,15 +689,14 @@ void ClusterManagerImpl::postThreadLocalClusterUpdate(const Cluster& cluster, ui
HostsPerLocalityConstSharedPtr healthy_hosts_per_locality_copy =
host_set->healthyHostsPerLocality().clone();

tls_->runOnAllThreads([
this, name = cluster.info()->name(), priority, hosts_copy, healthy_hosts_copy,
hosts_per_locality_copy, healthy_hosts_per_locality_copy,
locality_weights = host_set->localityWeights(), hosts_added, hosts_removed
]() {
ThreadLocalClusterManagerImpl::updateClusterMembership(
name, priority, hosts_copy, healthy_hosts_copy, hosts_per_locality_copy,
healthy_hosts_per_locality_copy, locality_weights, hosts_added, hosts_removed, *tls_);
});
tls_->runOnAllThreads(
[this, name = cluster.info()->name(), priority, hosts_copy, healthy_hosts_copy,
hosts_per_locality_copy, healthy_hosts_per_locality_copy,
locality_weights = host_set->localityWeights(), hosts_added, hosts_removed]() {
ThreadLocalClusterManagerImpl::updateClusterMembership(
name, priority, hosts_copy, healthy_hosts_copy, hosts_per_locality_copy,
healthy_hosts_per_locality_copy, locality_weights, hosts_added, hosts_removed, *tls_);
});
}

void ClusterManagerImpl::postThreadLocalHealthFailure(const HostSharedPtr& host) {
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/access_loggers/http_grpc/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ HttpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& confi
std::shared_ptr<GrpcAccessLogStreamer> grpc_access_log_streamer =
context.singletonManager().getTyped<GrpcAccessLogStreamer>(
SINGLETON_MANAGER_REGISTERED_NAME(grpc_access_log_streamer),
[&context, grpc_service = proto_config.common_config().grpc_service() ] {
[&context, grpc_service = proto_config.common_config().grpc_service()] {
return std::make_shared<GrpcAccessLogStreamerImpl>(
context.clusterManager().grpcAsyncClientManager().factoryForGrpcService(
grpc_service, context.scope(), false),
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/common/lua/lua.h
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ class LuaException : public EnvoyException {
public:
using EnvoyException::EnvoyException;
};
}
} // namespace Lua
} // namespace Common
} // namespace Filters
} // namespace Extensions
Expand Down
12 changes: 6 additions & 6 deletions source/extensions/filters/http/ext_authz/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped(
if (proto_config.has_http_service()) {
const uint32_t timeout_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config.http_service().server_uri(),
timeout, DefaultTimeout);
return [
filter_config, timeout_ms, cluster_name = proto_config.http_service().server_uri().cluster(),
path_prefix = proto_config.http_service().path_prefix()
](Http::FilterChainFactoryCallbacks & callbacks) {
return [filter_config, timeout_ms,
cluster_name = proto_config.http_service().server_uri().cluster(),
path_prefix = proto_config.http_service().path_prefix()](
Http::FilterChainFactoryCallbacks& callbacks) {
auto client = std::make_unique<Filters::Common::ExtAuthz::RawHttpClientImpl>(
cluster_name, filter_config->cm(), std::chrono::milliseconds(timeout_ms), path_prefix,
filter_config->allowedAuthorizationHeaders(), filter_config->allowedRequestHeaders());
Expand All @@ -43,8 +43,8 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped(
const uint32_t timeout_ms =
PROTOBUF_GET_MS_OR_DEFAULT(proto_config.grpc_service(), timeout, DefaultTimeout);

return [ grpc_service = proto_config.grpc_service(), &context, filter_config,
timeout_ms ](Http::FilterChainFactoryCallbacks & callbacks) {
return [grpc_service = proto_config.grpc_service(), &context, filter_config,
timeout_ms](Http::FilterChainFactoryCallbacks& callbacks) {
const auto async_client_factory =
context.clusterManager().grpcAsyncClientManager().factoryForGrpcService(
grpc_service, context.scope(), true);
Expand Down
Loading