Skip to content

Commit

Permalink
Nack on wasm code cache miss option. (#270)
Browse files Browse the repository at this point in the history
Signed-off-by: John Plevyak <jplevyak@gmail.com>
  • Loading branch information
jplevyak committed Oct 3, 2020
1 parent 6dd9ed9 commit 5ff81dd
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 19 deletions.
7 changes: 6 additions & 1 deletion api/envoy/extensions/wasm/v3/wasm.proto
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// [#protodoc-title: Wasm service]

// Configuration for a Wasm VM.
// [#next-free-field: 6]
// [#next-free-field: 7]
message VmConfig {
// An ID which will be used along with a hash of the wasm code (or the name of the registered Null
// VM plugin) to determine which VM will be used for the plugin. All plugins which use the same
Expand All @@ -40,6 +40,11 @@ message VmConfig {
// Warning: this should only be enable for trusted sources as the precompiled code is not
// verified.
bool allow_precompiled = 5;

// If true and the code needs to be remotely fetched and it is not in the cache then NACK the configuration
// update and do a background fetch to fill the cache, otherwise fetch the code asynchronously and enter
// warming state.
bool nack_on_code_cache_miss = 6;
}

// Base Configuration for Wasm Plugins e.g. filters and services.
Expand Down
7 changes: 6 additions & 1 deletion generated_api_shadow/envoy/extensions/wasm/v3/wasm.proto

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

13 changes: 5 additions & 8 deletions source/extensions/common/wasm/wasm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ class RemoteDataFetcherAdapter : public Config::DataFetcher::RemoteDataFetcherCa
};

const std::string INLINE_STRING = "<inline>";
// NB: xDS currently does not support failing asynchronously, so we fail immediately
// if remote Wasm code is not cached and do a background fill.
const bool DEFAULT_FAIL_IF_NOT_CACHED = true;
bool fail_if_code_not_cached = DEFAULT_FAIL_IF_NOT_CACHED;
const int CODE_CACHE_SECONDS_NEGATIVE_CACHING = 10;
const int CODE_CACHE_SECONDS_CACHING_TTL = 24 * 3600; // 24 hours.
MonotonicTime::duration cache_time_offset_for_testing{};
Expand Down Expand Up @@ -246,9 +242,8 @@ void Wasm::log(absl::string_view root_id, const Http::RequestHeaderMap* request_
context->log(request_headers, response_headers, response_trailers, stream_info);
}

void clearCodeCacheForTesting(bool fail_if_not_cached) {
void clearCodeCacheForTesting() {
std::lock_guard<std::mutex> guard(code_cache_mutex);
fail_if_code_not_cached = fail_if_not_cached;
if (code_cache) {
delete code_cache;
code_cache = nullptr;
Expand Down Expand Up @@ -388,7 +383,9 @@ static bool createWasmInternal(const VmConfig& vm_config, const PluginSharedPtr&
}
wasm_extension->onRemoteCacheEntriesChanged(code_cache->size());
}
if (!fail_if_code_not_cached) {
// NB: xDS currently does not support failing asynchronously, so we fail immediately
// if remote Wasm code is not cached and do a background fill.
if (!vm_config.nack_on_code_cache_miss()) {
if (code.empty()) {
ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::wasm), trace,
"Failed to load Wasm code (fetch failed) from {}", source);
Expand All @@ -400,7 +397,7 @@ static bool createWasmInternal(const VmConfig& vm_config, const PluginSharedPtr&
dispatcher.deferredDelete(Envoy::Event::DeferredDeletablePtr{holder->release()});
}
};
if (fail_if_code_not_cached) {
if (vm_config.nack_on_code_cache_miss()) {
auto adapter = std::make_unique<RemoteDataFetcherAdapter>(fetch_callback);
auto fetcher = std::make_unique<Config::DataFetcher::RemoteDataFetcher>(
cluster_manager, vm_config.code().remote().http_uri(), vm_config.code().remote().sha256(),
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/common/wasm/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ getOrCreateThreadLocalWasm(const WasmHandleSharedPtr& base_wasm, const PluginSha
Event::Dispatcher& dispatcher,
CreateContextFn create_root_context_for_testing = nullptr);

void clearCodeCacheForTesting(bool fail_if_not_cached);
void clearCodeCacheForTesting();
std::string anyToBytes(const ProtobufWkt::Any& any);
void setTimeOffsetForCodeCacheForTesting(MonotonicTime::duration d);

Expand Down
2 changes: 1 addition & 1 deletion test/extensions/common/wasm/wasm_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class TestContext : public Extensions::Common::Wasm::Context {

class WasmCommonTest : public testing::TestWithParam<std::string> {
public:
void SetUp() { clearCodeCacheForTesting(false); }
void SetUp() { clearCodeCacheForTesting(); }
};

INSTANTIATE_TEST_SUITE_P(Runtimes, WasmCommonTest, testing::Values("v8", "null"));
Expand Down
9 changes: 4 additions & 5 deletions test/extensions/filters/http/wasm/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class WasmFilterConfigTest : public Event::TestUsingSimulatedTime,
ON_CALL(context_, dispatcher()).WillByDefault(ReturnRef(dispatcher_));
}

void SetUp() { Envoy::Extensions::Common::Wasm::clearCodeCacheForTesting(false); }
void SetUp() { Envoy::Extensions::Common::Wasm::clearCodeCacheForTesting(); }

void initializeForRemote() {
retry_timer_ = new Event::MockTimer();
Expand Down Expand Up @@ -217,14 +217,14 @@ TEST_P(WasmFilterConfigTest, YamlLoadFromRemoteWasm) {
}

TEST_P(WasmFilterConfigTest, YamlLoadFromRemoteWasmFailOnUncachedThenSucceed) {
Envoy::Extensions::Common::Wasm::clearCodeCacheForTesting(true);
const std::string code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(
"{{ test_rundir }}/test/extensions/filters/http/wasm/test_data/test_cpp.wasm"));
const std::string sha256 = Hex::encode(
Envoy::Common::Crypto::UtilitySingleton::get().getSha256Digest(Buffer::OwnedImpl(code)));
const std::string yaml = TestEnvironment::substitute(absl::StrCat(R"EOF(
config:
vm_config:
nack_on_code_cache_miss: true
runtime: "envoy.wasm.runtime.)EOF",
GetParam(), R"EOF("
code:
Expand Down Expand Up @@ -282,14 +282,14 @@ TEST_P(WasmFilterConfigTest, YamlLoadFromRemoteWasmFailOnUncachedThenSucceed) {
}

TEST_P(WasmFilterConfigTest, YamlLoadFromRemoteWasmFailCachedThenSucceed) {
Envoy::Extensions::Common::Wasm::clearCodeCacheForTesting(true);
const std::string code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(
"{{ test_rundir }}/test/extensions/filters/http/wasm/test_data/test_cpp.wasm"));
const std::string sha256 = Hex::encode(
Envoy::Common::Crypto::UtilitySingleton::get().getSha256Digest(Buffer::OwnedImpl(code)));
const std::string yaml = TestEnvironment::substitute(absl::StrCat(R"EOF(
config:
vm_config:
nack_on_code_cache_miss: true
runtime: "envoy.wasm.runtime.)EOF",
GetParam(), R"EOF("
code:
Expand Down Expand Up @@ -418,8 +418,7 @@ TEST_P(WasmFilterConfigTest, YamlLoadFromRemoteWasmFailCachedThenSucceed) {

EXPECT_CALL(context_, initManager()).WillRepeatedly(ReturnRef(init_manager5));

EXPECT_THROW_WITH_MESSAGE(factory.createFilterFactoryFromProto(proto_config2, "stats", context_),
WasmException, "Unable to create Wasm HTTP filter ");
factory.createFilterFactoryFromProto(proto_config2, "stats", context_);

EXPECT_CALL(init_watcher_, ready());
context_.initManager().initialize(init_watcher_);
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/filters/network/wasm/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class WasmNetworkFilterConfigTest : public testing::TestWithParam<std::string> {
ON_CALL(context_, dispatcher()).WillByDefault(ReturnRef(dispatcher_));
}

void SetUp() override { Envoy::Extensions::Common::Wasm::clearCodeCacheForTesting(false); }
void SetUp() override { Envoy::Extensions::Common::Wasm::clearCodeCacheForTesting(); }

void initializeForRemote() {
retry_timer_ = new Event::MockTimer();
Expand Down
2 changes: 1 addition & 1 deletion test/test_common/wasm_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace Wasm {

template <typename Base = testing::Test> class WasmTestBase : public Base {
public:
void SetUp() override { clearCodeCacheForTesting(false); }
void SetUp() override { clearCodeCacheForTesting(); }

void setupBase(const std::string& runtime, const std::string& code, CreateContextFn create_root,
std::string root_id = "", std::string vm_configuration = "") {
Expand Down

0 comments on commit 5ff81dd

Please sign in to comment.