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.

Sign up for GitHub

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

Jump to bottom

v1.8 backports 2021-01-19 #14654

Merged
merged 12 commits into from
Jan 21, 2021
Merged

v1.8 backports 2021-01-19 #14654

merged 12 commits into from
Jan 21, 2021

Conversation

vadorovsky
Copy link
Member

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

$ for pr in 14565 14516 14591 14524 14623 14645; do contrib/backporting/set-labels.py $pr done 1.8; done

tgraf and others added 12 commits January 19, 2021 10:43
@tgraf @vadorovsky
clustermesh: Ignore symlink files on fsnotify events
527bbd1 
[ upstream commit f034507 ]

Kubernetes secrets are mapped into the pod using symlinks. The initial
scan was already correctly ignoring symlinks but the fsnotify events
have not been. This has resulted in invalid cluster configurations being
added:

```
ClusterMesh:            0/3 clusters ready, 0 global-services
   cluster2: not-ready, 0 nodes, 0 identities, 0 services, 0 failures (last: never)
   └  Waiting for initial connection to be established
   ..2021_01_08_21_11_57.892158678: not-ready, 0 nodes, 0 identities, 0 services, 0 failures (last: never)
   └  Waiting for initial connection to be established
   ..data: not-ready, 0 nodes, 0 identities, 0 services, 0 failures (last: never)
   └  Waiting for initial connection to be established
```

Fixes: 076b018 ("Inter cluster connectivity (ClusterMesh)")

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Fix RemoveCIDR edge condition
92f5c68 
[ upstream commit 6ad5a22 ]

A CIDR should be able to be removed from itself.

Add test cases that would fail without the fix:

$ go test

----------------------------------------------------------------------
FAIL: ip_test.go:184: IPTestSuite.TestRemoveCIDRsEdgeCase

ip_test.go:190:
    s.testIPNetsEqual(allowedCIDRs, expectedCIDRs, c)
ip_test.go:83:
    c.Assert(created, HasLen, len(expected))
... obtained []*net.IPNet = []*net.IPNet(nil)
... n int = 1

----------------------------------------------------------------------
FAIL: ip_test.go:194: IPTestSuite.TestRemoveCIDRsEdgeCase2

ip_test.go:200:
    s.testIPNetsEqual(allowedCIDRs, expectedCIDRs, c)
ip_test.go:83:
    c.Assert(created, HasLen, len(expected))
... obtained []*net.IPNet = []*net.IPNet(nil)
... n int = 1

----------------------------------------------------------------------
FAIL: ip_test.go:176: IPTestSuite.TestRemoveSameCIDR

ip_test.go:180:
    c.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"allow CIDR prefix must be a superset of remove CIDR prefix"} ("allow CIDR prefix must be a superset of remove CIDR prefix")

OOPS: 14 passed, 3 FAILED

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Simplify removeCIDR() and rename as excludeContainedCIDR()
1bc7f9b 
[ upstream commit 28745d3 ]

Rename removeCIDR() as excludeContainedCIDR() and remove checks that
the caller already makes.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Remove indirections from excludeContainedCIDR()
e5091b6 
[ upstream commit e813f5a ]

Remove unnecessary pointer indirections from excludeContainedCIDR().

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Simplify excludeContainedCIDR()
e8c7e0b 
[ upstream commit 2e40bf5 ]

allowCIDR contains removeCIDR so they share all the bits in the allowCIDR.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Address bits from high to low
6b4c3f5 
[ upstream commit aafe9e2 ]

Address bits from high to low, starting from 0. This allows
arbitrarily sized masks to be manipulated without forcing masks to any
given size. For example, 0th bit is the highest bit, bits 0-7 are in
the highest byte.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Reduce re-entering loops
a82b699 
[ upstream commit 3a9cf53 ]

Remove redundant CIDRs in one pass instead of restarting the loops again.

Remove redundant CIDRs also from AllowCIDRs.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@jrajahalme @vadorovsky
ip: Remove RemoveCIDRs error return
3cac7e1 
[ upstream commit 255cb86 ]

Error return was only used for mixed address families. This is not
needed as net.Contains() seems to behave as expected when given mixed
address families. Remove the error return and add unit tests.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@aanm @vadorovsky
pkg/node: fix concurrent access of entry node
2474241 
[ upstream commit a544c51 ]

