k8s: keep CiliumNode labels synced with Node object #18609

jibi · 2022-01-25T13:09:19Z

See individual commits

aanm

Just 2 small nits. LGTM, doesn't require my 2nd review. However, it would be great if we could have some e2e tests that covers this.

daemon/cmd/daemon.go

pkg/k8s/watchers/cilium_node.go

daemon/cmd/daemon.go

aanm

Discussed offline, there are some necessary changes to handle the kvstore mode node update

test/k8s/Node.go

jibi · 2022-03-28T13:59:13Z

/test

aanm · 2022-03-28T21:15:14Z

pkg/k8s/watchers/node.go

+	if !reflect.DeepEqual(oldNode.Status.Addresses, newNode.Status.Addresses) {
+		return false
+	}
+


If the title says we won't do anything for addresses then we can drop this?

Correct, this is not needed now, I'll drop it 👍
(but I'll still keep this function as I'll probably need the annotations check for the KV store case)

This commit ensures that any update to the labels of a k8s Node object is reflected in the corresponding CiliumNode one. Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>

jibi · 2022-03-28T21:20:42Z

/test

As the previous commit mentions, this does not occur on main, v1.16, and v1.15 due to cilium#29036. However, in v1.14 and v1.13, we need to take special care because the NameManager and SelectorCache lock can be taken while the Endpoint lock is held during Endpoint deletion. Here are the relevant stacktraces regarding the deadlock: ``` 1: sync.Mutex.Lock [75 minutes] [Created by http.(*Server).Serve in goroutine 1699 @ server.go:3086] sync sema.go:77 runtime_SemacquireMutex(*uint32(0x5), false, 43690) sync mutex.go:171 (*Mutex).lockSlow(*Mutex(cilium#1733)) sync mutex.go:90 (*Mutex).Lock(...) sync rwmutex.go:147 (*RWMutex).Lock(*RWMutex(0xb0)) fqdn name_manager.go:70 (*NameManager).Lock(0xffffffffffffffff) policy selectorcache.go:964 (*SelectorCache).RemoveSelectors(cilium#1088, {cilium#28569, 0xb, 1}, {cilium#643, cilium#32582}) policy l4.go:810 (*L4Filter).removeSelectors(cilium#32582, cilium#29992) policy l4.go:817 (*L4Filter).detach(cilium#719, cilium#29993) policy l4.go:988 L4PolicyMap.Detach(...) policy l4.go:1179 (*L4Policy).Detach(cilium#20318, cilium#1383) policy resolve.go:103 (*selectorPolicy).Detach(...) policy distillery.go:81 (*PolicyCache).delete(cilium#1354, cilium#19354) policy distillery.go:138 (*PolicyCache).LocalEndpointIdentityRemoved(cilium#523, cilium#1163) identitymanager manager.go:167 (*IdentityManager).remove(cilium#706, cilium#19354) identitymanager manager.go:147 (*IdentityManager).Remove(cilium#706, cilium#19354) identitymanager manager.go:52 Remove(...) endpoint endpoint.go:1146 (*Endpoint).leaveLocked(cilium#1883, cilium#12221, {0x30, 0}) endpoint endpoint.go:2192 (*Endpoint).Delete(cilium#1883, {0x80, 0xaa}) endpointmanager manager.go:380 (*EndpointManager).removeEndpoint(cilium#1161, 0, {0xff, 0xff}) endpointmanager manager.go:394 (*EndpointManager).RemoveEndpoint(...) cmd endpoint.go:684 (*Daemon).deleteEndpointQuiet(...) cmd endpoint.go:666 (*Daemon).deleteEndpoint(cilium#1155, cilium#1883) cmd endpoint.go:713 (*Daemon).DeleteEndpoint(cilium#1155, {cilium#27399, cilium#8108}) cmd endpoint.go:770 (*deleteEndpointID).Handle(cilium#21193, {cilium#2452, {cilium#27399, 0x4d}}) endpoint delete_endpoint_id.go:66 (*DeleteEndpointID).ServeHTTP(cilium#5934, {cilium#666, cilium#2242}, cilium#2452) middleware operation.go:28 (*Context).RoutesHandler.NewOperationExecutor.func1({cilium#666, cilium#2242}, cilium#2452) http server.go:2136 HandlerFunc.ServeHTTP(ReadCloser(cilium#130), func{cilium#2242, 0x3}) middleware router.go:78 NewRouter.func1({cilium#666, cilium#2242}, cilium#2451) http server.go:2136 HandlerFunc.ServeHTTP(ReadCloser(cilium#718), func{cilium#2242, #59}) middleware redoc.go:72 Redoc.func1({cilium#666, cilium#2242}, cilium#1251) http server.go:2136 HandlerFunc.ServeHTTP(ReadCloser(cilium#4920), func{cilium#2242, #45}) middleware spec.go:46 Spec.func1({cilium#666, cilium#2242}, cilium#4921) http server.go:2136 HandlerFunc.ServeHTTP(ReadCloser(cilium#10532), func{cilium#2242, cilium#23015}) metrics middleware.go:64 (*APIEventTSHelper).ServeHTTP(cilium#1459, {cilium#668, cilium#10533}, cilium#2451) api apipanic.go:42 (*APIPanicHandler).ServeHTTP(cilium#722, {cilium#668, cilium#10533}, cilium#4922) http server.go:2938 serverHandler.ServeHTTP(*Server(cilium#8105), cilium#668, cilium#10533, 0x6) http server.go:2009 (*conn).serve(*conn(cilium#16005), Context{cilium#673, cilium#1554}) 8: sync.Mutex.Lock [74 minutes] [Created by http.(*Server).Serve in goroutine 1699 @ server.go:3086] sync sema.go:77 runtime_SemacquireMutex(*, 0x47, cilium#1154) sync mutex.go:171 (*Mutex).lockSlow(cilium#706) sync mutex.go:90 (*Mutex).Lock(...) sync rwmutex.go:147 (*RWMutex).Lock(*) identitymanager manager.go:99 (*IdentityManager).RemoveOldAddNew(cilium#706, 0, cilium#1154) identitymanager manager.go:123 RemoveOldAddNew(...) endpoint policy.go:852 (*Endpoint).SetIdentity(*, cilium#1154, 0) endpoint endpoint.go:1932 (*Endpoint).identityLabelsChanged(*, {cilium#674, *}, 1) endpoint endpoint.go:1780 (*Endpoint).runIdentityResolver(*, {cilium#674, *}, 1, 1) endpoint endpoint.go:1720 (*Endpoint).UpdateLabels(*, {cilium#674, *}, *, *, 8) cmd endpoint.go:477 (*Daemon).createEndpoint(cilium#1155, {cilium#673, *}, {cilium#683, cilium#1155}, *) cmd endpoint.go:542 (*putEndpointID).Handle(cilium#21192, {*, *, {*, 0xe}}) endpoint put_endpoint_id.go:58 (*PutEndpointID).ServeHTTP(cilium#3961, {cilium#666, *}, *) middleware operation.go:28 (*Context).RoutesHandler.NewOperationExecutor.func1({cilium#666, *}, *) http server.go:2136 HandlerFunc.ServeHTTP(*, {cilium#666, *}, *) middleware router.go:78 NewRouter.func1({cilium#666, *}, *) http server.go:2136 HandlerFunc.ServeHTTP(*, {cilium#666, *}, *) middleware redoc.go:72 Redoc.func1({cilium#666, *}, cilium#1251) http server.go:2136 HandlerFunc.ServeHTTP(*, {cilium#666, *}, #45) middleware spec.go:46 Spec.func1({cilium#666, *}, *) http server.go:2136 HandlerFunc.ServeHTTP(*, {cilium#666, *}, *) metrics middleware.go:64 (*APIEventTSHelper).ServeHTTP(cilium#1459, {cilium#668, *}, *) api apipanic.go:42 (*APIPanicHandler).ServeHTTP(#49, {cilium#668, *}, *) http server.go:2938 serverHandler.ServeHTTP({cilium#653}, {cilium#668, *}, 6) http server.go:2009 (*conn).serve(*, {cilium#673, cilium#1554}) 5: sync.Mutex.Lock [75 minutes] [Created by eventqueue.(*EventQueue).Run in goroutine 1482 @ eventqueue.go:229] sync sema.go:77 runtime_SemacquireMutex(cilium#142, 0xe8, *) sync mutex.go:171 (*Mutex).lockSlow(cilium#1733) sync mutex.go:90 (*Mutex).Lock(...) sync rwmutex.go:147 (*RWMutex).Lock(0x68) fqdn name_manager.go:70 (*NameManager).Lock(*) policy selectorcache.go:798 (*SelectorCache).AddFQDNSelector(cilium#1088, {cilium#643, *}, {{*, 0x4d}, {0, 0}}) policy l4.go:628 (*L4Filter).cacheFQDNSelector(...) policy l4.go:623 (*L4Filter).cacheFQDNSelectors(*, {*, 4, cilium#193}, cilium#536) policy l4.go:725 createL4Filter({cilium#680, *}, {*, 1, 1}, 0, {cilium#660, *}, {{*, 4}, ...}, ...) policy l4.go:879 createL4EgressFilter(...) policy rule.go:717 mergeEgressPortProto({cilium#680, *}, #44, {*, 0xa, 0}, *, {cilium#660, *}, {{*, ...}, ...}, ...) policy rule.go:672 mergeEgress.func1({cilium#660, *}) api l4.go:284 PortRules.Iterate({*, 1, cilium#546}, *) policy rule.go:624 mergeEgress({cilium#680, *}, *, {*, 1, 1}, 0, {cilium#661, *}, {cilium#662, ...}, ...) policy rule.go:753 (*rule).resolveEgressPolicy(*, {cilium#680, *}, *, *, *, {0, 0, 0}, {0, ...}) policy rules.go:103 ruleSlice.resolveL4EgressPolicy({*, *, *}, {cilium#680, *}, *) policy repository.go:718 (*Repository).resolvePolicyLocked(cilium#1089, *) policy distillery.go:119 (*PolicyCache).updateSelectorPolicy(cilium#1354, *) policy distillery.go:153 (*PolicyCache).UpdatePolicy(...) endpoint policy.go:262 (*Endpoint).regeneratePolicy(*) endpoint bpf.go:744 (*Endpoint).runPreCompilationSteps(*, *, *) endpoint bpf.go:589 (*Endpoint).regenerateBPF(*, *) endpoint policy.go:457 (*Endpoint).regenerate(*, *) endpoint events.go:53 (*EndpointRegenerationEvent).Handle(*, *) eventqueue eventqueue.go:245 (*EventQueue).run.func1() sync once.go:74 (*Once).doSlow(*, *) sync once.go:65 (*Once).Do(...) eventqueue eventqueue.go:233 (*EventQueue).run(*) 1: select [75 minutes] [Created by eventqueue.(*EventQueue).Run in goroutine 1482 @ eventqueue.go:229] semaphore semaphore.go:60 (*Weighted).Acquire(cilium#1092, {cilium#671, cilium#722}, cilium#766) lock semaphored_mutex.go:30 (*SemaphoredMutex).Lock(...) ipcache ipcache.go:140 (*IPCache).Lock(...) ipcache cidr.go:56 (*IPCache).AllocateCIDRs(cilium#1316, {0, 0, cilium#2037}, {0, 0, 0}, 0) ipcache cidr.go:103 (*IPCache).AllocateCIDRsForIPs(0, {0, cilium#697, 0}, 0xffffffffffffffff) cmd identity.go:114 cachingIdentityAllocator.AllocateCIDRsForIPs(...) policy selectorcache.go:509 (*SelectorCache).allocateIdentityMappings(cilium#1088, {{0, 0}, {cilium#5036, 0x2b}}, #45) policy selectorcache.go:843 (*SelectorCache).AddFQDNSelector(cilium#1088, {cilium#643, cilium#27811}, {{0, 0}, {cilium#5036, 0x2b}}) policy l4.go:628 (*L4Filter).cacheFQDNSelector(...) policy l4.go:623 (*L4Filter).cacheFQDNSelectors(cilium#27811, {#42936, 0x287, cilium#193}, cilium#536) policy l4.go:725 createL4Filter({cilium#680, cilium#22826}, {cilium#22828, 1, 1}, 0, {cilium#660, cilium#24510}, {{cilium#4487, 3}, ...}, ...) policy l4.go:879 createL4EgressFilter(...) policy rule.go:717 mergeEgressPortProto({cilium#680, cilium#22826}, #44, {cilium#22828, 0xa, #78536}, #79633, {cilium#660, cilium#24510}, {{cilium#4487, ...}, ...}, ...) policy rule.go:672 mergeEgress.func1({cilium#660, cilium#24510}) api l4.go:284 PortRules.Iterate({cilium#24510, 1, cilium#546}, cilium#11741) policy rule.go:624 mergeEgress({cilium#680, cilium#22826}, cilium#18687, {cilium#22828, 1, 1}, 0, {cilium#661, cilium#5624}, {cilium#662, ...}, ...) policy rule.go:753 (*rule).resolveEgressPolicy(cilium#24575, {cilium#680, cilium#22826}, cilium#18687, cilium#29345, cilium#4782, {0, 0, 0}, {0, ...}) policy rules.go:103 ruleSlice.resolveL4EgressPolicy({cilium#10690, 0xb, 5}, {cilium#680, cilium#22826}, cilium#18687) policy repository.go:718 (*Repository).resolvePolicyLocked(cilium#1089, cilium#18461) policy distillery.go:119 (*PolicyCache).updateSelectorPolicy(cilium#1354, cilium#18461) policy distillery.go:153 (*PolicyCache).UpdatePolicy(...) endpoint policy.go:262 (*Endpoint).regeneratePolicy(cilium#1748) endpoint bpf.go:744 (*Endpoint).runPreCompilationSteps(cilium#1748, cilium#27542, cilium#4781) endpoint bpf.go:589 (*Endpoint).regenerateBPF(cilium#1748, cilium#27542) endpoint policy.go:457 (*Endpoint).regenerate(cilium#1748, cilium#27542) endpoint events.go:53 (*EndpointRegenerationEvent).Handle(cilium#18609, cilium#703) eventqueue eventqueue.go:245 (*EventQueue).run.func1() sync once.go:74 (*Once).doSlow(*Once(#45414), func(cilium#69)) sync once.go:65 (*Once).Do(...) eventqueue eventqueue.go:233 (*EventQueue).run(cilium#6023) ``` Generated from pp tool: https://github.com/maruel/panicparse Signed-off-by: Chris Tarazi <chris@isovalent.com>

