From f928dc6bbb0f583d04d03df83030c1e4c2dbbf3b Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Tue, 6 Oct 2015 17:40:02 -0700 Subject: [PATCH 01/33] Fix incorrect assert --- src/core/channel/compress_filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/channel/compress_filter.c b/src/core/channel/compress_filter.c index 182fbf18bfc..c32f150715c 100644 --- a/src/core/channel/compress_filter.c +++ b/src/core/channel/compress_filter.c @@ -242,7 +242,7 @@ static void process_send_ops(grpc_call_element *elem, GPR_ASSERT(calld->remaining_slice_bytes > 0); /* Increase input ref count, gpr_slice_buffer_add takes ownership. */ gpr_slice_buffer_add(&calld->slices, gpr_slice_ref(sop->data.slice)); - GPR_ASSERT(GPR_SLICE_LENGTH(sop->data.slice) >= + GPR_ASSERT(GPR_SLICE_LENGTH(sop->data.slice) <= calld->remaining_slice_bytes); calld->remaining_slice_bytes -= (gpr_uint32)GPR_SLICE_LENGTH(sop->data.slice); From 64824bebea2dfdeb273776c0ba14c370941af1bb Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 6 Oct 2015 19:45:36 -0700 Subject: [PATCH 02/33] Fixed unprotected access to call field --- src/core/surface/call.c | 16 ++++++++++++---- src/core/surface/call.h | 5 ++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index d15a3bcbade..c8b880b0ad0 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -522,8 +522,12 @@ static void set_compression_algorithm(grpc_call *call, } grpc_compression_algorithm grpc_call_get_compression_algorithm( - const grpc_call *call) { - return call->compression_algorithm; + grpc_call *call) { + grpc_compression_algorithm algorithm; + gpr_mu_lock(&call->mu); + algorithm = call->compression_algorithm; + gpr_mu_unlock(&call->mu); + return algorithm; } static void set_encodings_accepted_by_peer( @@ -561,8 +565,12 @@ gpr_uint32 grpc_call_get_encodings_accepted_by_peer(grpc_call *call) { return call->encodings_accepted_by_peer; } -gpr_uint32 grpc_call_get_message_flags(const grpc_call *call) { - return call->incoming_message_flags; +gpr_uint32 grpc_call_get_message_flags(grpc_call *call) { + gpr_uint32 flags; + gpr_mu_lock(&call->mu); + flags = call->incoming_message_flags; + gpr_mu_unlock(&call->mu); + return flags; } static void set_status_details(grpc_call *call, status_source source, diff --git a/src/core/surface/call.h b/src/core/surface/call.h index f421a81619f..cd1db3d417c 100644 --- a/src/core/surface/call.h +++ b/src/core/surface/call.h @@ -169,10 +169,9 @@ void *grpc_call_context_get(grpc_call *call, grpc_context_index elem); gpr_uint8 grpc_call_is_client(grpc_call *call); -grpc_compression_algorithm grpc_call_get_compression_algorithm( - const grpc_call *call); +grpc_compression_algorithm grpc_call_get_compression_algorithm(grpc_call *call); -gpr_uint32 grpc_call_get_message_flags(const grpc_call *call); +gpr_uint32 grpc_call_get_message_flags(grpc_call *call); /** Returns a bitset for the encodings (compression algorithms) supported by \a * call's peer. From b17b7f48710c16f72f617d79ffb2bceed0b9f8bc Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 7 Oct 2015 08:48:51 -0700 Subject: [PATCH 03/33] Add a timeout to benchmark test runs --- test/cpp/qps/driver.cc | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index ac763e4b3c6..dd5c4f4f73f 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -82,9 +82,12 @@ static deque get_hosts(const string& name) { namespace runsc { // ClientContext allocator -static ClientContext* AllocContext(list* contexts) { +template +static ClientContext* AllocContext(list* contexts, T deadline) { contexts->emplace_back(); - return &contexts->back(); + auto context = &contexts->back(); + context->set_deadline(deadline); + return context; } struct ServerData { @@ -147,6 +150,11 @@ std::unique_ptr RunScenario( // Trim to just what we need workers.resize(num_clients + num_servers); + gpr_timespec deadline = + gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), + gpr_time_from_seconds( + warmup_seconds + benchmark_seconds + 20, GPR_TIMESPAN)); + // Start servers using runsc::ServerData; // servers is array rather than std::vector to avoid gcc-4.4 issues @@ -160,7 +168,7 @@ std::unique_ptr RunScenario( result_server_config.set_host(workers[i]); *args.mutable_setup() = server_config; servers[i].stream = - servers[i].stub->RunServer(runsc::AllocContext(&contexts)); + servers[i].stub->RunServer(runsc::AllocContext(&contexts, deadline)); GPR_ASSERT(servers[i].stream->Write(args)); ServerStatus init_status; GPR_ASSERT(servers[i].stream->Read(&init_status)); @@ -188,7 +196,7 @@ std::unique_ptr RunScenario( result_client_config.set_host(workers[i + num_servers]); *args.mutable_setup() = client_config; clients[i].stream = - clients[i].stub->RunTest(runsc::AllocContext(&contexts)); + clients[i].stub->RunTest(runsc::AllocContext(&contexts, deadline)); GPR_ASSERT(clients[i].stream->Write(args)); ClientStatus init_status; GPR_ASSERT(clients[i].stream->Read(&init_status)); From b063c87596f4e05e4aaac9f69541e01278920451 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 7 Oct 2015 11:40:13 -0700 Subject: [PATCH 04/33] mark unreachable code with a macro --- include/grpc/support/port_platform.h | 12 ++++++++++++ src/core/channel/client_channel.c | 4 +--- src/core/client_config/uri_parser.c | 4 ++-- src/core/httpcli/parser.c | 11 +++-------- src/core/iomgr/tcp_client_posix.c | 2 +- src/core/iomgr/tcp_server_posix.c | 2 +- src/core/surface/byte_buffer.c | 7 ++----- src/core/surface/call.c | 3 +-- src/core/surface/channel_connectivity.c | 8 ++------ src/core/surface/completion_queue.c | 3 +-- src/core/transport/chttp2/frame_data.c | 4 +--- src/core/transport/chttp2/frame_goaway.c | 4 +--- src/core/transport/chttp2/hpack_parser.c | 8 ++------ src/core/transport/chttp2/parsing.c | 16 ++++------------ src/core/transport/chttp2/stream_encoder.c | 6 +++--- src/cpp/server/server.cc | 3 +-- 16 files changed, 38 insertions(+), 59 deletions(-) diff --git a/include/grpc/support/port_platform.h b/include/grpc/support/port_platform.h index 434dd6de874..f98038da520 100644 --- a/include/grpc/support/port_platform.h +++ b/include/grpc/support/port_platform.h @@ -181,6 +181,7 @@ #ifndef _BSD_SOURCE #define _BSD_SOURCE #endif +#define GPR_FORBID_UNREACHABLE_CODE #define GPR_MSG_IOVLEN_TYPE int #if TARGET_OS_IPHONE #define GPR_PLATFORM_STRING "ios" @@ -336,4 +337,15 @@ typedef uintptr_t gpr_uintptr; #endif #endif +#ifdef GPR_FORBID_UNREACHABLE_CODE +#define GPR_UNREACHABLE_CODE(STATEMENT) +#else +#define GPR_UNREACHABLE_CODE(STATEMENT) \ + do { \ + gpr_log(GPR_ERROR, "Should never reach here."); \ + abort(); \ + STATEMENT; \ + } while (0) +#endif /* GPR_FORBID_UNREACHABLE_CODE */ + #endif /* GRPC_SUPPORT_PORT_PLATFORM_H */ diff --git a/src/core/channel/client_channel.c b/src/core/channel/client_channel.c index b59b62a6aa7..4b5cd04dc11 100644 --- a/src/core/channel/client_channel.c +++ b/src/core/channel/client_channel.c @@ -645,9 +645,7 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, case CALL_WAITING_FOR_CONFIG: case CALL_WAITING_FOR_CALL: case CALL_WAITING_FOR_SEND: - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - break; + GPR_UNREACHABLE_CODE(return ); } } diff --git a/src/core/client_config/uri_parser.c b/src/core/client_config/uri_parser.c index df9f32d403c..cbdfffcf8e8 100644 --- a/src/core/client_config/uri_parser.c +++ b/src/core/client_config/uri_parser.c @@ -37,6 +37,7 @@ #include #include +#include #include /** a size_t default value... maps to all 1's */ @@ -120,8 +121,7 @@ static int parse_fragment_or_query(const char *uri_text, size_t *i) { } else { return 1; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return 0); default: (*i) += advance; break; diff --git a/src/core/httpcli/parser.c b/src/core/httpcli/parser.c index 404906d5ae2..046770c0944 100644 --- a/src/core/httpcli/parser.c +++ b/src/core/httpcli/parser.c @@ -139,8 +139,7 @@ static int finish_line(grpc_httpcli_parser *parser) { } break; case GRPC_HTTPCLI_BODY: - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return 0); } parser->cur_line_length = 0; @@ -165,8 +164,7 @@ static int addbyte(grpc_httpcli_parser *parser, gpr_uint8 byte) { } else { return 1; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return 0); case GRPC_HTTPCLI_BODY: if (parser->r.body_length == parser->body_capacity) { parser->body_capacity = GPR_MAX(8, parser->body_capacity * 3 / 2); @@ -177,10 +175,7 @@ static int addbyte(grpc_httpcli_parser *parser, gpr_uint8 byte) { parser->r.body_length++; return 1; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - - return 0; + GPR_UNREACHABLE_CODE(return 0); } void grpc_httpcli_parser_init(grpc_httpcli_parser *parser) { diff --git a/src/core/iomgr/tcp_client_posix.c b/src/core/iomgr/tcp_client_posix.c index aca2691c416..fe200392641 100644 --- a/src/core/iomgr/tcp_client_posix.c +++ b/src/core/iomgr/tcp_client_posix.c @@ -191,7 +191,7 @@ static void on_writable(grpc_exec_ctx *exec_ctx, void *acp, int success) { goto finish; } - abort(); + GPR_UNREACHABLE_CODE(return ); finish: if (fd != NULL) { diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index 13bd67576f5..99c76dcbe9a 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -352,7 +352,7 @@ static void on_read(grpc_exec_ctx *exec_ctx, void *arg, int success) { gpr_free(addr_str); } - abort(); + GPR_UNREACHABLE_CODE(return ); error: gpr_mu_lock(&sp->server->mu); diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index a930949f2d4..fb39c4531d9 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -75,9 +75,7 @@ grpc_byte_buffer *grpc_byte_buffer_copy(grpc_byte_buffer *bb) { return grpc_raw_byte_buffer_create(bb->data.raw.slice_buffer.slices, bb->data.raw.slice_buffer.count); } - gpr_log(GPR_INFO, "should never get here"); - abort(); - return NULL; + GPR_UNREACHABLE_CODE(return NULL); } void grpc_byte_buffer_destroy(grpc_byte_buffer *bb) { @@ -95,6 +93,5 @@ size_t grpc_byte_buffer_length(grpc_byte_buffer *bb) { case GRPC_BB_RAW: return bb->data.raw.slice_buffer.length; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return 0); } diff --git a/src/core/surface/call.c b/src/core/surface/call.c index d15a3bcbade..bc1b95a305d 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -429,8 +429,7 @@ static grpc_cq_completion *allocate_completion(grpc_call *call) { gpr_mu_unlock(&call->completion_mu); return &call->completions[i]; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return NULL); } static void done_completion(grpc_exec_ctx *exec_ctx, void *call, diff --git a/src/core/surface/channel_connectivity.c b/src/core/surface/channel_connectivity.c index f72fb041426..ca3c02c536a 100644 --- a/src/core/surface/channel_connectivity.c +++ b/src/core/surface/channel_connectivity.c @@ -100,9 +100,7 @@ static void finished_completion(grpc_exec_ctx *exec_ctx, void *pw, switch (w->phase) { case WAITING: case CALLED_BACK: - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - break; + GPR_UNREACHABLE_CODE(return ); case CALLING_BACK: w->phase = CALLED_BACK; break; @@ -149,9 +147,7 @@ static void partly_done(grpc_exec_ctx *exec_ctx, state_watcher *w, w->phase = CALLING_BACK_AND_FINISHED; break; case CALLING_BACK_AND_FINISHED: - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - break; + GPR_UNREACHABLE_CODE(return ); case CALLED_BACK: delete = 1; break; diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c index e818ccba488..9e552c2cdf8 100644 --- a/src/core/surface/completion_queue.c +++ b/src/core/surface/completion_queue.c @@ -254,8 +254,7 @@ static void del_plucker(grpc_completion_queue *cc, void *tag, return; } } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return ); } grpc_event grpc_completion_queue_pluck(grpc_completion_queue *cc, void *tag, diff --git a/src/core/transport/chttp2/frame_data.c b/src/core/transport/chttp2/frame_data.c index acfa7c002eb..07179a4571e 100644 --- a/src/core/transport/chttp2/frame_data.c +++ b/src/core/transport/chttp2/frame_data.c @@ -168,7 +168,5 @@ grpc_chttp2_parse_error grpc_chttp2_data_parser_parse( } } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - return GRPC_CHTTP2_CONNECTION_ERROR; + GPR_UNREACHABLE_CODE(return GRPC_CHTTP2_CONNECTION_ERROR); } diff --git a/src/core/transport/chttp2/frame_goaway.c b/src/core/transport/chttp2/frame_goaway.c index 2ff1eda89b6..c5758bcb714 100644 --- a/src/core/transport/chttp2/frame_goaway.c +++ b/src/core/transport/chttp2/frame_goaway.c @@ -152,9 +152,7 @@ grpc_chttp2_parse_error grpc_chttp2_goaway_parser_parse( } return GRPC_CHTTP2_PARSE_OK; } - gpr_log(GPR_ERROR, "Should never end up here"); - abort(); - return GRPC_CHTTP2_CONNECTION_ERROR; + GPR_UNREACHABLE_CODE(return GRPC_CHTTP2_CONNECTION_ERROR); } void grpc_chttp2_goaway_append(gpr_uint32 last_stream_id, gpr_uint32 error_code, diff --git a/src/core/transport/chttp2/hpack_parser.c b/src/core/transport/chttp2/hpack_parser.c index 20ea5133751..20d8312d547 100644 --- a/src/core/transport/chttp2/hpack_parser.c +++ b/src/core/transport/chttp2/hpack_parser.c @@ -1166,9 +1166,7 @@ static int append_string(grpc_chttp2_hpack_parser *p, const gpr_uint8 *cur, append_bytes(str, decoded, 3); goto b64_byte0; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - return 1; + GPR_UNREACHABLE_CODE(return 1); } /* append a null terminator to a string */ @@ -1313,9 +1311,7 @@ static int parse_value_string(grpc_chttp2_hpack_parser *p, const gpr_uint8 *cur, return 0; } /* Add code to prevent return without value error */ - gpr_log(GPR_ERROR, "Should never reach beyond switch in parse_value_string"); - abort(); - return 0; + GPR_UNREACHABLE_CODE(return 0); } static int parse_value_string_with_indexed_key(grpc_chttp2_hpack_parser *p, diff --git a/src/core/transport/chttp2/parsing.c b/src/core/transport/chttp2/parsing.c index f7a0a10581b..a0977ccaf6c 100644 --- a/src/core/transport/chttp2/parsing.c +++ b/src/core/transport/chttp2/parsing.c @@ -417,14 +417,10 @@ int grpc_chttp2_perform_read(grpc_exec_ctx *exec_ctx, transport_parsing->incoming_frame_size -= (gpr_uint32)(end - cur); return 1; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return 0); } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - - return 0; + GPR_UNREACHABLE_CODE(return 0); } static int init_frame_parser(grpc_chttp2_transport_parsing *transport_parsing) { @@ -580,9 +576,7 @@ static int init_data_frame_parser( case GRPC_CHTTP2_CONNECTION_ERROR: return 0; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - return 0; + GPR_UNREACHABLE_CODE(return 0); } static void free_timeout(void *p) { gpr_free(p); } @@ -820,7 +814,5 @@ static int parse_frame_slice(grpc_exec_ctx *exec_ctx, case GRPC_CHTTP2_CONNECTION_ERROR: return 0; } - gpr_log(GPR_ERROR, "should never reach here"); - abort(); - return 0; + GPR_UNREACHABLE_CODE(return 0); } diff --git a/src/core/transport/chttp2/stream_encoder.c b/src/core/transport/chttp2/stream_encoder.c index 83227e677db..6c7f7a9ea7f 100644 --- a/src/core/transport/chttp2/stream_encoder.c +++ b/src/core/transport/chttp2/stream_encoder.c @@ -428,7 +428,7 @@ static grpc_mdelem *hpack_enc(grpc_chttp2_hpack_compressor *c, emit_lithdr_noidx(c, dynidx(c, indices_key), elem, st); return elem; } - abort(); + GPR_UNREACHABLE_CODE(return NULL); } indices_key = c->indices_keys[HASH_FRAGMENT_3(key_hash)]; @@ -442,7 +442,7 @@ static grpc_mdelem *hpack_enc(grpc_chttp2_hpack_compressor *c, emit_lithdr_noidx(c, dynidx(c, indices_key), elem, st); return elem; } - abort(); + GPR_UNREACHABLE_CODE(return NULL); } /* no elem, key in the table... fall back to literal emission */ @@ -454,7 +454,7 @@ static grpc_mdelem *hpack_enc(grpc_chttp2_hpack_compressor *c, emit_lithdr_noidx_v(c, elem, st); return elem; } - abort(); + GPR_UNREACHABLE_CODE(return NULL); } #define STRLEN_LIT(x) (sizeof(x) - 1) diff --git a/src/cpp/server/server.cc b/src/cpp/server/server.cc index a44e1d20250..e09744b842b 100644 --- a/src/cpp/server/server.cc +++ b/src/cpp/server/server.cc @@ -153,8 +153,7 @@ class Server::SyncRequest GRPC_FINAL : public CompletionQueueTag { GPR_ASSERT((*req)->in_flight_); return true; } - gpr_log(GPR_ERROR, "Should never reach here"); - abort(); + GPR_UNREACHABLE_CODE(return false); } void SetupRequest() { cq_ = grpc_completion_queue_create(nullptr); } From a6a9a6df85797227f3435dd043325791f93b06a4 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 7 Oct 2015 16:40:04 -0700 Subject: [PATCH 05/33] Add incompressible responses and status echoing to Node interop server --- src/node/interop/interop_server.js | 63 +++++++++++++++++------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/src/node/interop/interop_server.js b/src/node/interop/interop_server.js index 3e83304faa3..5321005c863 100644 --- a/src/node/interop/interop_server.js +++ b/src/node/interop/interop_server.js @@ -44,6 +44,9 @@ var testProto = grpc.load({ var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial'; var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin'; +var incompressible_data = fs.readFileSync( + __dirname + '/../../../test/cpp/interop/rnd.dat'); + /** * Create a buffer filled with size zeroes * @param {number} size The length of the buffer @@ -83,6 +86,19 @@ function getEchoTrailer(call) { return response_trailer; } +function getPayload(payload_type, size) { + if (payload_type === 'RANDOM') { + payload_type = ['COMPRESSABLE', + 'UNCOMPRESSABLE'][Math.random() < 0.5 ? 0 : 1]; + } + var body; + switch (payload_type) { + case 'COMPRESSABLE': body = zeroBuffer(size); break; + case 'UNCOMPRESSABLE': incompressible_data.slice(size); break; + } + return {type: payload_type, body: body}; +} + /** * Respond to an empty parameter with an empty response. * NOTE: this currently does not work due to issue #137 @@ -104,13 +120,14 @@ function handleEmpty(call, callback) { function handleUnary(call, callback) { echoHeader(call); var req = call.request; - var zeros = zeroBuffer(req.response_size); - var payload_type = req.response_type; - if (payload_type === 'RANDOM') { - payload_type = ['COMPRESSABLE', - 'UNCOMPRESSABLE'][Math.random() < 0.5 ? 0 : 1]; + if (req.response_status) { + var status = req.response_status; + status.metadata = getEchoTrailer(call); + callback(status); + return; } - callback(null, {payload: {type: payload_type, body: zeros}}, + var payload = getPayload(req.response_type, req.response_size); + callback(null, {payload: payload}, getEchoTrailer(call)); } @@ -139,18 +156,14 @@ function handleStreamingInput(call, callback) { function handleStreamingOutput(call) { echoHeader(call); var req = call.request; - var payload_type = req.response_type; - if (payload_type === 'RANDOM') { - payload_type = ['COMPRESSABLE', - 'UNCOMPRESSABLE'][Math.random() < 0.5 ? 0 : 1]; + if (req.response_status) { + var status = req.response_status; + status.metadata = getEchoTrailer(call); + call.emit('error', status); + return; } _.each(req.response_parameters, function(resp_param) { - call.write({ - payload: { - body: zeroBuffer(resp_param.size), - type: payload_type - } - }); + call.write({payload: getPayload(req.response_type, resp_param.size)}); }); call.end(getEchoTrailer(call)); } @@ -163,18 +176,14 @@ function handleStreamingOutput(call) { function handleFullDuplex(call) { echoHeader(call); call.on('data', function(value) { - var payload_type = value.response_type; - if (payload_type === 'RANDOM') { - payload_type = ['COMPRESSABLE', - 'UNCOMPRESSABLE'][Math.random() < 0.5 ? 0 : 1]; + if (value.response_status) { + var status = value.response_status; + status.metadata = getEchoTrailer(call); + call.emit('error', status); + return; } _.each(value.response_parameters, function(resp_param) { - call.write({ - payload: { - body: zeroBuffer(resp_param.size), - type: payload_type - } - }); + call.write({payload: getPayload(value.response_type, resp_param.size)}); }); }); call.on('end', function() { @@ -188,7 +197,7 @@ function handleFullDuplex(call) { * @param {Call} call Call to handle */ function handleHalfDuplex(call) { - throw new Error('HalfDuplexCall not yet implemented'); + call.emit('error', Error('HalfDuplexCall not yet implemented')); } /** From 83815eab40568e142f05376dae48c2cef41bfefd Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 8 Oct 2015 09:31:45 -0700 Subject: [PATCH 06/33] Added interval_us delay in Node interop server --- src/node/interop/interop_server.js | 47 ++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/src/node/interop/interop_server.js b/src/node/interop/interop_server.js index 5321005c863..cc527364b4a 100644 --- a/src/node/interop/interop_server.js +++ b/src/node/interop/interop_server.js @@ -35,6 +35,7 @@ var fs = require('fs'); var path = require('path'); +var async = require('async'); var _ = require('lodash'); var grpc = require('..'); var testProto = grpc.load({ @@ -86,6 +87,22 @@ function getEchoTrailer(call) { return response_trailer; } +/** + * @typedef Payload + * @type {object} + * @property {string} payload_type The payload type + * @property {Buffer} body The payload body + */ + +/** + * Get a payload of the specified type and size. If the requested payload is + * COMPRESSABLE, it returns a zero buffer. If the type is UNCOMRESSABLE, it + * returns a slice of pre-loaded uncompressable data. If the type is RANDOM, + * it returns one of the other choices, chosen at random. + * @param {string} payload_type The type of payload to return + * @param {Number} size The size of the payload body + * @return {Payload} The requested payload + */ function getPayload(payload_type, size) { if (payload_type === 'RANDOM') { payload_type = ['COMPRESSABLE', @@ -99,6 +116,15 @@ function getPayload(payload_type, size) { return {type: payload_type, body: body}; } +function respondWithStream(call, request, callback) { + async.eachSeries(request.response_parameters, function(resp_param, callback) { + setTimeout(function() { + call.write({payload: getPayload(request.response_type, resp_param.size)}); + callback(); + }, resp_param.interval_us/1000); + }, callback); +} + /** * Respond to an empty parameter with an empty response. * NOTE: this currently does not work due to issue #137 @@ -162,10 +188,13 @@ function handleStreamingOutput(call) { call.emit('error', status); return; } - _.each(req.response_parameters, function(resp_param) { - call.write({payload: getPayload(req.response_type, resp_param.size)}); + respondWithStream(call, req, function(err) { + if (err) { + call.emit(err); + } else { + call.end(getEchoTrailer(call)); + } }); - call.end(getEchoTrailer(call)); } /** @@ -175,6 +204,7 @@ function handleStreamingOutput(call) { */ function handleFullDuplex(call) { echoHeader(call); + var call_ended; call.on('data', function(value) { if (value.response_status) { var status = value.response_status; @@ -182,12 +212,17 @@ function handleFullDuplex(call) { call.emit('error', status); return; } - _.each(value.response_parameters, function(resp_param) { - call.write({payload: getPayload(value.response_type, resp_param.size)}); + call.pause(); + respondWithStream(call, value, function(err) { + call.resume(); + if (call_ended) { + call.end(getEchoTrailer(call)); + } }); }); call.on('end', function() { - call.end(getEchoTrailer(call)); + call_ended = true; + }); } From 30df27aee191cfeea3ceb9e0993ef91d48343cad Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 8 Oct 2015 10:54:22 -0700 Subject: [PATCH 07/33] Made Node interop client use all specified command line flags --- src/node/interop/interop_client.js | 97 ++++++++++++++---------------- 1 file changed, 45 insertions(+), 52 deletions(-) diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js index 14cc6c0efe1..ac820dce3e3 100644 --- a/src/node/interop/interop_client.js +++ b/src/node/interop/interop_client.js @@ -44,13 +44,6 @@ var GoogleAuth = require('google-auth-library'); var assert = require('assert'); -var AUTH_SCOPE = 'https://www.googleapis.com/auth/xapi.zoo'; -var AUTH_SCOPE_RESPONSE = 'xapi.zoo'; -var AUTH_USER = ('155450119199-vefjjaekcc6cmsd5914v6lqufunmh9ue' + - '@developer.gserviceaccount.com'); -var COMPUTE_ENGINE_USER = ('155450119199-r5aaqa2vqoa9g5mv2m6s3m1l293rlmel' + - '@developer.gserviceaccount.com'); - var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial'; var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin'; @@ -369,7 +362,7 @@ function authTest(expected_user, scope, client, done) { assert.strictEqual(resp.payload.body.length, 314159); assert.strictEqual(resp.username, expected_user); if (scope) { - assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); + assert(scope.indexOf(resp.oauth_scope) > -1); } if (done) { done(); @@ -377,56 +370,49 @@ function authTest(expected_user, scope, client, done) { }); } -function oauth2Test(expected_user, scope, per_rpc, client, done) { - (new GoogleAuth()).getApplicationDefault(function(err, credential) { - assert.ifError(err); +function computeEngineCreds(client, done, extra) { + authTest(extra.service_account, null, client, done); +} + +function serviceAccountCreds(client, done, extra) { + authTest(extra.default_service_account, extra.oauth_scope, client, done); +} + +function jwtTokenCreds(client, done, extra) { + authTest(extra.default_service_account, null, client, done); +} + +function oauth2Test(client, done, extra) { var arg = { fill_username: true, fill_oauth_scope: true }; - credential = credential.createScoped(scope); - credential.getAccessToken(function(err, token) { - assert.ifError(err); - var updateMetadata = function(authURI, metadata, callback) { - metadata.add('authorization', 'Bearer ' + token); - callback(null, metadata); - }; - var makeTestCall = function(error, client_metadata) { - assert.ifError(error); - client.unaryCall(arg, function(err, resp) { - assert.ifError(err); - assert.strictEqual(resp.username, expected_user); - assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); - if (done) { - done(); - } - }, client_metadata); - }; - if (per_rpc) { - updateMetadata('', new grpc.Metadata(), makeTestCall); - } else { - client.$updateMetadata = updateMetadata; - makeTestCall(null, new grpc.Metadata()); - } - }); + client.unaryCall(arg, function(err, resp) { + assert.ifError(err); + assert.strictEqual(resp.username, extra.service_account); + assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1); + if (done) { + done(); + } }); } -function perRpcAuthTest(expected_user, scope, per_rpc, client, done) { +function perRpcAuthTest(client, done, extra) { (new GoogleAuth()).getApplicationDefault(function(err, credential) { assert.ifError(err); var arg = { fill_username: true, fill_oauth_scope: true }; + var scope = extra.oauth_scope; if (credential.createScopedRequired() && scope) { credential = credential.createScoped(scope); } var creds = grpc.credentials.createFromGoogleCredential(credential); client.unaryCall(arg, function(err, resp) { assert.ifError(err); - assert.strictEqual(resp.username, expected_user); - assert.strictEqual(resp.oauth_scope, AUTH_SCOPE_RESPONSE); + assert.strictEqual(resp.username, extra.service_account); + assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1); if (done) { done(); } @@ -483,15 +469,15 @@ var test_cases = { cancel_after_first_response: {run: cancelAfterFirstResponse}, timeout_on_sleeping_server: {run: timeoutOnSleepingServer}, custom_metadata: {run: customMetadata}, - compute_engine_creds: {run: _.partial(authTest, COMPUTE_ENGINE_USER, null), - getCreds: _.partial(getApplicationCreds, null)}, - service_account_creds: {run: _.partial(authTest, AUTH_USER, AUTH_SCOPE), - getCreds: _.partial(getApplicationCreds, AUTH_SCOPE)}, - jwt_token_creds: {run: _.partial(authTest, AUTH_USER, null), - getCreds: _.partial(getApplicationCreds, null)}, - oauth2_auth_token: {run: _.partial(oauth2Test, AUTH_USER, AUTH_SCOPE, false), - getCreds: _.partial(getOauth2Creds, AUTH_SCOPE)}, - per_rpc_creds: {run: _.partial(perRpcAuthTest, AUTH_USER, AUTH_SCOPE, true)} + compute_engine_creds: {run: computeEngineCreds, + getCreds: getApplicationCreds}, + service_account_creds: {run: serviceAccountCreds, + getCreds: getApplicationCreds}, + jwt_token_creds: {run: jwtTokenCreds, + getCreds: getApplicationCreds}, + oauth2_auth_token: {run: oauth2Test, + getCreds: getOauth2Creds}, + per_rpc_creds: {run: perRpcAuthTest} }; /** @@ -504,8 +490,9 @@ var test_cases = { * @param {bool} tls Indicates that a secure channel should be used * @param {function} done Callback to call when the test is completed. Included * primarily for use with mocha + * @param {object=} extra Extra options for some tests */ -function runTest(address, host_override, test_case, tls, test_ca, done) { +function runTest(address, host_override, test_case, tls, test_ca, done, extra) { // TODO(mlumish): enable TLS functionality var options = {}; var creds; @@ -534,7 +521,7 @@ function runTest(address, host_override, test_case, tls, test_ca, done) { }; if (test.getCreds) { - test.getCreds(function(err, new_creds) { + test.getCreds(extra.oauth_scope, function(err, new_creds) { execute(err, grpc.credentials.combineChannelCredentials( creds, new_creds)); }); @@ -547,13 +534,19 @@ if (require.main === module) { var parseArgs = require('minimist'); var argv = parseArgs(process.argv, { string: ['server_host', 'server_host_override', 'server_port', 'test_case', - 'use_tls', 'use_test_ca'] + 'use_tls', 'use_test_ca', 'default_service_account', 'oauth_scope', + 'service_account_key_file'] }); + var extra_args = { + service_account: argv.default_service_account, + oauth_scope: argv.oauth_scope, + service_account_key_file: argv.service_account_key_file + }; runTest(argv.server_host + ':' + argv.server_port, argv.server_host_override, argv.test_case, argv.use_tls === 'true', argv.use_test_ca === 'true', function () { console.log('OK:', argv.test_case); - }); + }, extra_args); } /** From b8ceb7c5500e70b669329878dda35156a9306175 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 8 Oct 2015 11:07:16 -0700 Subject: [PATCH 08/33] Added remaining implementable node interop tests, except compression --- src/node/interop/interop_client.js | 78 +++++++++++++++++++++++----- src/node/src/server.js | 2 +- src/node/test/interop_sanity_test.js | 8 +++ 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js index ac820dce3e3..54a256f277f 100644 --- a/src/node/interop/interop_client.js +++ b/src/node/interop/interop_client.js @@ -338,6 +338,41 @@ function customMetadata(client, done) { stream.end(); } +function statusCodeAndMessage(client, done) { + done = multiDone(done, 2); + var arg = { + response_status: { + code: 2, + message: "test status message" + } + }; + client.unaryCall(arg, function(err, resp) { + assert(err); + assert.strictEqual(err.code, 2); + assert.strictEqual(err.message, "test status message"); + done(); + }); + var duplex = client.fullDuplexCall(); + duplex.on('status', function(status) { + assert(status); + assert.strictEqual(status.code, 2); + assert.strictEqual(status.details, "test status message"); + done(); + }); + duplex.on('error', function(){}); + duplex.write(arg); + duplex.end(); +} + +function unimplementedMethod(client, done) { + client.unimplementedCall({}, function(err, resp) { + assert(err); + assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED); + assert(!err.message); + done(); + }); +} + /** * Run one of the authentication tests. * @param {string} expected_user The expected username in the response @@ -459,25 +494,44 @@ function getOauth2Creds(scope, callback) { * Map from test case names to test functions */ var test_cases = { - empty_unary: {run: emptyUnary}, - large_unary: {run: largeUnary}, - client_streaming: {run: clientStreaming}, - server_streaming: {run: serverStreaming}, - ping_pong: {run: pingPong}, - empty_stream: {run: emptyStream}, - cancel_after_begin: {run: cancelAfterBegin}, - cancel_after_first_response: {run: cancelAfterFirstResponse}, - timeout_on_sleeping_server: {run: timeoutOnSleepingServer}, - custom_metadata: {run: customMetadata}, + empty_unary: {run: emptyUnary, + Client: testProto.TestService}, + large_unary: {run: largeUnary, + Client: testProto.TestService}, + client_streaming: {run: clientStreaming, + Client: testProto.TestService}, + server_streaming: {run: serverStreaming, + Client: testProto.TestService}, + ping_pong: {run: pingPong, + Client: testProto.TestService}, + empty_stream: {run: emptyStream, + Client: testProto.TestService}, + cancel_after_begin: {run: cancelAfterBegin, + Client: testProto.TestService}, + cancel_after_first_response: {run: cancelAfterFirstResponse, + Client: testProto.TestService}, + timeout_on_sleeping_server: {run: timeoutOnSleepingServer, + Client: testProto.TestService}, + custom_metadata: {run: customMetadata, + Client: testProto.TestService}, + status_code_and_message: {run: statusCodeAndMessage, + Client: testProto.TestService}, + unimplemented_method: {run: unimplementedMethod, + Client: testProto.UnimplementedService}, compute_engine_creds: {run: computeEngineCreds, + Client: testProto.TestService, getCreds: getApplicationCreds}, service_account_creds: {run: serviceAccountCreds, + Client: testProto.TestService, getCreds: getApplicationCreds}, jwt_token_creds: {run: jwtTokenCreds, + Client: testProto.TestService, getCreds: getApplicationCreds}, oauth2_auth_token: {run: oauth2Test, + Client: testProto.TestService, getCreds: getOauth2Creds}, - per_rpc_creds: {run: perRpcAuthTest} + per_rpc_creds: {run: perRpcAuthTest, + Client: testProto.TestService} }; /** @@ -516,7 +570,7 @@ function runTest(address, host_override, test_case, tls, test_ca, done, extra) { var execute = function(err, creds) { assert.ifError(err); - var client = new testProto.TestService(address, creds, options); + var client = new test.Client(address, creds, options); test.run(client, done); }; diff --git a/src/node/src/server.js b/src/node/src/server.js index 87b5b7ad06e..a974d593c95 100644 --- a/src/node/src/server.js +++ b/src/node/src/server.js @@ -629,7 +629,7 @@ function Server(options) { (new Metadata())._getCoreRepresentation(); batch[grpc.opType.SEND_STATUS_FROM_SERVER] = { code: grpc.status.UNIMPLEMENTED, - details: 'This method is not available on this server.', + details: '', metadata: {} }; batch[grpc.opType.RECV_CLOSE_ON_SERVER] = true; diff --git a/src/node/test/interop_sanity_test.js b/src/node/test/interop_sanity_test.js index 804c1d45e4a..f008a87585c 100644 --- a/src/node/test/interop_sanity_test.js +++ b/src/node/test/interop_sanity_test.js @@ -94,4 +94,12 @@ describe('Interop tests', function() { interop_client.runTest(port, name_override, 'custom_metadata', true, true, done); }); + it('should pass status_code_and_message', function(done) { + interop_client.runTest(port, name_override, 'status_code_and_message', + true, true, done); + }); + it('should pass unimplemented_method', function(done) { + interop_client.runTest(port, name_override, 'unimplemented_method', + true, true, done); + }); }); From 9a52908f210b5ed86c9ecaff6b11623c3a1cab82 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Thu, 8 Oct 2015 13:12:14 -0700 Subject: [PATCH 09/33] Fixing #3680 The server auth filter needs a reference on the server credentials so that the processor that belongs to the creds is not destroyed when the server auth filter is still using it. The server auth filter also does not need the security connector but just the auth context. --- src/core/security/credentials.c | 42 ++++++++++++++++++++++++ src/core/security/credentials.h | 8 ++++- src/core/security/security_context.c | 26 ++++++++++----- src/core/security/security_context.h | 11 +++---- src/core/security/server_auth_filter.c | 36 ++++++++++---------- src/core/security/server_secure_chttp2.c | 4 +-- 6 files changed, 91 insertions(+), 36 deletions(-) diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c index 398db20e8cf..5e155d83b9c 100644 --- a/src/core/security/credentials.c +++ b/src/core/security/credentials.c @@ -181,6 +181,48 @@ void grpc_server_credentials_set_auth_metadata_processor( creds->processor = processor; } +static void server_credentials_pointer_arg_destroy(void *p) { + grpc_server_credentials_unref(p); +} + +static void *server_credentials_pointer_arg_copy(void *p) { + return grpc_server_credentials_ref(p); +} + +grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *p) { + grpc_arg arg; + memset(&arg, 0, sizeof(grpc_arg)); + arg.type = GRPC_ARG_POINTER; + arg.key = GRPC_SERVER_CREDENTIALS_ARG; + arg.value.pointer.p = p; + arg.value.pointer.copy = server_credentials_pointer_arg_copy; + arg.value.pointer.destroy = server_credentials_pointer_arg_destroy; + return arg; +} + +grpc_server_credentials *grpc_server_credentials_from_arg( + const grpc_arg *arg) { + if (strcmp(arg->key, GRPC_SERVER_CREDENTIALS_ARG) != 0) return NULL; + if (arg->type != GRPC_ARG_POINTER) { + gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, + GRPC_SERVER_CREDENTIALS_ARG); + return NULL; + } + return arg->value.pointer.p; +} + +grpc_server_credentials *grpc_find_server_credentials_in_args( + const grpc_channel_args *args) { + size_t i; + if (args == NULL) return NULL; + for (i = 0; i < args->num_args; i++) { + grpc_server_credentials *p = + grpc_server_credentials_from_arg(&args->args[i]); + if (p != NULL) return p; + } + return NULL; +} + /* -- Ssl credentials. -- */ static void ssl_destruct(grpc_credentials *creds) { diff --git a/src/core/security/credentials.h b/src/core/security/credentials.h index b213e052d34..01203b08f1f 100644 --- a/src/core/security/credentials.h +++ b/src/core/security/credentials.h @@ -215,7 +215,6 @@ typedef struct { grpc_server_credentials *c, grpc_security_connector **sc); } grpc_server_credentials_vtable; -/* TODO(jboeuf): Add a refcount. */ struct grpc_server_credentials { const grpc_server_credentials_vtable *vtable; const char *type; @@ -231,6 +230,13 @@ grpc_server_credentials *grpc_server_credentials_ref( void grpc_server_credentials_unref(grpc_server_credentials *creds); +#define GRPC_SERVER_CREDENTIALS_ARG "grpc.server_credentials" + +grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *c); +grpc_server_credentials *grpc_server_credentials_from_arg(const grpc_arg *arg); +grpc_server_credentials *grpc_find_server_credentials_in_args( + const grpc_channel_args *args); + /* -- Ssl credentials. -- */ typedef struct { diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index fb905e0b228..f544c1d943d 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -305,33 +305,43 @@ void grpc_auth_property_reset(grpc_auth_property *property) { memset(property, 0, sizeof(grpc_auth_property)); } -grpc_arg grpc_auth_metadata_processor_to_arg(grpc_auth_metadata_processor *p) { +static void auth_context_pointer_arg_destroy(void *p) { + GRPC_AUTH_CONTEXT_UNREF(p, "auth_context_pointer_arg"); +} + +static void *auth_context_pointer_arg_copy(void *p) { + return GRPC_AUTH_CONTEXT_REF(p, "auth_context_pointer_arg"); +} + +grpc_arg grpc_auth_context_to_arg(grpc_auth_context *p) { grpc_arg arg; memset(&arg, 0, sizeof(grpc_arg)); arg.type = GRPC_ARG_POINTER; - arg.key = GRPC_AUTH_METADATA_PROCESSOR_ARG; + arg.key = GRPC_AUTH_CONTEXT_ARG; arg.value.pointer.p = p; + arg.value.pointer.copy = auth_context_pointer_arg_copy; + arg.value.pointer.destroy = auth_context_pointer_arg_destroy; return arg; } -grpc_auth_metadata_processor *grpc_auth_metadata_processor_from_arg( +grpc_auth_context *grpc_auth_context_from_arg( const grpc_arg *arg) { - if (strcmp(arg->key, GRPC_AUTH_METADATA_PROCESSOR_ARG) != 0) return NULL; + if (strcmp(arg->key, GRPC_AUTH_CONTEXT_ARG) != 0) return NULL; if (arg->type != GRPC_ARG_POINTER) { gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, - GRPC_AUTH_METADATA_PROCESSOR_ARG); + GRPC_AUTH_CONTEXT_ARG); return NULL; } return arg->value.pointer.p; } -grpc_auth_metadata_processor *grpc_find_auth_metadata_processor_in_args( +grpc_auth_context *grpc_find_auth_context_in_args( const grpc_channel_args *args) { size_t i; if (args == NULL) return NULL; for (i = 0; i < args->num_args; i++) { - grpc_auth_metadata_processor *p = - grpc_auth_metadata_processor_from_arg(&args->args[i]); + grpc_auth_context *p = + grpc_auth_context_from_arg(&args->args[i]); if (p != NULL) return p; } return NULL; diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index a9a03064108..2bbdc4be973 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -103,13 +103,12 @@ typedef struct { grpc_server_security_context *grpc_server_security_context_create(void); void grpc_server_security_context_destroy(void *ctx); -/* --- Auth metadata processing. --- */ -#define GRPC_AUTH_METADATA_PROCESSOR_ARG "grpc.auth_metadata_processor" +/* --- Channel args for auth context --- */ +#define GRPC_AUTH_CONTEXT_ARG "grpc.auth_context" -grpc_arg grpc_auth_metadata_processor_to_arg(grpc_auth_metadata_processor *p); -grpc_auth_metadata_processor *grpc_auth_metadata_processor_from_arg( - const grpc_arg *arg); -grpc_auth_metadata_processor *grpc_find_auth_metadata_processor_in_args( +grpc_arg grpc_auth_context_to_arg(grpc_auth_context *c); +grpc_auth_context *grpc_auth_context_from_arg(const grpc_arg *arg); +grpc_auth_context *grpc_find_auth_context_in_args( const grpc_channel_args *args); #endif /* GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H */ diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c index 30ca9f57a25..2e18369fe8c 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -34,7 +34,7 @@ #include #include "src/core/security/auth_filters.h" -#include "src/core/security/security_connector.h" +#include "src/core/security/credentials.h" #include "src/core/security/security_context.h" #include @@ -58,8 +58,8 @@ typedef struct call_data { } call_data; typedef struct channel_data { - grpc_security_connector *security_connector; - grpc_auth_metadata_processor processor; + grpc_auth_context *auth_context; + grpc_server_credentials *creds; grpc_mdctx *mdctx; } channel_data; @@ -160,12 +160,12 @@ static void auth_on_recv(grpc_exec_ctx *exec_ctx, void *user_data, grpc_stream_op *op = &ops[i]; if (op->type != GRPC_OP_METADATA || calld->got_client_metadata) continue; calld->got_client_metadata = 1; - if (chand->processor.process == NULL) continue; + if (chand->creds->processor.process == NULL) continue; calld->md_op = op; calld->md = metadata_batch_to_md_array(&op->data.metadata); - chand->processor.process(chand->processor.state, calld->auth_context, - calld->md.metadata, calld->md.count, - on_md_processing_done, elem); + chand->creds->processor.process( + chand->creds->processor.state, calld->auth_context, + calld->md.metadata, calld->md.count, on_md_processing_done, elem); return; } } @@ -221,7 +221,7 @@ static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, } server_ctx = grpc_server_security_context_create(); server_ctx->auth_context = - grpc_auth_context_create(chand->security_connector->auth_context); + grpc_auth_context_create(chand->auth_context); server_ctx->auth_context->pollset = initial_op->bind_pollset; initial_op->context[GRPC_CONTEXT_SECURITY].value = server_ctx; initial_op->context[GRPC_CONTEXT_SECURITY].destroy = @@ -241,9 +241,8 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem, grpc_channel *master, const grpc_channel_args *args, grpc_mdctx *mdctx, int is_first, int is_last) { - grpc_security_connector *sc = grpc_find_security_connector_in_args(args); - grpc_auth_metadata_processor *processor = - grpc_find_auth_metadata_processor_in_args(args); + grpc_auth_context *auth_context = grpc_find_auth_context_in_args(args); + grpc_server_credentials *creds = grpc_find_server_credentials_in_args(args); /* grab pointers to our data from the channel element */ channel_data *chand = elem->channel_data; @@ -252,15 +251,14 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx, path */ GPR_ASSERT(!is_first); GPR_ASSERT(!is_last); - GPR_ASSERT(sc != NULL); - GPR_ASSERT(processor != NULL); + GPR_ASSERT(auth_context != NULL); + GPR_ASSERT(creds != NULL); /* initialize members */ - GPR_ASSERT(!sc->is_client_side); - chand->security_connector = - GRPC_SECURITY_CONNECTOR_REF(sc, "server_auth_filter"); + chand->auth_context = + GRPC_AUTH_CONTEXT_REF(auth_context, "server_auth_filter"); + chand->creds = grpc_server_credentials_ref(creds); chand->mdctx = mdctx; - chand->processor = *processor; } /* Destructor for channel data */ @@ -268,8 +266,8 @@ static void destroy_channel_elem(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem) { /* grab pointers to our data from the channel element */ channel_data *chand = elem->channel_data; - GRPC_SECURITY_CONNECTOR_UNREF(chand->security_connector, - "server_auth_filter"); + GRPC_AUTH_CONTEXT_UNREF(chand->auth_context, "server_auth_filter"); + grpc_server_credentials_unref(chand->creds); } const grpc_channel_filter grpc_server_auth_filter = { diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index 881e44a3fed..82c639e8301 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -93,9 +93,9 @@ static void setup_transport(grpc_exec_ctx *exec_ctx, void *statep, grpc_server_secure_state *state = statep; grpc_channel_args *args_copy; grpc_arg args_to_add[2]; - args_to_add[0] = grpc_security_connector_to_arg(state->sc); + args_to_add[0] = grpc_server_credentials_to_arg(state->creds); args_to_add[1] = - grpc_auth_metadata_processor_to_arg(&state->creds->processor); + grpc_auth_context_to_arg(state->sc->auth_context); args_copy = grpc_channel_args_copy_and_add( grpc_server_get_channel_args(state->server), args_to_add, GPR_ARRAY_SIZE(args_to_add)); From 35505de42c3595bd2a8c2acad06f975c6f728b41 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 8 Oct 2015 13:31:33 -0700 Subject: [PATCH 10/33] Retry timeouts on pull requests --- tools/run_tests/run_tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index e938520403d..6b7ee7eabe4 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -101,7 +101,8 @@ class SimpleConfig(object): timeout_seconds=self.timeout_seconds, hash_targets=hash_targets if self.allow_hashing else None, - flake_retries=5 if args.allow_flakes else 0) + flake_retries=5 if args.allow_flakes else 0, + timeout_retries=3 if args.allow_flakes else 0) # ValgrindConfig: compile with some CONFIG=config, but use valgrind to run @@ -121,7 +122,7 @@ class ValgrindConfig(object): shortname='valgrind %s' % cmdline[0], hash_targets=None, flake_retries=5 if args.allow_flakes else 0, - timeout_retries=2 if args.allow_flakes else 0) + timeout_retries=3 if args.allow_flakes else 0) def get_c_tests(travis, test_lang) : From 0c331880d0062f39f434ba8ad2341252e38bed79 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Thu, 8 Oct 2015 14:51:54 -0700 Subject: [PATCH 11/33] Hid test-only functions from surface/call.h --- BUILD | 3 + build.yaml | 1 + gRPC.podspec | 2 + src/core/surface/call.c | 12 ++-- src/core/surface/call.h | 10 --- src/core/surface/call_test_only.h | 65 +++++++++++++++++++ test/core/end2end/tests/compressed_payload.c | 11 ++-- test/cpp/interop/client_helper.h | 6 +- test/cpp/interop/server_helper.cc | 6 +- tools/doxygen/Doxyfile.core.internal | 1 + tools/run_tests/sources_and_headers.json | 4 ++ vsprojects/vcxproj/grpc/grpc.vcxproj | 1 + vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 3 + .../grpc_unsecure/grpc_unsecure.vcxproj | 1 + .../grpc_unsecure.vcxproj.filters | 3 + 15 files changed, 104 insertions(+), 25 deletions(-) create mode 100644 src/core/surface/call_test_only.h diff --git a/BUILD b/BUILD index 3e2a45b8a0a..ffeb31e424b 100644 --- a/BUILD +++ b/BUILD @@ -223,6 +223,7 @@ cc_library( "src/core/surface/api_trace.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", + "src/core/surface/call_test_only.h", "src/core/surface/channel.h", "src/core/surface/completion_queue.h", "src/core/surface/event_string.h", @@ -509,6 +510,7 @@ cc_library( "src/core/surface/api_trace.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", + "src/core/surface/call_test_only.h", "src/core/surface/channel.h", "src/core/surface/completion_queue.h", "src/core/surface/event_string.h", @@ -1296,6 +1298,7 @@ objc_library( "src/core/surface/api_trace.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", + "src/core/surface/call_test_only.h", "src/core/surface/channel.h", "src/core/surface/completion_queue.h", "src/core/surface/event_string.h", diff --git a/build.yaml b/build.yaml index 98fb0348ca3..e277b60b77e 100644 --- a/build.yaml +++ b/build.yaml @@ -183,6 +183,7 @@ filegroups: - src/core/surface/api_trace.h - src/core/surface/byte_buffer_queue.h - src/core/surface/call.h + - src/core/surface/call_test_only.h - src/core/surface/channel.h - src/core/surface/completion_queue.h - src/core/surface/event_string.h diff --git a/gRPC.podspec b/gRPC.podspec index 717e7005ee5..6378cf0413c 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -227,6 +227,7 @@ Pod::Spec.new do |s| 'src/core/surface/api_trace.h', 'src/core/surface/byte_buffer_queue.h', 'src/core/surface/call.h', + 'src/core/surface/call_test_only.h', 'src/core/surface/channel.h', 'src/core/surface/completion_queue.h', 'src/core/surface/event_string.h', @@ -518,6 +519,7 @@ Pod::Spec.new do |s| 'src/core/surface/api_trace.h', 'src/core/surface/byte_buffer_queue.h', 'src/core/surface/call.h', + 'src/core/surface/call_test_only.h', 'src/core/surface/channel.h', 'src/core/surface/completion_queue.h', 'src/core/surface/event_string.h', diff --git a/src/core/surface/call.c b/src/core/surface/call.c index c8b880b0ad0..630be2f73ac 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -521,7 +521,7 @@ static void set_compression_algorithm(grpc_call *call, call->compression_algorithm = algo; } -grpc_compression_algorithm grpc_call_get_compression_algorithm( +grpc_compression_algorithm grpc_call_test_only_get_compression_algorithm( grpc_call *call) { grpc_compression_algorithm algorithm; gpr_mu_lock(&call->mu); @@ -561,11 +561,15 @@ static void set_encodings_accepted_by_peer( } } -gpr_uint32 grpc_call_get_encodings_accepted_by_peer(grpc_call *call) { - return call->encodings_accepted_by_peer; +gpr_uint32 grpc_call_test_only_get_encodings_accepted_by_peer(grpc_call *call) { + gpr_uint32 encodings_accepted_by_peer; + gpr_mu_lock(&call->mu); + encodings_accepted_by_peer = call->encodings_accepted_by_peer; + gpr_mu_unlock(&call->mu); + return encodings_accepted_by_peer; } -gpr_uint32 grpc_call_get_message_flags(grpc_call *call) { +gpr_uint32 grpc_call_test_only_get_message_flags(grpc_call *call) { gpr_uint32 flags; gpr_mu_lock(&call->mu); flags = call->incoming_message_flags; diff --git a/src/core/surface/call.h b/src/core/surface/call.h index cd1db3d417c..9b7c6f9bfbe 100644 --- a/src/core/surface/call.h +++ b/src/core/surface/call.h @@ -169,16 +169,6 @@ void *grpc_call_context_get(grpc_call *call, grpc_context_index elem); gpr_uint8 grpc_call_is_client(grpc_call *call); -grpc_compression_algorithm grpc_call_get_compression_algorithm(grpc_call *call); - -gpr_uint32 grpc_call_get_message_flags(grpc_call *call); - -/** Returns a bitset for the encodings (compression algorithms) supported by \a - * call's peer. - * - * To be indexed by grpc_compression_algorithm enum values. */ -gpr_uint32 grpc_call_get_encodings_accepted_by_peer(grpc_call *call); - #ifdef __cplusplus } #endif diff --git a/src/core/surface/call_test_only.h b/src/core/surface/call_test_only.h new file mode 100644 index 00000000000..df4be3248b2 --- /dev/null +++ b/src/core/surface/call_test_only.h @@ -0,0 +1,65 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef GRPC_INTERNAL_CORE_SURFACE_CALL_TEST_ONLY_H +#define GRPC_INTERNAL_CORE_SURFACE_CALL_TEST_ONLY_H + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** Return the compression algorithm from \a call. + * + * \warning This function should \b only be used in test code. */ +grpc_compression_algorithm grpc_call_test_only_get_compression_algorithm( + grpc_call *call); + +/** Return the message flags from \a call. + * + * \warning This function should \b only be used in test code. */ +gpr_uint32 grpc_call_test_only_get_message_flags(grpc_call *call); + +/** Returns a bitset for the encodings (compression algorithms) supported by \a + * call's peer. + * + * To be indexed by grpc_compression_algorithm enum values. */ +gpr_uint32 grpc_call_test_only_get_encodings_accepted_by_peer(grpc_call *call); + + +#ifdef __cplusplus +} +#endif + +#endif /* GRPC_INTERNAL_CORE_SURFACE_CALL_TEST_ONLY_H */ diff --git a/test/core/end2end/tests/compressed_payload.c b/test/core/end2end/tests/compressed_payload.c index a4614a2aba4..f321fe1e7c7 100644 --- a/test/core/end2end/tests/compressed_payload.c +++ b/test/core/end2end/tests/compressed_payload.c @@ -46,7 +46,7 @@ #include "test/core/end2end/cq_verifier.h" #include "src/core/channel/channel_args.h" #include "src/core/channel/compress_filter.h" -#include "src/core/surface/call.h" +#include "src/core/surface/call_test_only.h" enum { TIMEOUT = 200000 }; @@ -196,12 +196,13 @@ static void request_with_payload_template( cq_expect_completion(cqv, tag(101), 1); cq_verify(cqv); - GPR_ASSERT(GPR_BITCOUNT(grpc_call_get_encodings_accepted_by_peer(s)) == 3); - GPR_ASSERT(GPR_BITGET(grpc_call_get_encodings_accepted_by_peer(s), + GPR_ASSERT( + GPR_BITCOUNT(grpc_call_test_only_get_encodings_accepted_by_peer(s)) == 3); + GPR_ASSERT(GPR_BITGET(grpc_call_test_only_get_encodings_accepted_by_peer(s), GRPC_COMPRESS_NONE) != 0); - GPR_ASSERT(GPR_BITGET(grpc_call_get_encodings_accepted_by_peer(s), + GPR_ASSERT(GPR_BITGET(grpc_call_test_only_get_encodings_accepted_by_peer(s), GRPC_COMPRESS_DEFLATE) != 0); - GPR_ASSERT(GPR_BITGET(grpc_call_get_encodings_accepted_by_peer(s), + GPR_ASSERT(GPR_BITGET(grpc_call_test_only_get_encodings_accepted_by_peer(s), GRPC_COMPRESS_GZIP) != 0); op = ops; diff --git a/test/cpp/interop/client_helper.h b/test/cpp/interop/client_helper.h index 0221df93db5..ace193042ee 100644 --- a/test/cpp/interop/client_helper.h +++ b/test/cpp/interop/client_helper.h @@ -38,7 +38,7 @@ #include -#include "src/core/surface/call.h" +#include "src/core/surface/call_test_only.h" namespace grpc { namespace testing { @@ -57,11 +57,11 @@ class InteropClientContextInspector { // Inspector methods, able to peek inside ClientContext, follow. grpc_compression_algorithm GetCallCompressionAlgorithm() const { - return grpc_call_get_compression_algorithm(context_.call_); + return grpc_call_test_only_get_compression_algorithm(context_.call_); } gpr_uint32 GetMessageFlags() const { - return grpc_call_get_message_flags(context_.call_); + return grpc_call_test_only_get_message_flags(context_.call_); } private: diff --git a/test/cpp/interop/server_helper.cc b/test/cpp/interop/server_helper.cc index 45707508466..5138a38170a 100644 --- a/test/cpp/interop/server_helper.cc +++ b/test/cpp/interop/server_helper.cc @@ -38,7 +38,7 @@ #include #include -#include "src/core/surface/call.h" +#include "src/core/surface/call_test_only.h" #include "test/core/end2end/data/ssl_test_data.h" DECLARE_bool(use_tls); @@ -65,11 +65,11 @@ InteropServerContextInspector::InteropServerContextInspector( grpc_compression_algorithm InteropServerContextInspector::GetCallCompressionAlgorithm() const { - return grpc_call_get_compression_algorithm(context_.call_); + return grpc_call_test_only_get_compression_algorithm(context_.call_); } gpr_uint32 InteropServerContextInspector::GetEncodingsAcceptedByClient() const { - return grpc_call_get_encodings_accepted_by_peer(context_.call_); + return grpc_call_test_only_get_encodings_accepted_by_peer(context_.call_); } std::shared_ptr diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 5658a102d75..5fd3ee70d63 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -858,6 +858,7 @@ src/core/statistics/census_rpc_stats.h \ src/core/surface/api_trace.h \ src/core/surface/byte_buffer_queue.h \ src/core/surface/call.h \ +src/core/surface/call_test_only.h \ src/core/surface/channel.h \ src/core/surface/completion_queue.h \ src/core/surface/event_string.h \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 1ceff15a3be..76d66ed7a07 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -12365,6 +12365,7 @@ "src/core/surface/api_trace.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", + "src/core/surface/call_test_only.h", "src/core/surface/channel.h", "src/core/surface/completion_queue.h", "src/core/surface/event_string.h", @@ -12607,6 +12608,7 @@ "src/core/surface/call.h", "src/core/surface/call_details.c", "src/core/surface/call_log_batch.c", + "src/core/surface/call_test_only.h", "src/core/surface/channel.c", "src/core/surface/channel.h", "src/core/surface/channel_connectivity.c", @@ -12861,6 +12863,7 @@ "src/core/surface/api_trace.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", + "src/core/surface/call_test_only.h", "src/core/surface/channel.h", "src/core/surface/completion_queue.h", "src/core/surface/event_string.h", @@ -13073,6 +13076,7 @@ "src/core/surface/call.h", "src/core/surface/call_details.c", "src/core/surface/call_log_batch.c", + "src/core/surface/call_test_only.h", "src/core/surface/channel.c", "src/core/surface/channel.h", "src/core/surface/channel_connectivity.c", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 183edbc05bf..88b883f67cf 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -344,6 +344,7 @@ + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 66ce9ca05be..1a46ac8e34b 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -743,6 +743,9 @@ src\core\surface + + src\core\surface + src\core\surface diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index b527179f9f3..cddd00374b2 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -323,6 +323,7 @@ + diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index 7be3c9ec93b..01e0d8dadde 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -641,6 +641,9 @@ src\core\surface + + src\core\surface + src\core\surface From 90540b4338943f388434ccfade4bb20cace4c777 Mon Sep 17 00:00:00 2001 From: yang-g Date: Thu, 8 Oct 2015 15:13:35 -0700 Subject: [PATCH 12/33] add another return in case the macro is not defined and the compiler complains about no return statement --- src/core/surface/call.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index c8152b6eae9..f9e6a596741 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -435,6 +435,7 @@ static grpc_cq_completion *allocate_completion(grpc_call *call) { return &call->completions[i]; } GPR_UNREACHABLE_CODE(return NULL); + return NULL; } static void done_completion(grpc_exec_ctx *exec_ctx, void *call, From e2686282ace62f8b6a5e9ed7c836b415605d053f Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 8 Oct 2015 16:27:07 -0700 Subject: [PATCH 13/33] kill interop clients on timeout --- tools/run_tests/dockerjob.py | 34 ++++++++++----------------- tools/run_tests/jobset.py | 6 ++++- tools/run_tests/run_interop_tests.py | 35 +++++++++++++++++++++------- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/tools/run_tests/dockerjob.py b/tools/run_tests/dockerjob.py index 11686d46b09..266edd4375b 100755 --- a/tools/run_tests/dockerjob.py +++ b/tools/run_tests/dockerjob.py @@ -38,18 +38,15 @@ import subprocess _DEVNULL = open(os.devnull, 'w') -def wait_for_file(filepath, timeout_seconds=15): - """Wait until given file exists and returns its content.""" - started = time.time() - while time.time() - started < timeout_seconds: - if os.path.isfile(filepath): - with open(filepath, 'r') as f: - content = f.read() - # make sure we don't return empty content - if content: - return content - time.sleep(1) - raise Exception('Failed to read file %s.' % filepath) + +def random_name(base_name): + """Randomizes given base name.""" + return '%s_%s' % (base_name, uuid.uuid4()) + + +def docker_kill(cid): + """Kills a docker container. Returns True if successful.""" + return subprocess.call(['docker','kill', str(cid)]) == 0 def docker_mapped_port(cid, port): @@ -92,23 +89,16 @@ class DockerJob: def __init__(self, spec): self._spec = spec self._job = jobset.Job(spec, bin_hash=None, newline_on_success=True, travis=True, add_env={}, xml_report=None) - self._cidfile = spec.cidfile - self._cid = None - - def cid(self): - """Gets cid of this container""" - if not self._cid: - self._cid = wait_for_file(self._cidfile) - return self._cid + self._container_name = spec.container_name def mapped_port(self, port): - return docker_mapped_port(self.cid(), port) + return docker_mapped_port(self._container_name, port) def kill(self, suppress_failure=False): """Sends kill signal to the container.""" if suppress_failure: self._job.suppress_failure_message() - return subprocess.call(['docker','kill', self.cid()]) == 0 + return docker_kill(self._container_name) def is_running(self): """Polls a job and returns True if given job is still running.""" diff --git a/tools/run_tests/jobset.py b/tools/run_tests/jobset.py index 87be703b4cd..17a63c02e84 100755 --- a/tools/run_tests/jobset.py +++ b/tools/run_tests/jobset.py @@ -135,13 +135,14 @@ class JobSpec(object): def __init__(self, cmdline, shortname=None, environ=None, hash_targets=None, cwd=None, shell=False, timeout_seconds=5*60, flake_retries=0, - timeout_retries=0): + timeout_retries=0, kill_handler=None): """ Arguments: cmdline: a list of arguments to pass as the command line environ: a dictionary of environment variables to set in the child process hash_targets: which files to include in the hash representing the jobs version (or empty, indicating the job should not be hashed) + kill_handler: a handler that will be called whenever job.kill() is invoked """ if environ is None: environ = {} @@ -156,6 +157,7 @@ class JobSpec(object): self.timeout_seconds = timeout_seconds self.flake_retries = flake_retries self.timeout_retries = timeout_retries + self.kill_handler = kill_handler def identity(self): return '%r %r %r' % (self.cmdline, self.environ, self.hash_targets) @@ -254,6 +256,8 @@ class Job(object): def kill(self): if self._state == _RUNNING: self._state = _KILLED + if self._spec.kill_handler: + self._spec.kill_handler(self) self._process.terminate() def suppress_failure_message(self): diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index f328f4642ee..4d09ae7fcda 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -321,17 +321,29 @@ def add_auth_options(language, test_case, cmdline, env): return (cmdline, env) +def _job_kill_handler(job): + if job._spec.container_name: + dockerjob.docker_kill(job._spec.container_name) + + def cloud_to_prod_jobspec(language, test_case, docker_image=None, auth=False): """Creates jobspec for cloud-to-prod interop test""" cmdline = language.cloud_to_prod_args() + ['--test_case=%s' % test_case] cwd = language.client_cwd environ = language.cloud_to_prod_env() + container_name = None if auth: cmdline, environ = add_auth_options(language, test_case, cmdline, environ) cmdline = bash_login_cmdline(cmdline) if docker_image: - cmdline = docker_run_cmdline(cmdline, image=docker_image, cwd=cwd, environ=environ) + container_name = dockerjob.random_name('interop_client_%s' % language) + cmdline = docker_run_cmdline(cmdline, + image=docker_image, + cwd=cwd, + environ=environ, + docker_args=['--net=host', + '--name', container_name]) cwd = None environ = None @@ -343,7 +355,9 @@ def cloud_to_prod_jobspec(language, test_case, docker_image=None, auth=False): shortname="%s:%s:%s" % (suite_name, language, test_case), timeout_seconds=2*60, flake_retries=5 if args.allow_flakes else 0, - timeout_retries=2 if args.allow_flakes else 0) + timeout_retries=2 if args.allow_flakes else 0, + kill_handler=_job_kill_handler) + test_job.container_name = container_name return test_job @@ -356,11 +370,14 @@ def cloud_to_cloud_jobspec(language, test_case, server_name, server_host, '--server_port=%s' % server_port ]) cwd = language.client_cwd if docker_image: + container_name = dockerjob.random_name('interop_client_%s' % language) cmdline = docker_run_cmdline(cmdline, image=docker_image, cwd=cwd, - docker_args=['--net=host']) + docker_args=['--net=host', + '--name', container_name]) cwd = None + test_job = jobset.JobSpec( cmdline=cmdline, cwd=cwd, @@ -368,25 +385,27 @@ def cloud_to_cloud_jobspec(language, test_case, server_name, server_host, test_case), timeout_seconds=2*60, flake_retries=5 if args.allow_flakes else 0, - timeout_retries=2 if args.allow_flakes else 0) + timeout_retries=2 if args.allow_flakes else 0, + kill_handler=_job_kill_handler) + test_job.container_name = container_name return test_job def server_jobspec(language, docker_image): """Create jobspec for running a server""" - cidfile = tempfile.mktemp() + container_name = dockerjob.random_name('interop_server_%s' % language) cmdline = bash_login_cmdline(language.server_args() + ['--port=%s' % _DEFAULT_SERVER_PORT]) docker_cmdline = docker_run_cmdline(cmdline, image=docker_image, cwd=language.server_cwd, docker_args=['-p', str(_DEFAULT_SERVER_PORT), - '--cidfile', cidfile]) + '--name', container_name]) server_job = jobset.JobSpec( cmdline=docker_cmdline, - shortname="interop_server:%s" % language, + shortname="interop_server_%s" % language, timeout_seconds=30*60) - server_job.cidfile = cidfile + server_job.container_name = container_name return server_job From 9b9812133c8310a07d60a2c7b2b0effb6dba24ad Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 8 Oct 2015 17:25:52 -0700 Subject: [PATCH 14/33] use --name instead of --cidfile --- tools/jenkins/build_interop_image.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tools/jenkins/build_interop_image.sh b/tools/jenkins/build_interop_image.sh index 3664eed84d9..166efbd9e2f 100755 --- a/tools/jenkins/build_interop_image.sh +++ b/tools/jenkins/build_interop_image.sh @@ -77,7 +77,7 @@ docker build -t $BASE_IMAGE --force-rm=true tools/jenkins/$BASE_NAME || exit $? # Create a local branch so the child Docker script won't complain git branch -f jenkins-docker -CIDFILE=`mktemp -u --suffix=.cid` +CONTAINER_NAME="build_${BASE_NAME}_$(uuidgen)" # Prepare image for interop tests, commit it on success. (docker run \ @@ -85,17 +85,14 @@ CIDFILE=`mktemp -u --suffix=.cid` -i $TTY_FLAG \ $MOUNT_ARGS \ -v /tmp/ccache:/tmp/ccache \ - --cidfile=$CIDFILE \ + --name=$CONTAINER_NAME \ $BASE_IMAGE \ bash -l /var/local/jenkins/grpc/tools/jenkins/$BASE_NAME/build_interop.sh \ - && docker commit `cat $CIDFILE` $INTEROP_IMAGE \ + && docker commit $CONTAINER_NAME $INTEROP_IMAGE \ && echo "Successfully built image $INTEROP_IMAGE") EXITCODE=$? # remove intermediate container, possibly killing it first -docker rm -f `cat $CIDFILE` - -# remove the cidfile -rm -rf `cat $CIDFILE` +docker rm -f $CONTAINER_NAME exit $EXITCODE From 65854ebbd26f13294cdcf30484adb8f5c3d3ba29 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 8 Oct 2015 17:35:44 -0700 Subject: [PATCH 15/33] stop using --cidfile for run_tests --- tools/jenkins/build_docker_and_run_tests.sh | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tools/jenkins/build_docker_and_run_tests.sh b/tools/jenkins/build_docker_and_run_tests.sh index 8b7809f2e23..6e3166ce579 100755 --- a/tools/jenkins/build_docker_and_run_tests.sh +++ b/tools/jenkins/build_docker_and_run_tests.sh @@ -53,8 +53,8 @@ DOCKER_IMAGE_NAME=grpc_jenkins_slave${docker_suffix}_`sha1sum tools/jenkins/grpc # Make sure docker image has been built. Should be instantaneous if so. docker build -t $DOCKER_IMAGE_NAME tools/jenkins/grpc_jenkins_slave$docker_suffix -# Make sure the CID file is gone. -rm -f docker.cid +# Choose random name for docker container +CONTAINER_NAME="run_tests_$(uuidgen)" # Run tests inside docker docker run \ @@ -70,23 +70,21 @@ docker run \ -v /var/run/docker.sock:/var/run/docker.sock \ -v $(which docker):/bin/docker \ -w /var/local/git/grpc \ - --cidfile=docker.cid \ + --name=$CONTAINER_NAME \ $DOCKER_IMAGE_NAME \ bash -l /var/local/jenkins/grpc/tools/jenkins/docker_run_tests.sh || DOCKER_FAILED="true" -DOCKER_CID=`cat docker.cid` - if [ "$XML_REPORT" != "" ] then - docker cp "$DOCKER_CID:/var/local/git/grpc/$XML_REPORT" $git_root + docker cp "$CONTAINER_NAME:/var/local/git/grpc/$XML_REPORT" $git_root fi -docker cp "$DOCKER_CID:/var/local/git/grpc/reports.zip" $git_root || true +docker cp "$CONTAINER_NAME:/var/local/git/grpc/reports.zip" $git_root || true unzip $git_root/reports.zip -d $git_root || true rm -f reports.zip # remove the container, possibly killing it first -docker rm -f $DOCKER_CID || true +docker rm -f $CONTAINER_NAME || true if [ "$DOCKER_FAILED" != "" ] && [ "$XML_REPORT" == "" ] then From c86909ce71b04a78a6cf7f662e1ce937d7f8ef3b Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 09:25:26 -0700 Subject: [PATCH 16/33] Fixed node interop build script and Dockerfile --- tools/jenkins/grpc_interop_node/Dockerfile | 1 + tools/jenkins/grpc_interop_node/build_interop.sh | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/jenkins/grpc_interop_node/Dockerfile b/tools/jenkins/grpc_interop_node/Dockerfile index 587227b9429..db5aff844de 100644 --- a/tools/jenkins/grpc_interop_node/Dockerfile +++ b/tools/jenkins/grpc_interop_node/Dockerfile @@ -48,6 +48,7 @@ RUN apt-get update && apt-get install -y \ libc6-dbg \ libc6-dev \ libgtest-dev \ + libssl-dev \ libtool \ make \ strace \ diff --git a/tools/jenkins/grpc_interop_node/build_interop.sh b/tools/jenkins/grpc_interop_node/build_interop.sh index 84e25e33088..3b69715c9af 100755 --- a/tools/jenkins/grpc_interop_node/build_interop.sh +++ b/tools/jenkins/grpc_interop_node/build_interop.sh @@ -45,5 +45,4 @@ make install-certs # build Node interop client & server npm install -g node-gyp -make install_c -C /var/local/git/grpc -(cd src/node && npm install && node-gyp rebuild) +(npm install && node-gyp rebuild) From 6374f9dc8368d3cbd65293e129a28a448adb75e9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 11:36:33 -0700 Subject: [PATCH 17/33] Fix a couple of issues with the node interop client This fixes how the node interop client handles authentication failure and how it checks the service account email responses. --- src/node/interop/interop_client.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js index 54a256f277f..6ee271f431b 100644 --- a/src/node/interop/interop_client.js +++ b/src/node/interop/interop_client.js @@ -44,6 +44,9 @@ var GoogleAuth = require('google-auth-library'); var assert = require('assert'); +var SERVICE_ACCOUNT_EMAIL = require( + process.env.GOOGLE_APPLICATION_CREDENTIALS).client_email; + var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial'; var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin'; @@ -410,21 +413,21 @@ function computeEngineCreds(client, done, extra) { } function serviceAccountCreds(client, done, extra) { - authTest(extra.default_service_account, extra.oauth_scope, client, done); + authTest(SERVICE_ACCOUNT_EMAIL, extra.oauth_scope, client, done); } function jwtTokenCreds(client, done, extra) { - authTest(extra.default_service_account, null, client, done); + authTest(SERVICE_ACCOUNT_EMAIL, null, client, done); } function oauth2Test(client, done, extra) { - var arg = { - fill_username: true, - fill_oauth_scope: true - }; + var arg = { + fill_username: true, + fill_oauth_scope: true + }; client.unaryCall(arg, function(err, resp) { assert.ifError(err); - assert.strictEqual(resp.username, extra.service_account); + assert.strictEqual(resp.username, SERVICE_ACCOUNT_EMAIL); assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1); if (done) { done(); @@ -446,7 +449,7 @@ function perRpcAuthTest(client, done, extra) { var creds = grpc.credentials.createFromGoogleCredential(credential); client.unaryCall(arg, function(err, resp) { assert.ifError(err); - assert.strictEqual(resp.username, extra.service_account); + assert.strictEqual(resp.username, SERVICE_ACCOUNT_EMAIL); assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1); if (done) { done(); @@ -571,11 +574,12 @@ function runTest(address, host_override, test_case, tls, test_ca, done, extra) { var execute = function(err, creds) { assert.ifError(err); var client = new test.Client(address, creds, options); - test.run(client, done); + test.run(client, done, extra); }; if (test.getCreds) { test.getCreds(extra.oauth_scope, function(err, new_creds) { + assert.ifError(err); execute(err, grpc.credentials.combineChannelCredentials( creds, new_creds)); }); @@ -593,8 +597,7 @@ if (require.main === module) { }); var extra_args = { service_account: argv.default_service_account, - oauth_scope: argv.oauth_scope, - service_account_key_file: argv.service_account_key_file + oauth_scope: argv.oauth_scope }; runTest(argv.server_host + ':' + argv.server_port, argv.server_host_override, argv.test_case, argv.use_tls === 'true', argv.use_test_ca === 'true', From e5ba29f337121befd876b2a9649fdb2cf7c7797c Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Fri, 9 Oct 2015 11:18:45 -0700 Subject: [PATCH 18/33] php: implement empty_stream interop test --- src/php/tests/interop/interop_client.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/php/tests/interop/interop_client.php b/src/php/tests/interop/interop_client.php index d55d5629b74..d782c101c23 100755 --- a/src/php/tests/interop/interop_client.php +++ b/src/php/tests/interop/interop_client.php @@ -250,6 +250,22 @@ function pingPong($stub) { 'Call did not complete successfully'); } +/** + * Run the empty_stream test. + * Passes when run against the Node server as of 2015-10-09 + * @param $stub Stub object that has service methods. + */ +function emptyStream($stub) { + // for the current PHP implementation, $call->read() will wait + // forever for a server response if the server is not sending any. + // so this test is imeplemented as a timeout to indicate the absence + // of receiving any response from the server + $call = $stub->FullDuplexCall(array('timeout' => 100000)); + hardAssert($call->read() === null, 'Server returned too many responses'); + hardAssert($call->getStatus()->code === Grpc\STATUS_DEADLINE_EXCEEDED, + 'Call did not complete successfully'); +} + /** * Run the cancel_after_begin test. * Passes when run against the Node server as of 2015-08-28 @@ -370,6 +386,9 @@ switch ($args['test_case']) { case 'ping_pong': pingPong($stub); break; + case 'empty_stream': + emptyStream($stub); + break; case 'cancel_after_begin': cancelAfterBegin($stub); break; From 66ec9bb0efd0d60d1eb676875c4ca1512bc952a7 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Fri, 9 Oct 2015 13:16:48 -0700 Subject: [PATCH 19/33] php: call writesDone in interop test --- src/php/tests/interop/interop_client.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/php/tests/interop/interop_client.php b/src/php/tests/interop/interop_client.php index d782c101c23..0590264ef80 100755 --- a/src/php/tests/interop/interop_client.php +++ b/src/php/tests/interop/interop_client.php @@ -261,8 +261,9 @@ function emptyStream($stub) { // so this test is imeplemented as a timeout to indicate the absence // of receiving any response from the server $call = $stub->FullDuplexCall(array('timeout' => 100000)); + $call->writesDone(); hardAssert($call->read() === null, 'Server returned too many responses'); - hardAssert($call->getStatus()->code === Grpc\STATUS_DEADLINE_EXCEEDED, + hardAssert($call->getStatus()->code === Grpc\STATUS_OK, 'Call did not complete successfully'); } From 32a145b73c6c0ba29780de6e6d95e0decf436acd Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 13:31:47 -0700 Subject: [PATCH 20/33] Moved grpc.gyp.template to binding.gyp.template --- templates/{grpc.gyp.template => binding.gyp.template} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename templates/{grpc.gyp.template => binding.gyp.template} (100%) diff --git a/templates/grpc.gyp.template b/templates/binding.gyp.template similarity index 100% rename from templates/grpc.gyp.template rename to templates/binding.gyp.template From fd994f11b86a7977e1e4656796da37aec950708d Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 14:02:28 -0700 Subject: [PATCH 21/33] Consolidated gyp files to fix Node installation issue --- binding.gyp | 232 +++++++++++++++++++++++++- grpc.gyp | 295 --------------------------------- package.json | 3 +- templates/binding.gyp.template | 112 ++++++++----- 4 files changed, 299 insertions(+), 343 deletions(-) delete mode 100644 grpc.gyp diff --git a/binding.gyp b/binding.gyp index 2a5cb1ced49..b775113f8fd 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,3 +1,10 @@ +# GRPC Node gyp file +# This currently builds the Node extension and dependencies +# This file has been automatically generated from a template file. +# Please look at the templates directory instead. +# This file can be regenerated from the template by running +# tools/buildgen/generate_projects.sh + # Copyright 2015, Google Inc. # All rights reserved. # @@ -26,11 +33,230 @@ # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +# Some of this file is built with the help of +# https://n8.io/converting-a-c-library-to-gyp/ { - "variables" : { + 'variables': { 'config': '0. + # io.js always reports versions >0 and always exports ALPN symbols. + 'defines': [ + 'TSI_OPENSSL_ALPN_SUPPORT=/dev/null 2>&1 && echo 1 || echo 0)' - ], - 'include_dirs': [ - '.', - 'include' - ], - # TODO: Check for libraries with pkg-config - 'libraries': [ - '-lcrypto', - '-lssl', - '-ldl', - '-lpthread', - '-lz' - ], - 'direct_dependent_settings': { - 'include_dirs': [ - '.', - 'include' - ], - } - }, - 'targets': [ - { - 'target_name': 'gpr', - 'product_prefix': 'lib', - 'type': 'static_library', - 'dependencies': [ - ], - 'sources': [ - 'src/core/support/alloc.c', - 'src/core/support/cmdline.c', - 'src/core/support/cpu_iphone.c', - 'src/core/support/cpu_linux.c', - 'src/core/support/cpu_posix.c', - 'src/core/support/cpu_windows.c', - 'src/core/support/env_linux.c', - 'src/core/support/env_posix.c', - 'src/core/support/env_win32.c', - 'src/core/support/file.c', - 'src/core/support/file_posix.c', - 'src/core/support/file_win32.c', - 'src/core/support/histogram.c', - 'src/core/support/host_port.c', - 'src/core/support/log.c', - 'src/core/support/log_android.c', - 'src/core/support/log_linux.c', - 'src/core/support/log_posix.c', - 'src/core/support/log_win32.c', - 'src/core/support/murmur_hash.c', - 'src/core/support/slice.c', - 'src/core/support/slice_buffer.c', - 'src/core/support/stack_lockfree.c', - 'src/core/support/string.c', - 'src/core/support/string_posix.c', - 'src/core/support/string_win32.c', - 'src/core/support/subprocess_posix.c', - 'src/core/support/sync.c', - 'src/core/support/sync_posix.c', - 'src/core/support/sync_win32.c', - 'src/core/support/thd.c', - 'src/core/support/thd_posix.c', - 'src/core/support/thd_win32.c', - 'src/core/support/time.c', - 'src/core/support/time_posix.c', - 'src/core/support/time_win32.c', - 'src/core/support/tls_pthread.c', - ], - }, - { - 'target_name': 'grpc', - 'product_prefix': 'lib', - 'type': 'static_library', - 'dependencies': [ - 'gpr', - ], - 'sources': [ - 'src/core/httpcli/httpcli_security_connector.c', - 'src/core/security/base64.c', - 'src/core/security/client_auth_filter.c', - 'src/core/security/credentials.c', - 'src/core/security/credentials_metadata.c', - 'src/core/security/credentials_posix.c', - 'src/core/security/credentials_win32.c', - 'src/core/security/google_default_credentials.c', - 'src/core/security/handshake.c', - 'src/core/security/json_token.c', - 'src/core/security/jwt_verifier.c', - 'src/core/security/secure_endpoint.c', - 'src/core/security/security_connector.c', - 'src/core/security/security_context.c', - 'src/core/security/server_auth_filter.c', - 'src/core/security/server_secure_chttp2.c', - 'src/core/surface/init_secure.c', - 'src/core/surface/secure_channel_create.c', - 'src/core/tsi/fake_transport_security.c', - 'src/core/tsi/ssl_transport_security.c', - 'src/core/tsi/transport_security.c', - 'src/core/census/grpc_context.c', - 'src/core/census/grpc_filter.c', - 'src/core/channel/channel_args.c', - 'src/core/channel/channel_stack.c', - 'src/core/channel/client_channel.c', - 'src/core/channel/compress_filter.c', - 'src/core/channel/connected_channel.c', - 'src/core/channel/http_client_filter.c', - 'src/core/channel/http_server_filter.c', - 'src/core/channel/noop_filter.c', - 'src/core/client_config/client_config.c', - 'src/core/client_config/connector.c', - 'src/core/client_config/lb_policies/pick_first.c', - 'src/core/client_config/lb_policies/round_robin.c', - 'src/core/client_config/lb_policy.c', - 'src/core/client_config/lb_policy_factory.c', - 'src/core/client_config/lb_policy_registry.c', - 'src/core/client_config/resolver.c', - 'src/core/client_config/resolver_factory.c', - 'src/core/client_config/resolver_registry.c', - 'src/core/client_config/resolvers/dns_resolver.c', - 'src/core/client_config/resolvers/sockaddr_resolver.c', - 'src/core/client_config/subchannel.c', - 'src/core/client_config/subchannel_factory.c', - 'src/core/client_config/subchannel_factory_decorators/add_channel_arg.c', - 'src/core/client_config/subchannel_factory_decorators/merge_channel_args.c', - 'src/core/client_config/uri_parser.c', - 'src/core/compression/algorithm.c', - 'src/core/compression/message_compress.c', - 'src/core/debug/trace.c', - 'src/core/httpcli/format_request.c', - 'src/core/httpcli/httpcli.c', - 'src/core/httpcli/parser.c', - 'src/core/iomgr/alarm.c', - 'src/core/iomgr/alarm_heap.c', - 'src/core/iomgr/closure.c', - 'src/core/iomgr/endpoint.c', - 'src/core/iomgr/endpoint_pair_posix.c', - 'src/core/iomgr/endpoint_pair_windows.c', - 'src/core/iomgr/exec_ctx.c', - 'src/core/iomgr/fd_posix.c', - 'src/core/iomgr/iocp_windows.c', - 'src/core/iomgr/iomgr.c', - 'src/core/iomgr/iomgr_posix.c', - 'src/core/iomgr/iomgr_windows.c', - 'src/core/iomgr/pollset_multipoller_with_epoll.c', - 'src/core/iomgr/pollset_multipoller_with_poll_posix.c', - 'src/core/iomgr/pollset_posix.c', - 'src/core/iomgr/pollset_set_posix.c', - 'src/core/iomgr/pollset_set_windows.c', - 'src/core/iomgr/pollset_windows.c', - 'src/core/iomgr/resolve_address_posix.c', - 'src/core/iomgr/resolve_address_windows.c', - 'src/core/iomgr/sockaddr_utils.c', - 'src/core/iomgr/socket_utils_common_posix.c', - 'src/core/iomgr/socket_utils_linux.c', - 'src/core/iomgr/socket_utils_posix.c', - 'src/core/iomgr/socket_windows.c', - 'src/core/iomgr/tcp_client_posix.c', - 'src/core/iomgr/tcp_client_windows.c', - 'src/core/iomgr/tcp_posix.c', - 'src/core/iomgr/tcp_server_posix.c', - 'src/core/iomgr/tcp_server_windows.c', - 'src/core/iomgr/tcp_windows.c', - 'src/core/iomgr/time_averaged_stats.c', - 'src/core/iomgr/udp_server.c', - 'src/core/iomgr/wakeup_fd_eventfd.c', - 'src/core/iomgr/wakeup_fd_nospecial.c', - 'src/core/iomgr/wakeup_fd_pipe.c', - 'src/core/iomgr/wakeup_fd_posix.c', - 'src/core/iomgr/workqueue_posix.c', - 'src/core/iomgr/workqueue_windows.c', - 'src/core/json/json.c', - 'src/core/json/json_reader.c', - 'src/core/json/json_string.c', - 'src/core/json/json_writer.c', - 'src/core/profiling/basic_timers.c', - 'src/core/profiling/stap_timers.c', - 'src/core/surface/api_trace.c', - 'src/core/surface/byte_buffer.c', - 'src/core/surface/byte_buffer_queue.c', - 'src/core/surface/byte_buffer_reader.c', - 'src/core/surface/call.c', - 'src/core/surface/call_details.c', - 'src/core/surface/call_log_batch.c', - 'src/core/surface/channel.c', - 'src/core/surface/channel_connectivity.c', - 'src/core/surface/channel_create.c', - 'src/core/surface/completion_queue.c', - 'src/core/surface/event_string.c', - 'src/core/surface/init.c', - 'src/core/surface/lame_client.c', - 'src/core/surface/metadata_array.c', - 'src/core/surface/server.c', - 'src/core/surface/server_chttp2.c', - 'src/core/surface/server_create.c', - 'src/core/surface/version.c', - 'src/core/transport/chttp2/alpn.c', - 'src/core/transport/chttp2/bin_encoder.c', - 'src/core/transport/chttp2/frame_data.c', - 'src/core/transport/chttp2/frame_goaway.c', - 'src/core/transport/chttp2/frame_ping.c', - 'src/core/transport/chttp2/frame_rst_stream.c', - 'src/core/transport/chttp2/frame_settings.c', - 'src/core/transport/chttp2/frame_window_update.c', - 'src/core/transport/chttp2/hpack_parser.c', - 'src/core/transport/chttp2/hpack_table.c', - 'src/core/transport/chttp2/huffsyms.c', - 'src/core/transport/chttp2/incoming_metadata.c', - 'src/core/transport/chttp2/parsing.c', - 'src/core/transport/chttp2/status_conversion.c', - 'src/core/transport/chttp2/stream_encoder.c', - 'src/core/transport/chttp2/stream_lists.c', - 'src/core/transport/chttp2/stream_map.c', - 'src/core/transport/chttp2/timeout_encoding.c', - 'src/core/transport/chttp2/varint.c', - 'src/core/transport/chttp2/writing.c', - 'src/core/transport/chttp2_transport.c', - 'src/core/transport/connectivity_state.c', - 'src/core/transport/metadata.c', - 'src/core/transport/stream_op.c', - 'src/core/transport/transport.c', - 'src/core/transport/transport_op_string.c', - 'src/core/census/context.c', - 'src/core/census/initialize.c', - 'src/core/census/operation.c', - 'src/core/census/tracing.c', - ], - }, - ] -} diff --git a/package.json b/package.json index 0eea3475a34..c624c45107e 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "lib": "src/node/src" }, "scripts": { - "lint": "node ./node_modules/jshint/bin/jshint src/node/src src/node/test src/node/examples src/node/interop src/node/index.js", + "lint": "node ./node_modules/jshint/bin/jshint src/node/src src/node/test src/node/interop src/node/index.js", "test": "./node_modules/.bin/mocha src/node/test && npm run-script lint", "gen_docs": "./node_modules/.bin/jsdoc -c src/node/jsdoc_conf.json", "coverage": "./node_modules/.bin/istanbul cover ./node_modules/.bin/_mocha src/node/test" @@ -53,7 +53,6 @@ "src/core", "test/proto", "include", - "grpc.gyp", "binding.gyp" ], "main": "src/node/index.js", diff --git a/templates/binding.gyp.template b/templates/binding.gyp.template index 9a7637d8bf4..1e5b2ea9113 100644 --- a/templates/binding.gyp.template +++ b/templates/binding.gyp.template @@ -1,7 +1,7 @@ %YAML 1.2 --- | - # GRPC gyp file - # This currently builds C code. + # GRPC Node gyp file + # This currently builds the Node extension and dependencies # This file has been automatically generated from a template file. # Please look at the templates directory instead. # This file can be regenerated from the template by running @@ -39,54 +39,20 @@ # Some of this file is built with the help of # https://n8.io/converting-a-c-library-to-gyp/ { + 'variables': { + 'config': '0. + # io.js always reports versions >0 and always exports ALPN symbols. 'defines': [ - 'TSI_OPENSSL_ALPN_SUPPORT=/dev/null 2>&1 && echo 1 || echo 0)' + 'TSI_OPENSSL_ALPN_SUPPORT= Date: Fri, 9 Oct 2015 14:02:43 -0700 Subject: [PATCH 22/33] Fixed some issues with the Node tests --- src/node/src/credentials.js | 2 +- src/node/test/async_test.js | 2 +- src/node/test/channel_test.js | 2 +- src/node/test/credentials_test.js | 10 +++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/node/src/credentials.js b/src/node/src/credentials.js index 009ff633067..ddc094f8968 100644 --- a/src/node/src/credentials.js +++ b/src/node/src/credentials.js @@ -153,7 +153,7 @@ exports.combineCallCredentials = function() { current = current.compose(arguments[i]); } return current; -} +}; /** * Create an insecure credentials object. This is used to create a channel that diff --git a/src/node/test/async_test.js b/src/node/test/async_test.js index ce3ce50a2d6..6d71ea24f54 100644 --- a/src/node/test/async_test.js +++ b/src/node/test/async_test.js @@ -57,7 +57,7 @@ describe('Async functionality', function() { grpc.ServerCredentials.createInsecure()); server.start(); math_client = new math.Math('localhost:' + port_num, - grpc.Credentials.createInsecure()); + grpc.credentials.createInsecure()); done(); }); after(function() { diff --git a/src/node/test/channel_test.js b/src/node/test/channel_test.js index b86f89b8a85..05269f7b6e4 100644 --- a/src/node/test/channel_test.js +++ b/src/node/test/channel_test.js @@ -149,7 +149,7 @@ describe('channel', function() { afterEach(function() { channel.close(); }); - it.only('should time out if called alone', function(done) { + it('should time out if called alone', function(done) { var old_state = channel.getConnectivityState(); var deadline = new Date(); deadline.setSeconds(deadline.getSeconds() + 1); diff --git a/src/node/test/credentials_test.js b/src/node/test/credentials_test.js index 8eb91ee69e6..7fc311a888d 100644 --- a/src/node/test/credentials_test.js +++ b/src/node/test/credentials_test.js @@ -130,8 +130,8 @@ describe('client credentials', function() { callback(null, metadata); }; var creds = grpc.credentials.createFromMetadataGenerator(metadataUpdater); - var combined_creds = grpc.credentials.combineCredentials(client_ssl_creds, - creds); + var combined_creds = grpc.credentials.combineChannelCredentials( + client_ssl_creds, creds); var client = new Client('localhost:' + port, combined_creds, client_options); var call = client.unary({}, function(err, data) { @@ -150,8 +150,8 @@ describe('client credentials', function() { callback(null, metadata); }; var creds = grpc.credentials.createFromMetadataGenerator(metadataUpdater); - var combined_creds = grpc.credentials.combineCredentials(client_ssl_creds, - creds); + var combined_creds = grpc.credentials.combineChannelCredentials( + client_ssl_creds, creds); var client = new Client('localhost:' + port, combined_creds, client_options); var call = client.unary({}, function(err, data) { @@ -231,7 +231,7 @@ describe('client credentials', function() { updater_creds, alt_updater_creds); var call = client.unary({}, function(err, data) { assert.ifError(err); - }, null, {credentials: updater_creds}); + }, null, {credentials: combined_updater}); call.on('metadata', function(metadata) { assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); assert.deepEqual(metadata.get('other_plugin_key'), From 15def98e344bfb220cb03f5cccc7c793f2ecd811 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 14:11:02 -0700 Subject: [PATCH 23/33] Expanded comment in binding.gyp --- binding.gyp | 4 ++++ templates/binding.gyp.template | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/binding.gyp b/binding.gyp index b775113f8fd..ed7fc757c93 100644 --- a/binding.gyp +++ b/binding.gyp @@ -44,6 +44,10 @@ 'target_defaults': { # Emperically, Node only exports ALPN symbols if its major version is >0. # io.js always reports versions >0 and always exports ALPN symbols. + # Therefore, Node's major version will be truthy if and only if it + # supports ALPN. The output of "node -v" is v[major].[minor].[patch], + # like "v4.1.1" in a recent version. We use grep to extract just the + # major version. "4", would be the output for the example. 'defines': [ 'TSI_OPENSSL_ALPN_SUPPORT=0. # io.js always reports versions >0 and always exports ALPN symbols. + # Therefore, Node's major version will be truthy if and only if it + # supports ALPN. The output of "node -v" is v[major].[minor].[patch], + # like "v4.1.1" in a recent version. We use grep to extract just the + # major version. "4", would be the output for the example. 'defines': [ 'TSI_OPENSSL_ALPN_SUPPORT= Date: Fri, 9 Oct 2015 02:32:25 +0200 Subject: [PATCH 24/33] Ensuring docker dependencies present in the container. --- tools/jenkins/grpc_jenkins_slave/Dockerfile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/jenkins/grpc_jenkins_slave/Dockerfile b/tools/jenkins/grpc_jenkins_slave/Dockerfile index 5f2b425c8c2..129a8db24fa 100644 --- a/tools/jenkins/grpc_jenkins_slave/Dockerfile +++ b/tools/jenkins/grpc_jenkins_slave/Dockerfile @@ -157,6 +157,12 @@ RUN apt-get update && apt-get install -y \ RUN apt-get install -y libzookeeper-mt-dev +################## +# Docker "inception". +# Note this is quite the ugly hack. +# This makes sure that the docker binary we inject has its dependencies. +RUN curl https://get.docker.com/ | sh +RUN apt-get remove --purge -y docker-engine RUN mkdir /var/local/jenkins From 98c0be52284bb6b4a8a4a3c32e5157a2d1f7776d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Oct 2015 14:33:34 -0700 Subject: [PATCH 25/33] add retries for docker port command --- tools/run_tests/dockerjob.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/dockerjob.py b/tools/run_tests/dockerjob.py index 266edd4375b..0d4a1fc117e 100755 --- a/tools/run_tests/dockerjob.py +++ b/tools/run_tests/dockerjob.py @@ -49,10 +49,19 @@ def docker_kill(cid): return subprocess.call(['docker','kill', str(cid)]) == 0 -def docker_mapped_port(cid, port): +def docker_mapped_port(cid, port, timeout_seconds=15): """Get port mapped to internal given internal port for given container.""" - output = subprocess.check_output('docker port %s %s' % (cid, port), shell=True) - return int(output.split(':', 2)[1]) + started = time.time() + while time.time() - started < timeout_seconds: + try: + output = subprocess.check_output('docker port %s %s' % (cid, port), + stderr=_DEVNULL + shell=True) + return int(output.split(':', 2)[1]) + except subprocess.CalledProcessError as e: + pass + raise Exception('Failed to get exposed port %s for container %s.' % + (port, cid)) def finish_jobs(jobs): @@ -68,7 +77,7 @@ def image_exists(image): """Returns True if given docker image exists.""" return subprocess.call(['docker','inspect', image], stdout=_DEVNULL, - stderr=_DEVNULL) == 0 + stderr=subprocess.STDOUT) == 0 def remove_image(image, skip_nonexistent=False, max_retries=10): From 0a14f62e178f8557ed84b689ca6076b3c0372747 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Oct 2015 14:34:29 -0700 Subject: [PATCH 26/33] dont use + character in generated container names --- tools/run_tests/run_interop_tests.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index 4d09ae7fcda..dea33f7ab20 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -71,6 +71,7 @@ class CXXLanguage: self.client_cmdline_base = ['bins/opt/interop_client'] self.client_cwd = None self.server_cwd = None + self.safename = 'cxx' def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -96,6 +97,7 @@ class CSharpLanguage: self.client_cmdline_base = ['mono', 'Grpc.IntegrationTesting.Client.exe'] self.client_cwd = 'src/csharp/Grpc.IntegrationTesting.Client/bin/Debug' self.server_cwd = 'src/csharp/Grpc.IntegrationTesting.Server/bin/Debug' + self.safename = str(self) def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -121,6 +123,7 @@ class JavaLanguage: self.client_cmdline_base = ['./run-test-client.sh'] self.client_cwd = '../grpc-java' self.server_cwd = '../grpc-java' + self.safename = str(self) def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -147,6 +150,7 @@ class GoLanguage: # TODO: this relies on running inside docker self.client_cwd = '/go/src/google.golang.org/grpc/interop/client' self.server_cwd = '/go/src/google.golang.org/grpc/interop/server' + self.safename = str(self) def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -172,6 +176,7 @@ class NodeLanguage: self.client_cmdline_base = ['node', 'src/node/interop/interop_client.js'] self.client_cwd = None self.server_cwd = None + self.safename = str(self) def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -196,6 +201,7 @@ class PHPLanguage: def __init__(self): self.client_cmdline_base = ['src/php/bin/interop_client.sh'] self.client_cwd = None + self.safename = str(self) def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -218,6 +224,7 @@ class RubyLanguage: self.client_cmdline_base = ['ruby', 'src/ruby/bin/interop/interop_client.rb'] self.client_cwd = None self.server_cwd = None + self.safename = str(self) def cloud_to_prod_args(self): return (self.client_cmdline_base + _CLOUD_TO_PROD_BASE_ARGS + @@ -337,7 +344,7 @@ def cloud_to_prod_jobspec(language, test_case, docker_image=None, auth=False): cmdline = bash_login_cmdline(cmdline) if docker_image: - container_name = dockerjob.random_name('interop_client_%s' % language) + container_name = dockerjob.random_name('interop_client_%s' % language.safename) cmdline = docker_run_cmdline(cmdline, image=docker_image, cwd=cwd, @@ -370,7 +377,7 @@ def cloud_to_cloud_jobspec(language, test_case, server_name, server_host, '--server_port=%s' % server_port ]) cwd = language.client_cwd if docker_image: - container_name = dockerjob.random_name('interop_client_%s' % language) + container_name = dockerjob.random_name('interop_client_%s' % language.safename) cmdline = docker_run_cmdline(cmdline, image=docker_image, cwd=cwd, @@ -393,7 +400,7 @@ def cloud_to_cloud_jobspec(language, test_case, server_name, server_host, def server_jobspec(language, docker_image): """Create jobspec for running a server""" - container_name = dockerjob.random_name('interop_server_%s' % language) + container_name = dockerjob.random_name('interop_server_%s' % language.safename) cmdline = bash_login_cmdline(language.server_args() + ['--port=%s' % _DEFAULT_SERVER_PORT]) docker_cmdline = docker_run_cmdline(cmdline, @@ -411,10 +418,10 @@ def server_jobspec(language, docker_image): def build_interop_image_jobspec(language, tag=None): """Creates jobspec for building interop docker image for a language""" - safelang = str(language).replace("+", "x") if not tag: - tag = 'grpc_interop_%s:%s' % (safelang, uuid.uuid4()) - env = {'INTEROP_IMAGE': tag, 'BASE_NAME': 'grpc_interop_%s' % safelang} + tag = 'grpc_interop_%s:%s' % (language.safename, uuid.uuid4()) + env = {'INTEROP_IMAGE': tag, + 'BASE_NAME': 'grpc_interop_%s' % language.safename} if not args.travis: env['TTY_FLAG'] = '-t' build_job = jobset.JobSpec( From 1f1c5c541ad9b6c0d17085d3b18639563a5fb250 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Oct 2015 14:36:28 -0700 Subject: [PATCH 27/33] run empty_stream interop test --- tools/run_tests/run_interop_tests.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index dea33f7ab20..6daa967bba4 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -258,11 +258,9 @@ _LANGUAGES = { # languages supported as cloud_to_cloud servers _SERVERS = ['c++', 'node', 'csharp', 'java', 'go', 'ruby'] -# TODO(jtattermusch): add empty_stream once PHP starts supporting it. # TODO(jtattermusch): add timeout_on_sleeping_server once java starts supporting it. -# TODO(jtattermusch): add support for auth tests. _TEST_CASES = ['large_unary', 'empty_unary', 'ping_pong', - 'client_streaming', 'server_streaming', + 'empty_stream', 'client_streaming', 'server_streaming', 'cancel_after_begin', 'cancel_after_first_response'] _AUTH_TEST_CASES = ['compute_engine_creds', 'jwt_token_creds', From be13d818240bd2f37737d8717d4f77a417f44074 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 14:45:30 -0700 Subject: [PATCH 28/33] Fixed some issues with the tests --- src/node/interop/interop_client.js | 16 +++++++++++----- src/node/test/async_test.js | 2 +- src/node/test/channel_test.js | 2 +- src/node/test/credentials_test.js | 10 +++++----- src/node/test/interop_sanity_test.js | 2 +- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js index 6ee271f431b..cb55083d1ae 100644 --- a/src/node/interop/interop_client.js +++ b/src/node/interop/interop_client.js @@ -44,8 +44,14 @@ var GoogleAuth = require('google-auth-library'); var assert = require('assert'); -var SERVICE_ACCOUNT_EMAIL = require( - process.env.GOOGLE_APPLICATION_CREDENTIALS).client_email; +var SERVICE_ACCOUNT_EMAIL; +try { + SERVICE_ACCOUNT_EMAIL = require( + process.env.GOOGLE_APPLICATION_CREDENTIALS).client_email; +} catch (e) { + // This will cause the tests to fail if they need that string + SERVICE_ACCOUNT_EMAIL = null; +} var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial'; var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin'; @@ -346,20 +352,20 @@ function statusCodeAndMessage(client, done) { var arg = { response_status: { code: 2, - message: "test status message" + message: 'test status message' } }; client.unaryCall(arg, function(err, resp) { assert(err); assert.strictEqual(err.code, 2); - assert.strictEqual(err.message, "test status message"); + assert.strictEqual(err.message, 'test status message'); done(); }); var duplex = client.fullDuplexCall(); duplex.on('status', function(status) { assert(status); assert.strictEqual(status.code, 2); - assert.strictEqual(status.details, "test status message"); + assert.strictEqual(status.details, 'test status message'); done(); }); duplex.on('error', function(){}); diff --git a/src/node/test/async_test.js b/src/node/test/async_test.js index ce3ce50a2d6..6d71ea24f54 100644 --- a/src/node/test/async_test.js +++ b/src/node/test/async_test.js @@ -57,7 +57,7 @@ describe('Async functionality', function() { grpc.ServerCredentials.createInsecure()); server.start(); math_client = new math.Math('localhost:' + port_num, - grpc.Credentials.createInsecure()); + grpc.credentials.createInsecure()); done(); }); after(function() { diff --git a/src/node/test/channel_test.js b/src/node/test/channel_test.js index b86f89b8a85..05269f7b6e4 100644 --- a/src/node/test/channel_test.js +++ b/src/node/test/channel_test.js @@ -149,7 +149,7 @@ describe('channel', function() { afterEach(function() { channel.close(); }); - it.only('should time out if called alone', function(done) { + it('should time out if called alone', function(done) { var old_state = channel.getConnectivityState(); var deadline = new Date(); deadline.setSeconds(deadline.getSeconds() + 1); diff --git a/src/node/test/credentials_test.js b/src/node/test/credentials_test.js index 8eb91ee69e6..7fc311a888d 100644 --- a/src/node/test/credentials_test.js +++ b/src/node/test/credentials_test.js @@ -130,8 +130,8 @@ describe('client credentials', function() { callback(null, metadata); }; var creds = grpc.credentials.createFromMetadataGenerator(metadataUpdater); - var combined_creds = grpc.credentials.combineCredentials(client_ssl_creds, - creds); + var combined_creds = grpc.credentials.combineChannelCredentials( + client_ssl_creds, creds); var client = new Client('localhost:' + port, combined_creds, client_options); var call = client.unary({}, function(err, data) { @@ -150,8 +150,8 @@ describe('client credentials', function() { callback(null, metadata); }; var creds = grpc.credentials.createFromMetadataGenerator(metadataUpdater); - var combined_creds = grpc.credentials.combineCredentials(client_ssl_creds, - creds); + var combined_creds = grpc.credentials.combineChannelCredentials( + client_ssl_creds, creds); var client = new Client('localhost:' + port, combined_creds, client_options); var call = client.unary({}, function(err, data) { @@ -231,7 +231,7 @@ describe('client credentials', function() { updater_creds, alt_updater_creds); var call = client.unary({}, function(err, data) { assert.ifError(err); - }, null, {credentials: updater_creds}); + }, null, {credentials: combined_updater}); call.on('metadata', function(metadata) { assert.deepEqual(metadata.get('plugin_key'), ['plugin_value']); assert.deepEqual(metadata.get('other_plugin_key'), diff --git a/src/node/test/interop_sanity_test.js b/src/node/test/interop_sanity_test.js index f008a87585c..f8c0b141377 100644 --- a/src/node/test/interop_sanity_test.js +++ b/src/node/test/interop_sanity_test.js @@ -71,7 +71,7 @@ describe('Interop tests', function() { interop_client.runTest(port, name_override, 'server_streaming', true, true, done); }); - it('should pass ping_pong', function(done) { + it.only('should pass ping_pong', function(done) { interop_client.runTest(port, name_override, 'ping_pong', true, true, done); }); it('should pass empty_stream', function(done) { From 5dbbd9178db5a26b9a765534a561cf1e0cba5e94 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 14:45:55 -0700 Subject: [PATCH 29/33] Revert "Added interval_us delay in Node interop server" This reverts commit 83815eab40568e142f05376dae48c2cef41bfefd. --- src/node/interop/interop_server.js | 47 ++++-------------------------- 1 file changed, 6 insertions(+), 41 deletions(-) diff --git a/src/node/interop/interop_server.js b/src/node/interop/interop_server.js index cc527364b4a..5321005c863 100644 --- a/src/node/interop/interop_server.js +++ b/src/node/interop/interop_server.js @@ -35,7 +35,6 @@ var fs = require('fs'); var path = require('path'); -var async = require('async'); var _ = require('lodash'); var grpc = require('..'); var testProto = grpc.load({ @@ -87,22 +86,6 @@ function getEchoTrailer(call) { return response_trailer; } -/** - * @typedef Payload - * @type {object} - * @property {string} payload_type The payload type - * @property {Buffer} body The payload body - */ - -/** - * Get a payload of the specified type and size. If the requested payload is - * COMPRESSABLE, it returns a zero buffer. If the type is UNCOMRESSABLE, it - * returns a slice of pre-loaded uncompressable data. If the type is RANDOM, - * it returns one of the other choices, chosen at random. - * @param {string} payload_type The type of payload to return - * @param {Number} size The size of the payload body - * @return {Payload} The requested payload - */ function getPayload(payload_type, size) { if (payload_type === 'RANDOM') { payload_type = ['COMPRESSABLE', @@ -116,15 +99,6 @@ function getPayload(payload_type, size) { return {type: payload_type, body: body}; } -function respondWithStream(call, request, callback) { - async.eachSeries(request.response_parameters, function(resp_param, callback) { - setTimeout(function() { - call.write({payload: getPayload(request.response_type, resp_param.size)}); - callback(); - }, resp_param.interval_us/1000); - }, callback); -} - /** * Respond to an empty parameter with an empty response. * NOTE: this currently does not work due to issue #137 @@ -188,13 +162,10 @@ function handleStreamingOutput(call) { call.emit('error', status); return; } - respondWithStream(call, req, function(err) { - if (err) { - call.emit(err); - } else { - call.end(getEchoTrailer(call)); - } + _.each(req.response_parameters, function(resp_param) { + call.write({payload: getPayload(req.response_type, resp_param.size)}); }); + call.end(getEchoTrailer(call)); } /** @@ -204,7 +175,6 @@ function handleStreamingOutput(call) { */ function handleFullDuplex(call) { echoHeader(call); - var call_ended; call.on('data', function(value) { if (value.response_status) { var status = value.response_status; @@ -212,17 +182,12 @@ function handleFullDuplex(call) { call.emit('error', status); return; } - call.pause(); - respondWithStream(call, value, function(err) { - call.resume(); - if (call_ended) { - call.end(getEchoTrailer(call)); - } + _.each(value.response_parameters, function(resp_param) { + call.write({payload: getPayload(value.response_type, resp_param.size)}); }); }); call.on('end', function() { - call_ended = true; - + call.end(getEchoTrailer(call)); }); } From ab5bc721a099df03181fa6dc4ab06bff86a2d9d9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Oct 2015 14:46:29 -0700 Subject: [PATCH 30/33] prevent polluting output by subprocesses --- tools/run_tests/dockerjob.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/dockerjob.py b/tools/run_tests/dockerjob.py index 0d4a1fc117e..6427b8e0220 100755 --- a/tools/run_tests/dockerjob.py +++ b/tools/run_tests/dockerjob.py @@ -55,7 +55,7 @@ def docker_mapped_port(cid, port, timeout_seconds=15): while time.time() - started < timeout_seconds: try: output = subprocess.check_output('docker port %s %s' % (cid, port), - stderr=_DEVNULL + stderr=_DEVNULL, shell=True) return int(output.split(':', 2)[1]) except subprocess.CalledProcessError as e: @@ -85,7 +85,9 @@ def remove_image(image, skip_nonexistent=False, max_retries=10): if skip_nonexistent and not image_exists(image): return True for attempt in range(0, max_retries): - if subprocess.call(['docker','rmi', '-f', image]) == 0: + if subprocess.call(['docker','rmi', '-f', image], + stdout=_DEVNULL, + stderr=subprocess.STDOUT) == 0: return True time.sleep(2) print 'Failed to remove docker image %s' % image From cc6a2b89fc88ec31972d0fb52f5d215d10747773 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Oct 2015 14:53:51 -0700 Subject: [PATCH 31/33] eat output from docker kill as well --- tools/run_tests/dockerjob.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/dockerjob.py b/tools/run_tests/dockerjob.py index 6427b8e0220..1d67fe3033e 100755 --- a/tools/run_tests/dockerjob.py +++ b/tools/run_tests/dockerjob.py @@ -46,7 +46,9 @@ def random_name(base_name): def docker_kill(cid): """Kills a docker container. Returns True if successful.""" - return subprocess.call(['docker','kill', str(cid)]) == 0 + return subprocess.call(['docker','kill', str(cid)], + stdout=_DEVNULL, + stderr=subprocess.STDOUT) == 0 def docker_mapped_port(cid, port, timeout_seconds=15): From 0ca8c06b19a3074b69ef311fdfb607365ad7206d Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 9 Oct 2015 23:46:34 +0200 Subject: [PATCH 32/33] Adding basic redirect for the node coverage report html. --- tools/run_tests/run_node.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/run_tests/run_node.sh b/tools/run_tests/run_node.sh index 780969089da..0a11e87c37a 100755 --- a/tools/run_tests/run_node.sh +++ b/tools/run_tests/run_node.sh @@ -46,6 +46,8 @@ then lcov --base-directory . --directory . -c -o coverage.info genhtml -o ../reports/node_ext_coverage --num-spaces 2 \ -t 'Node gRPC test coverage' coverage.info + echo '' > \ + ../reports/node_coverage/index.html else ./node_modules/mocha/bin/mocha --timeout 8000 src/node/test fi From e754079d0d0ada139fa81f1b5ef9a1c31e785e23 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 9 Oct 2015 15:43:14 -0700 Subject: [PATCH 33/33] Fixed spelling error --- binding.gyp | 2 +- templates/binding.gyp.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/binding.gyp b/binding.gyp index ed7fc757c93..392835c7721 100644 --- a/binding.gyp +++ b/binding.gyp @@ -42,7 +42,7 @@ }, # TODO: Finish windows support 'target_defaults': { - # Emperically, Node only exports ALPN symbols if its major version is >0. + # Empirically, Node only exports ALPN symbols if its major version is >0. # io.js always reports versions >0 and always exports ALPN symbols. # Therefore, Node's major version will be truthy if and only if it # supports ALPN. The output of "node -v" is v[major].[minor].[patch], diff --git a/templates/binding.gyp.template b/templates/binding.gyp.template index 5fcd247442c..50d0823d1d4 100644 --- a/templates/binding.gyp.template +++ b/templates/binding.gyp.template @@ -44,7 +44,7 @@ }, # TODO: Finish windows support 'target_defaults': { - # Emperically, Node only exports ALPN symbols if its major version is >0. + # Empirically, Node only exports ALPN symbols if its major version is >0. # io.js always reports versions >0 and always exports ALPN symbols. # Therefore, Node's major version will be truthy if and only if it # supports ALPN. The output of "node -v" is v[major].[minor].[patch],