These two notify functions are very similar. There appear to be just
enough differences that trying to parameterize the differences may not
improve things.
For now, reduce some of the cosmetic differences so that the material
differences are more obvious.
Use named returned so that the caller has a better idea of what these
bools mean.
Return early to reduce the scope, and make it more obvious what values
are returned in which cases. Also reduces the number of conditional
expressions in each case.
The envoy version was updated after the PR which added this test was opened, and
merged before the test was merged, so it ended up with the wrong version.
The test had two racy bugs related to memdb references.
The first was when we initially populated data and retained the FederationState objects in a slice. Due to how the `inmemCodec` works these were actually the identical objects passed into memdb.
The second was that the `checkSame` assertion function was reading from memdb and setting the RaftIndexes to zeros to aid in equality checks. This was mutating the contents of memdb which is a no-no.
With this fix, the command:
```
i=0; while /usr/local/bin/go test -count=1 -timeout 30s github.com/hashicorp/consul/agent/consul -run '^(TestReplication_FederationStates)$'; do i=$((i + 1)); printf "$i "; done
```
That used to break on my machine in less than 20 runs is now running 150+ times without any issue.
Might also fix#7575
On recent Mac OS versions, the ulimit defaults to 256 by default, but many
systems (eg: some Linux distributions) often limit this value to 1024.
On validation of configuration, Consul now validates that the number of
allowed files descriptors is bigger than http_max_conns_per_client.
This make some unit tests failing on Mac OS.
Use a less important value in unit test, so tests runs well by default
on Mac OS without need for tuning the OS.
The api client should never rever to HTTP if the user explicitly
requested TLS. This change broke some tests because the tests always use
an non-TLS http server, but some tests explicitly enable TLS.
The new grpcAddress function contains all of the logic to translate the
command line options into the values used in the template.
The new type has two advantages.
1. It introduces a logical grouping of values in the BootstrapTplArgs
struct which is exceptionally large. This grouping makes the struct
easier to understand because each set of nested values can be seen
as a single entity.
2. It gives us a reasonable return value for this new function.
* Enable filtering language support for the v1/connect/intentions listing API
* Update website for filtering of Intentions
* Update website/source/api/connect/intentions.html.md
* Change style to match "join" singular
- Replaced "(Consul) cluster" with "Consul Datacenter"
- Removed "ing" so the feature fits "Consul Auto-join", and that the tense is correct.
Co-authored-by: danielehc <40759828+danielehc@users.noreply.github.com>
This change moves all the typeEntry lookups to the first step in the exported methods,
and makes unexporter internals accept the typeEntry struct.
This change is primarily intended to make it easier to extract the container of caches
from the Cache type.
It may incidentally reduce locking in fetch, but that was not a goal.
* [docs] Built-in Proxies not meant for production
* Adding link to Envoy for Connect
* Update website/source/docs/connect/proxies/built-in.md
Co-Authored-By: Blake Covarrubias <blake@covarrubi.as>
* Revising note
* Update website/source/docs/connect/proxies/built-in.md
period
Co-Authored-By: Hans Hasselberg <me@hans.io>
Co-authored-by: Blake Covarrubias <blake@covarrubi.as>
Co-authored-by: Hans Hasselberg <me@hans.io>
Sometimes, in the CI, it could receive a SIGURG, producing this line:
FAIL: TestForwardSignals/signal-interrupt (0.06s)
util_test.go:286: expected to read line "signal: interrupt" but got "signal: urgent I/O condition"
Only forward the signals we test to avoid this kind of false positive
Example of such unstable errors in CI:
https://circleci.com/gh/hashicorp/consul/153571
Exposing checks is supposed to allow a Consul agent bound to a different
IP address (e.g., in a different Kubernetes pod) to access healthchecks
through the proxy while the underlying service binds to localhost. This
is an important security feature that makes sure no external traffic
reaches the service except through the proxy.
However, as far as I can tell, this is subtly broken in the case where
the Consul agent cannot reach the proxy over localhost.
If a proxy is configured with: `{ LocalServiceAddress: "127.0.0.1",
Checks: true }`, as is typical with a sidecar proxy, the Consul checks
are currently rewritten to `127.0.0.1:<random port>`. A Consul agent
that does not share the loopback address cannot reach this address. Just
to make sure I was not misunderstanding, I tried configuring the proxy
with `{ LocalServiceAddress: "<pod ip>", Checks: true }`. In this case,
while the checks are rewritten as expected and the agent can reach the
dynamic port, the proxy can no longer reach its backend because the
traffic is no longer on the loopback interface.
I think rewriting the checks to use `proxy.Address`, the proxy's own
address, is more correct in this case. That is the IP where the proxy
can be reached, both by other proxies and by a Consul agent running on
a different IP. The local service address should continue to use
`127.0.0.1` in most cases.
If a proxied service is a gRPC or HTTP2 service, but a path is exposed
using the HTTP1 or TCP protocol, Envoy should not be configured with
`http2ProtocolOptions` for the cluster backing the path.
A situation where this comes up is a gRPC service whose healthcheck or
metrics route (e.g. for Prometheus) is an HTTP1 service running on
a different port. Previously, if these were exposed either using
`Expose: { Checks: true }` or `Expose: { Paths: ... }`, Envoy would
still be configured to communicate with the path over HTTP2, which would
not work properly.
I spent some time today on my local Mac to figure out why Consul 1.6.3+
was not accepting limits.http_max_conns_per_client.
This adds an explicit check on number of file descriptors to be sure
it might work (this is no guarantee as if many clients are reaching
the agent, it might consume even more file descriptors)
Anyway, many users are fighting with RLIMIT_NOFILE, having a clear
message would allow them to figure out what to fix.
Example of message (reload or start):
```
2020-03-11T16:38:37.062+0100 [ERROR] agent: Error starting agent: error="system allows a max of 512 file descriptors, but limits.http_max_conns_per_client: 8192 needs at least 8212"
```
The previous PR which added these was accidentally performing the download
in the root directory. For the api, and sdk directories it should be in done
in the same directory that will be used to run tests. Otherwise the
wrong dependencies will be downloaded which may add unnecessary time to
the CI run.
Daniel Nephin
Daniel Nephin
Matt Keeler
Pierre Souchay
Pierre Souchay
Andrea Scarpino
danielehc
Hans Hasselberg
Blake Covarrubias
Freddy
Iryna Shustava
Luke Kysow
Alvin Huang
Jono Sosulska
David Yu
Chris Piraino
Andy Lindeman
Shaker Islam