jibi added area/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/agent Cilium agent related. labels Jan 25, 2022

jibi requested review from ldelossa, aanm and a team January 25, 2022 13:09

maintainer-s-little-helper bot assigned aanm and ldelossa and unassigned ldelossa Jan 25, 2022

jibi added the needs-backport/1.11 label Jan 25, 2022

jibi mentioned this pull request Jan 25, 2022

Node IP changes don't get recognized #15406

Open

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch 2 times, most recently from 77f1c8d to 089ad2e Compare January 25, 2022 14:03

maintainer-s-little-helper bot mentioned this pull request Jan 25, 2022

CI: K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master #18566

Closed

aanm approved these changes Jan 26, 2022

View reviewed changes

daemon/cmd/daemon.go Outdated Show resolved Hide resolved

pkg/k8s/watchers/cilium_node.go Outdated Show resolved Hide resolved

maintainer-s-little-helper bot unassigned aanm Jan 26, 2022

jibi marked this pull request as draft January 26, 2022 14:55

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch 2 times, most recently from ea8306e to 9e110a1 Compare February 1, 2022 15:51

jibi marked this pull request as ready for review February 1, 2022 15:51

jibi requested a review from a team as a code owner February 1, 2022 15:51

jibi requested a review from nebril February 1, 2022 15:51