Fixes the following data race:

```
Read at 0x00c00079f200 by goroutine 43:
  github.com/cilium/cilium/pkg/node/manager.(*Manager).StartNeighborRefresh.func1.1()
      /go/src/github.com/cilium/cilium/pkg/node/manager/manager.go:581 +0xc4

Previous write at 0x00c00079f200 by goroutine 166:
  github.com/cilium/cilium/pkg/node/manager.(*Manager).NodeUpdated()
      /go/src/github.com/cilium/cilium/pkg/node/manager/manager.go:414 +0x824
  github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).ciliumNodeInit.func2()
      /go/src/github.com/cilium/cilium/pkg/k8s/watchers/cilium_node.go:71 +0x2d4
  k8s.io/client-go/tools/cache.ResourceEventHandlerFuncs.OnUpdate()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/client-go/tools/cache/controller.go:238 +0xa5
  k8s.io/client-go/tools/cache.(*ResourceEventHandlerFuncs).OnUpdate()
      <autogenerated>:1 +0x2e
  github.com/cilium/cilium/pkg/k8s/informer.NewInformerWithStore.func1()
      /go/src/github.com/cilium/cilium/pkg/k8s/informer/informer.go:114 +0x389
  k8s.io/client-go/tools/cache.(*DeltaFIFO).Pop()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/client-go/tools/cache/delta_fifo.go:507 +0x50d
  k8s.io/client-go/tools/cache.(*controller).processLoop()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/client-go/tools/cache/controller.go:183 +0x83
  k8s.io/client-go/tools/cache.(*controller).processLoop-fm()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/client-go/tools/cache/controller.go:181 +0x44
  k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:155 +0x75
  k8s.io/apimachinery/pkg/util/wait.BackoffUntil()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:156 +0xba
  k8s.io/apimachinery/pkg/util/wait.JitterUntil()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133 +0x114
  k8s.io/apimachinery/pkg/util/wait.Until()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:90 +0x507
  k8s.io/client-go/tools/cache.(*controller).Run()
      /go/src/github.com/cilium/cilium/vendor/k8s.io/client-go/tools/cache/controller.go:154 +0x4a9

Goroutine 43 (running) created at:
  github.com/cilium/cilium/pkg/node/manager.(*Manager).StartNeighborRefresh.func1()
      /go/src/github.com/cilium/cilium/pkg/node/manager/manager.go:578 +0x368
  github.com/cilium/cilium/pkg/controller.(*Controller).runController()
      /go/src/github.com/cilium/cilium/pkg/controller/controller.go:208 +0xd10

Goroutine 166 (running) created at:
  github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).ciliumNodeInit()
      /go/src/github.com/cilium/cilium/pkg/k8s/watchers/cilium_node.go:101 +0x184
```

Fixes: 5ec4d51 ("daemon, node: refresh neighbor by sending arping periodically")
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@pchaigno @vadorovsky
jenkinsfile: Allow enabling host firewall in k8s-all CI
97707db 
[ upstream commit 562aa3d ]

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@aanm @vadorovsky
pkg/kvstore: fix panic when closing etcd connections on error
4fa64bd 
[ upstream commit 0228afa ]

```
goroutine 319 [running]:
go.etcd.io/etcd/clientv3/concurrency.(*Session).Orphan(...)
        /go/src/github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3/concurrency/session.go:90
go.etcd.io/etcd/clientv3/concurrency.(*Session).Close(0xc000dcca50, 0xc00145d790, 0x2)
        /go/src/github.com/cilium/cilium/vendor/go.etcd.io/etcd/clientv3/concurrency/session.go:96 +0x2a
github.com/cilium/cilium/pkg/kvstore.(*etcdClient).Close(0xc0014f2c30)
        /go/src/github.com/cilium/cilium/pkg/kvstore/etcd.go:1652 +0xc8
github.com/cilium/cilium/pkg/clustermesh.(*remoteCluster).restartRemoteConnection.func1(0x2e862e0, 0xc0009fc300, 0x460d7a0, 0x415c26)
        /go/src/github.com/cilium/cilium/pkg/clustermesh/remote_cluster.go:178 +0x408
github.com/cilium/cilium/pkg/controller.(*Controller).runController(0xc0008d2100)
        /go/src/github.com/cilium/cilium/pkg/controller/controller.go:205 +0xa2a
created by github.com/cilium/cilium/pkg/controller.(*Manager).updateController
        /go/src/github.com/cilium/cilium/pkg/controller/manager.go:120 +0xb09
```

