Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pr/v1.7 backport 2020 10 20 #13655

Closed
wants to merge 722 commits into from
Closed

Conversation

nathanjsweet
Copy link
Member

v1.7 backports 2020-10-20

Once this PR is merged, you can update the PR labels via:

$ for pr in 13295 13640; do contrib/backporting/set-labels.py $pr done 1.7; done

soumynathan and others added 30 commits June 30, 2020 16:06
[ upstream commit 52b44d1 ]

This PR fixes setting MonitorAggregationLevel to max to correctly
reflect through CLI.

Fixes: #12000
Signed-off-by: Swaminathan Vasudevan <svasudevan@suse.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit b330436 ]

Restart command in documentation pipes complete output of the awk into
xargs which make creates invalid kubectl command. This commit updates
xargs to use per line mode and also makes it omit empty input to not
execute empty "kubectl delete pod" on the last line.

Signed-off-by: Arthur Evstifeev <aevstifeev@gitlab.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit d1f2b48 ]

This updates the script to allow the user to specify which namespace
Cilium is deployed in.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 3f96392 ]

Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit dc4f549 ]

Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 1f8cc15 ]

This commit makes it possible to do the following:

```
$ ./contrib/k8s/k8s-cilium-exec.sh bash -c "cilum status && hostname && echo"
```

Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 8191855 ]

This commit adds a few misc. changes such as defining a new helper
function and using double-quotes around variables for consistency.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
…tore

[ upstream commit 0ee0458 ]

Running Cilium with K8sEventHandover and without a KVStore configured
causes it on not being able to delete any CNP after being created.

Fixes: e4e83e8 ("daemon: Allow kvstore to be unconfigured")
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 947a586 ]

Helm name for the native routing CIDR is
"global.nativeRoutingCIDR". Cilium agent command line option name is
"native-routing-cidr".

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 95c74ca ]

When specify cilium-agent to use syslog driver, all logs will still be
output to `stdout`. This is because each module uses the `DefaultLogger`
(cilium's customized logger, not logrus's default logger), while syslog
hook is not added to this logger.

This patch fixes this problem by adding syslog hook to `DefaultLogger`.

Signed-off-by: arthurchiao <arthurchiao@hotmail.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 640f669 ]

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 462e08d ]

In an IPv6 only cluster, the operator host was set to `[::1]` in the
Helm chart. This is a problem as it is "bracketed" when
concatenated with the port to form an invalid address: `[[::1]]:9234`.
In turn, this prevented the liveness probe from working:

    Warning  Unhealthy         10m (x11 over 14m)   kubelet, kind-worker2  Liveness probe failed: Get http://[[::1]]:9234/healthz: dial tcp: address [[::1]]:9234: missing port in address

This commit fixes this issue by removing the extraneous brackets in the
Helm chart of the Cilium operator.

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 8524fca ]

A user reported an issue where Cilium would become unable to provision
new endpoints, with the following messages recurring in the agent logs:

[T=0s] ... level=info msg="New endpoint"
[T=0s] ... level=info msg="Resolving identity labels (blocking)
[T=43s]... level=error msg="Forcefully unlocking local kvstore lock"
[T=89s]... level=info msg="Removed endpoint"

Upon further investigation of "gops stack" output from a sysdump in the
environment, the following stack trace was observed on Cilium 1.6.8:

goroutine 589 [select, 14 minutes]:
github.com/cilium/cilium/vendor/google.golang.org/grpc/internal/transport.(*Stream).waitOnHeader(0xc00214d500, 0x8, 0xc000740d10)
        /go/src/github.com/cilium/cilium/vendor/google.golang.org/grpc/internal/transport/transport.go:245 +0xcc
github.com/cilium/cilium/vendor/google.golang.org/grpc/internal/transport.(*Stream).RecvCompress(...)
        /go/src/github.com/cilium/cilium/vendor/google.golang.org/grpc/internal/transport/transport.go:256
github.com/cilium/cilium/vendor/google.golang.org/grpc.(*csAttempt).recvMsg(0xc000740d10, 0x32ec340, 0xc001caf890, 0x0, 0xc001e52650, 0x0)
        /go/src/github.com/cilium/cilium/vendor/google.golang.org/grpc/stream.go:850 +0x70a
github.com/cilium/cilium/vendor/google.golang.org/grpc.(*clientStream).RecvMsg.func1(0xc000740d10, 0x0, 0x0)
        /go/src/github.com/cilium/cilium/vendor/google.golang.org/grpc/stream.go:715 +0x46