maintainer-s-little-helper bot assigned nebril Feb 1, 2022

jibi requested a review from aanm February 1, 2022 15:52

maintainer-s-little-helper bot assigned aanm Feb 1, 2022

jibi commented Feb 1, 2022

View reviewed changes

daemon/cmd/daemon.go Outdated Show resolved Hide resolved

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch from 9e110a1 to 1972cf0 Compare February 4, 2022 13:16

aanm requested changes Feb 7, 2022

View reviewed changes

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch from 28a53d7 to 1d3c4ca Compare March 28, 2022 11:52

qmonnet approved these changes Mar 28, 2022

View reviewed changes

test/k8s/Node.go Show resolved Hide resolved

jibi changed the title ~~k8s: keep CiliumNode labels synced with Node object~~ k8s: keep CiliumNode labels and addresses synced with Node object Mar 28, 2022

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch from 1d3c4ca to 296a199 Compare March 28, 2022 13:20

jibi marked this pull request as draft March 28, 2022 13:36

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch 2 times, most recently from c5f7044 to dd41fa0 Compare March 28, 2022 13:57

jibi marked this pull request as ready for review March 28, 2022 21:01

jibi changed the title ~~k8s: keep CiliumNode labels and addresses synced with Node object~~ k8s: keep CiliumNode labels synced with Node object Mar 28, 2022

