It looks like downstream_rq_completed is incremented in Mac even in the case connection is terminated on the Curl side due to the timeout. This doesn't happen on Linux.

Unfortunately I don't have access to a Mac machine to be able to debug this in detail.

@dio any suggestions how to proceed with this? Do we have a process when developers do not have access to a specific platform?

Interesting. Unfortunately, we only have this CI. I'll try to take a look at this on my local. However, seems like the test is successful now?

Yes, because I removed the following check that was succeeding on Linux but failing on Mac:

ASSERT_NE(nullptr, test_server_->counter("http.metadata_test.downstream_rq_completed")); EXPECT_EQ(0, test_server_->counter("http.metadata_test.downstream_rq_completed")->value());

For, to me unknown, reason downstream_rq_completed was incremented on Mac for each request Curl aborted due to the timeout. On Linux this counter was not incremented.

Let's have a comment regarding this for now.

Why use curl here at all and have it be blocking? Why can't you use the built in Envoy HTTP async client facilities?

We initially implemented this using the Envoy Http client in #5546. Although I do not know what was the original motivation, there was a suggestion on that PR to use libcurl.

@htuch and @lavignes can comment more on this.

Yeah. Don't have much more to add. When I originally implemented this, I ran into the issue that I ultimately needed to block signing the xDS request on the http request to get credentials. Note that the blocking is done on the gRPC client thread in my implementation. Not the main thread. It was after asking about methods for making blocking http requests it was suggested I make the blocking http client in #5546. But I don't recall in what issue it was suggested.

Then later in #5546 just using curl was proposed since it was being merged in as a dependency anyway. I felt more comfortable doing that since I wasn't very confident in my http client implementation. It was non-trivial for me to implement and test especially since I was very new to the codebase.

OK that's fine. Ultimately I don't think curl should be used here for a variety of reasons (blocking, more third party code to deal with, not the normal Envoy logs/metrics/traces, etc.) but we can merge and iterate.

I think I pointed out that libcurl was now available, but I can't remember if this was something we decided as the preferred approach. The main concern would be that we don't block on a worker thread either in the Envoy-based HTTP client or libcurl.

Why is this test tagged as exclusive? This acts as a serialization step and forces the total test run to increase by > 30s, as it can't be parallelized.

@htuch I'm not sure why this is set this way but @ivitjuk is no longer working on this so I'm taking this back over from here.

I think this is safe to remove. I can open a PR.

Integration test is creating an instance of the Envoy which is acting as a metadata endpoint. If we remove exclusive, wouldn't we risk port collision with other integration tests that create listeners?

Unless you are binding to fixed ports (which I didn't spot, we typically don't do this for the reason you state), this is safe to not be exclusive. The tests tend to bind to port zero, which allows safe concurrent execution.

Thanks, @lavignes can you make a PR to remove exclusive?

@ivitjuk @htuch done! #7545

@lavignes thanks, appreciated!