...
github.com/cilium/cilium/vendor/go.etcd.io/etcd/etcdserver/etcdserverpb.(*leaseClient).LeaseGrant(0xc00042a238, 0x38e6ae0, 0xc00005cb40, 0xc00150ad20, 0xc001fbaea0, 0x4
        /go/src/github.com/cilium/cilium/vendor/go.etcd.io/etcd/etcdserver/etcdserverpb/rpc.pb.go:3792 +0xd3
github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3.(*retryLeaseClient).LeaseGrant(0xc000ec0510, 0x38e6ae0, 0xc00005cb40, 0xc00150ad20, 0x53bcba0, 0x3, 0x3, 0x1319
        /go/src/github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3/retry.go:144 +0xeb
github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3.(*lessor).Grant(0xc0006a8640, 0x38e6ae0, 0xc00005cb40, 0x19, 0x0, 0xc000abd680, 0xc000abd708)
        /go/src/github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3/lease.go:216 +0x98
github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3/concurrency.NewSession(0xc000c15860, 0xc001e52f50, 0x1, 0x1, 0x0, 0x0, 0x0)
        /go/src/github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3/concurrency/session.go:46 +0x308
github.com/cilium/cilium/pkg/kvstore.(*etcdClient).renewLockSession(0xc000b00790, 0xc0227a56a6, 0x14dffe3633def)
        /go/src/github.com/cilium/cilium/pkg/kvstore/etcd.go:467 +0xde
github.com/cilium/cilium/pkg/kvstore.connectEtcdClient.func4(0x38e6ae0, 0xc00005d640, 0x53bd3e0, 0x2)
        /go/src/github.com/cilium/cilium/pkg/kvstore/etcd.go:583 +0x2a
github.com/cilium/cilium/pkg/controller.(*Controller).runController(0xc000c06ff0)
        /go/src/github.com/cilium/cilium/pkg/controller/controller.go:203 +0x9e1
created by github.com/cilium/cilium/pkg/controller.(*Manager).UpdateController
        /go/src/github.com/cilium/cilium/pkg/controller/manager.go:112 +0xae0

In particular, it's useful to note:
* The goroutine has been blocking for 14 minutes attempting to make progress
  on re-establishing the etcd session.
* This path is hit by a controller responsible for handling etcd session
  renewal in the event of etcd connectivity issues. It would not be triggered
  by initial etcd connectivity failures, only when etcd connectivity was
  successfully established then subsequently lost.
* NewSession() attempts to pick one of the configured etcd peers to connect to,
  and will block until connectivity to that peer is restored. It will not by
  itself back off and re-attempt connectivity to another node in the etcd
  cluster.
* This path is in the critical section for writelock of `etcdClient.RWMutex`
  so will block out all etcd client reads.

The above is consistent with the agent logs: Effectively, a new endpoint is
scheduled and Cilium attempts to allocate an identity for it. This process
requires the kvstore and will block on the lock. Given that it makes no
progress, first we see the lock liveness check kick to forcefully unlock
the path lock (`Forcefully unlocking local kvstore lock`), then later we see
cilium-cni / kubelet give up on creating the endpoint and remove it again.

This patch fixes the issue by introducing a timeout when attempting to renew
the session. Each time `NewSession()` is called, the etcd client library will
pick an etcd peer to connect to. The intent behind the timeout is to provide
a way for Cilium to detect when it is attempting to connect to an unhealthy
peer, and try to reconnect to one of the other peers in the etcd cluster in
the hopes that that other peer will be healthy.

In the event that there is an etcd connectivity outage where one of
three etcd peers is unhealthy, we expect that the remaining two can
retain chorum and continue to operate despite the outage of the third
peer. In this case if Cilium was attempting to connect to the third
(unhealthy) peer, Cilium would previously block indefinitely. With this
patch Cilium will time out after statusCheckTimeout (10s) and
re-establish a session to the remaining etcd peers, thereby unblocking
subsequent endpoint provisioning.

Signed-off-by: Joe Stringer <joe@cilium.io>
Co-authored-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit b99c7b8 ]

When renewSession() attempts to re-establish a session with an etcd
peer, it may block for some period of time, currently 10s. At the
moment, this timeout is added to the LockPath() timeout because the
LockPath() context timeout is created *after* acquiring the lock.
Move the context creation before the lock acquisition to allow these
timers to overlap to provide more accurate bounding on the timeouts
here.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 8e1a7fa ]

If the KVStore connectivity is not reliable during the endpoint restore
process Cilium can end up with an endpoint in a 'restoring' state in
case the ep's security identity resolution fails.
Adding a controller will make sure Cilium will retry to get an identity
for that endpoint until the endpoint is removed or the connectivity
with the allocator is successful.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 54d2175 ]

If the KVStore connectivity is not reliable during the endpoint restore
process Cilium can end up with an endpoint in a 'restoring' state in
case the global security identities sync would fail or time out.
Adding a controller will make sure Cilium will wait until the global
security identities are synced or until the endpoint is removed before
restoring the endpoint.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 8390305 ]

With the introduction of CiliumClusterwideNetworkPolicies a policy has
the ability to select the 'reserved:health' endpoints. If a user applies
a policy such as [0], it will block all connectivity for the health
endpoints. As this does not seem expected, since Cilium will report that
the connectivity across Cilium pods is not working, the user will need to
deploy a CCNP that allows connectivity between health endpoints. This
can be done with [1].

This commit introduces 'reserved:health' as an entity that can be
selected by CNP and / or CCNP.