Fixes: c1b05df ("pkg/kvstore: introduced a dedicated session for locks")
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@joestringer @vadorovsky
cilium-cni: Fix error handling for bad netns
233a039 
[ upstream commit 6e3ca8f ]

If kubelet gives cilium-cni bad input (no netns), the error here would
not be returned properly to the caller, which could result in a segfault:

    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x14e0c8b]
    goroutine 1 [running, locked to thread]:
    main.cmdAdd(0xc00015a000, 0xc0004d60e8, 0x5)
            /go/src/github.com/cilium/cilium/plugins/cilium-cni/cilium-cni.go:354 +0x5cb
    github.com/containernetworking/cni/pkg/skel.(*dispatcher).checkVersionAndCall(0xc0005e5d40, 0xc00015a000, 0x1a42f20, 0xc0004de000, 0x18d07c0, 0x0, 0x44a1ef)
            /go/src/github.com/cilium/cilium/vendor/github.com/containernetworking/cni/pkg/skel/skel.go:185 +0x258
    github.com/containernetworking/cni/pkg/skel.(*dispatcher).pluginMain(0xc0005e5d40, 0x18d07c0, 0x0, 0x18d07c8, 0x1a42f20, 0xc0004de000, 0xc000174000, 0x5d, 0xc000174000)
            /go/src/github.com/cilium/cilium/vendor/github.com/containernetworking/cni/pkg/skel/skel.go:221 +0x546
    github.com/containernetworking/cni/pkg/skel.PluginMainWithError(...)
            /go/src/github.com/cilium/cilium/vendor/github.com/containernetworking/cni/pkg/skel/skel.go:286
    github.com/containernetworking/cni/pkg/skel.PluginMain(0x18d07c0, 0x0, 0x18d07c8, 0x1a42f20, 0xc0004de000, 0xc000174000, 0x5d)
            /go/src/github.com/cilium/cilium/vendor/github.com/containernetworking/cni/pkg/skel/skel.go:301 +0x128
    main.main()
            /go/src/github.com/cilium/cilium/plugins/cilium-cni/cilium-cni.go:85 +0x33c

The above logs would typically be pushed to kubelet logs.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
@vadorovsky vadorovsky requested review from a team as code owners January 19, 2021 09:51
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.8 kind/backports This PR provides functionality previously merged into master. labels Jan 19, 2021
@vadorovsky
Copy link
Member Author

never-tell-me-the-odds

pchaigno
pchaigno approved these changes Jan 19, 2021
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My changes LGTM!

@pchaigno
Copy link
Member

pchaigno commented Jan 19, 2021 •
edited
Loading

never-tell-me-the-odds

@mrostecki We changed it to:

test-backport-1.8

aanm
aanm approved these changes Jan 19, 2021
View reviewed changes
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My changes look good to me. Thanks!

@aanm
Copy link
Member

aanm commented Jan 19, 2021 •
edited
Loading

test-upstream-k8s (previous failure: https://jenkins.cilium.io/job/Cilium-PR-Kubernetes-Upstream/2795/execution/node/64/log/)

@vadorovsky
Copy link
Member Author

vadorovsky commented Jan 20, 2021 •
edited
Loading

test-missed-k8s (previous failure: https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-K8s/3902/)

@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jan 20, 2021
@rolinh rolinh merged commit 5b5f427 into v1.8 Jan 21, 2021
@rolinh rolinh deleted the pr/v1.8-backport-2021-01-19 branch January 21, 2021 12:40
@joestringer
Copy link
Member

@rolinh this is a reminder to run the "labels update" command from backports PRs when you merge them. I've done it this time (hopefully one day we'll just automate this step...).

@joestringer joestringer mentioned this pull request Feb 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@rolinh rolinh rolinh approved these changes

@pchaigno pchaigno pchaigno approved these changes

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@vadorovsky @pchaigno @aanm @joestringer @rolinh @tgraf @jrajahalme