jibi requested a review from aanm March 28, 2022 21:13

aanm requested changes Mar 28, 2022

View reviewed changes

jibi added 2 commits March 28, 2022 23:17

k8s: keep CiliumNode labels synced with Node object

8e1c465

This commit ensures that any update to the labels of a k8s Node object is reflected in the corresponding CiliumNode one. Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>

watchers: don't ignore CiliumNode events if labels get updated

7a1c7ea

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>

jibi force-pushed the pr/jibi/sync-node-labels-to-ciliumnode-obj branch from dd41fa0 to 7a1c7ea Compare March 28, 2022 21:18

aanm approved these changes Mar 28, 2022

View reviewed changes

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 29, 2022

qmonnet merged commit f5a8eea into master Mar 29, 2022

qmonnet deleted the pr/jibi/sync-node-labels-to-ciliumnode-obj branch March 29, 2022 09:50

qmonnet mentioned this pull request Mar 30, 2022

v1.11 backports 2022-03-30 #19277

Merged

18 tasks

qmonnet added backport-pending/1.11 and removed needs-backport/1.11 labels Mar 30, 2022

christarazi mentioned this pull request Apr 1, 2022

BGP Control Plane #18860

Merged

pchaigno added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Apr 4, 2022

joestringer mentioned this pull request Apr 15, 2022

Prepare for release v1.11.4 #19462

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

k8s: keep CiliumNode labels synced with Node object #18609

k8s: keep CiliumNode labels synced with Node object #18609

Uh oh!

jibi commented Jan 25, 2022

Uh oh!

aanm left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aanm left a comment

Uh oh!

Uh oh!

jibi commented Mar 28, 2022

Uh oh!

aanm Mar 28, 2022

Uh oh!

jibi Mar 28, 2022 •

edited

Loading

Uh oh!

jibi commented Mar 28, 2022

Uh oh!

Uh oh!

k8s: keep CiliumNode labels synced with Node object #18609

k8s: keep CiliumNode labels synced with Node object #18609

Uh oh!

Conversation

jibi commented Jan 25, 2022

Uh oh!

aanm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aanm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jibi commented Mar 28, 2022

Uh oh!

aanm Mar 28, 2022

Choose a reason for hiding this comment

Uh oh!

jibi Mar 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jibi commented Mar 28, 2022

Uh oh!

Uh oh!

aanm left a comment •

edited

Loading

jibi Mar 28, 2022 •

edited

Loading