[0]
```

Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit c93459e ]

Under heavy load, the round-trip-time (RTT) for DNS requests between a
TCP client and a DNS proxy may exceed the 100ms timeout specified when
creating the client in the dnsproxy tests.

This was observed on the test-PR #12298, with a RTT value going up to
296ms (under exceptional memory strain).

This might be the cause for the rare flakes reported in #12042. Let's
increase this timeout. The timeout is only used a couple of times in the
tests, so increasing it by a few hundred milliseconds would have no
visible impact. And because we expect all requests from the TCP client
to succeed on the L4 anyway (i.e. it should never time out in our
tests), this should not prolong at all the execution of tests in the
normal case.

Let's also retrieve and print the RTT value for that request in case of
error, to get more info if this change were not enough to fix the flake.

Hopefully fixes: #12042
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 8cf1e20 ]

This commit fixes a typo that prevents LOCKDEBUG from working for the
`docker-plugin-image` make target.

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit ca767ee ]

'--no-wildcard' allows the socket match to find zero-bound (listening)
sockets, which we do not want, as this may intercept (reply) traffic
intended for other nodes when an ephemeral source port number
allocated in one node happens to be the same as the allocated proxy
port number in 'this' node (the node doing the iptables socket match
changed here).

Fixes: #12241
Related: #8864
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit fa8857f ]

Inherit the identity allocation context from the parent function when
calling into identityLabelsChanged(). This function isn't a background
thread, and it receives a context so it should respect the passed
context.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 8bb5382 ]

When there's some kind of late error / failure and a newly allocated
identity must be released, allow the kvstore connectivity timeout to be
customised via the standard kvstore connectivity timeout.

This path may still be called from endpoint create, so it's not
appropriate to block for up to two minutes to attempt to roll back the
identity allocation here.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit b796665 ]

This fixes the following CVEs for the Envoy version 1.13.x:

- CVE-2020-12603 (CVSS score 7.0, High): Envoy through 1.14.2 may consume excessive amounts of memory when proxying HTTP/2 requests or responses with many small (e.g., 1 byte) data frames.

- CVE-2020-12605 (CVSS score 7.0, High): Envoy through 1.14.2 may consume excessive amounts of memory when processing HTTP/1.1 headers with long field names or requests with long URLs.

- CVE-2020-8663 (CVSS score 7.0, High): Envoy version 1.14.2 or earlier may exhaust file descriptors and/or memory when accepting too many connections.

- CVE-2020-12604 (CVSS score 5.3, Medium): Envoy through 1.14.2 is susceptible to increased memory usage in the case where an HTTP/2 client requests a large payload but does not send enough window updates to consume the entire stream and does not reset the stream. The attacker can cause data associated with many streams to be buffered forever.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
[ upstream commit 471fe63 ]

Skip Istio test if running cilium-istioctl is not supported for the
current Go runtime.

Support running Istio test from OSX by downloading the osx version of
cilium-istioctl if the test suite is running in OSX. This allows
running the Istio test on a remote cluster (e.g., GKE) when Ginkgo is
running on OSX.

On Windows the test is skipped, even though the cilium-istioctl binary
is released also for Windows, but this has not been tested yet.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
[ upstream commit 47f8d32 ]

The curl is reaching out to a world / external resource so retrying is
acceptable, and helps test flakes.

Fixes: #11797

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
[ upstream commit 015ffbd ]

Remove ginkgo test source dependency on pkg/iptables, as that only
compiles on Linux. This allows running ginkgo from OSX against GKE,
for example.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
[ upstream commit 12ef7d1 ]

Only use the Ginkgo runtime OS for determining which cilium-istioctl
binary to download is the command executor is local, otherwise default
to "linux". This supports Ginkgo running in OSX both with local and
SSH Executors.

Fixes: #11905
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
[ upstream commit 31f8ba0 ]

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
[ upstream commit 606736c ]

Signed-off-by: Rene Zbinden <rene.zbinden@postfinance.ch>
Signed-off-by: André Martins <andre@cilium.io>
[ upstream commit e7d4f5c ]

Signed-off-by: Rene Zbinden <rene.zbinden@postfinance.ch>
Signed-off-by: André Martins <andre@cilium.io>
pchaigno and others added 3 commits October 20, 2020 19:44
[ upstream commit ec2c18a ]

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit ad6cffe ]

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit 6e85cb4 ]

The k8s Service "spec" disallows accessing ClusterIP services from
outside a cluster.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
@nathanjsweet nathanjsweet requested a review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested review from a team as code owners October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested review from a team as code owners October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team as a code owner October 21, 2020 00:53
@nathanjsweet nathanjsweet requested review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested review from a team as code owners October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team as a code owner October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested review from a team as code owners October 21, 2020 00:53
@nathanjsweet nathanjsweet requested a review from a team October 21, 2020 00:53
@nathanjsweet nathanjsweet requested review from a team as code owners October 21, 2020 00:53
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-merge/needs-release-note-label The author needs to describe the release impact of these changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet