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

Prepare for GCP IPAM #10691

Merged
merged 9 commits into from Mar 26, 2020
Merged

Prepare for GCP IPAM #10691

merged 9 commits into from Mar 26, 2020

Conversation

tgraf
Copy link
Member

@tgraf tgraf commented Mar 24, 2020

No description provided.

@tgraf tgraf added wip release-note/misc This PR makes changes that have no direct user impact. sig/ipam IP address management, including cloud IPAM labels Mar 24, 2020
@tgraf tgraf requested review from a team as code owners March 24, 2020 23:28
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 24, 2020
@tgraf tgraf force-pushed the pr/tgraf/gcp-ipam-prep branch 2 times, most recently from 90a5883 to 4b760a4 Compare March 25, 2020 08:34
errordeveloper
errordeveloper reviewed Mar 25, 2020
View reviewed changes
operator/azure.go Outdated Show resolved Hide resolved
@tgraf
ipam: Move iterator logic into generic InstanceMap
5ed4607 
Move code to iterate over interfaces and addresses into the InstanceMap.
This allows to defer deep copying until it is really necessary.

This also allows to move the logic to reserve IPs of all interfaces of
all instances into the generic GroupPoolAllocator for reuse in future
IPAM code.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
ipam: Fix data race in node manager test
99a0a79 
Fixes:
```
WARNING: DATA RACE
Read at 0x00c0001b48c0 by goroutine 37:
  github.com/cilium/cilium/pkg/ipam.(*nodeOperationsMock).PrepareIPAllocation()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node_manager_test.go:103 +0x1c5
  github.com/cilium/cilium/pkg/ipam.(*Node).determineMaintenanceAction()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node.go:492 +0x184
  github.com/cilium/cilium/pkg/ipam.(*Node).maintainIPPool()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node.go:528 +0x53
  github.com/cilium/cilium/pkg/ipam.(*Node).MaintainIPPool()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node.go:613 +0x69
  github.com/cilium/cilium/pkg/ipam.(*NodeManager).Update.func1()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node_manager.go:205 +0x8b
  github.com/cilium/cilium/pkg/trigger.(*Trigger).waiter()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/trigger/trigger.go:206 +0x4b9

Previous write at 0x00c0001b48c0 by goroutine 35:
  github.com/cilium/cilium/pkg/ipam.(*nodeOperationsMock).AllocateIPs()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node_manager_test.go:111 +0x117
  github.com/cilium/cilium/pkg/ipam.(*Node).maintainIPPool()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node.go:564 +0xa0d
  github.com/cilium/cilium/pkg/ipam.(*Node).MaintainIPPool()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node.go:613 +0x69
  github.com/cilium/cilium/pkg/ipam.(*NodeManager).Update.func1()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/ipam/node_manager.go:205 +0x8b
  github.com/cilium/cilium/pkg/trigger.(*Trigger).waiter()
      /home/vagrant/go/src/github.com/cilium/cilium/pkg/trigger/trigger.go:206 +0x4b9
```

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
ipam: Simplify locking in IPAM node interactions
71dd2c3 
Don't hold any locks when calling into IPAM specific operations. Use
node.mutex exclusively to protect data access and keep locking periods
as short as possible.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
ipam: Move instance ID retrieval into generic code
9899eb6 
This removes the need to track the CiliumNode resource in the Azure
specific code entirely.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
ipam: Don't recalculate if resource is not attached yet
74fd199 
There is no point in attempting recalculation of required addresses
unless the resource has been attached.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
ipam/allocator: Allow allocating IPs without knowing pool ID
9b34dd9 
Ideally, the pool ID is known. If not known, the list of pools can be
searched to identify the correct pool.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
operator: Remove global nodeManager variable
9defaaa 
Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
ipam: Add InstanceMap.GetInterface
71f1000 
Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
operator: Move initial sync and background sync in generic code
a1cdb3f 
Signed-off-by: Thomas Graf <thomas@cilium.io>
@coveralls
Copy link

coveralls commented Mar 25, 2020 •
edited

Coverage Status

Coverage increased (+0.08%) to 45.511% when pulling a1cdb3f on pr/tgraf/gcp-ipam-prep into 8fd7415 on master.

@tgraf
Copy link
Member Author

tgraf commented Mar 25, 2020 •
edited

test-me-please

00:49:18.049    Checks CIDR L3 Policy [It]
00:49:18.049    /home/jenkins/workspace/Cilium-PR-Ginkgo-Tests-Validated/runtime-gopath/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:430
00:49:18.049  
00:49:18.049    Unable to import policy: cannot render the policy:  write policy_fe935aaf.json: no space left on device
[2020-03-25T15:17:25.119Z]   Expected
[2020-03-25T15:17:25.119Z]       <*errors.errorString | 0xc0004323f0>: {
[2020-03-25T15:17:25.119Z]           s: "cannot render the policy:  write policy_fe935aaf.json: no space left on device",
[2020-03-25T15:17:25.119Z]       }
[2020-03-25T15:17:25.119Z]   to be nil

@tgraf
Copy link
Member Author

tgraf commented Mar 25, 2020

test-me-please

@tgraf tgraf added pending-review and removed wip labels Mar 26, 2020
@borkmann borkmann merged commit 63055b7 into master Mar 26, 2020
1.8.0 automation moved this from In progress to Merged Mar 26, 2020
@borkmann borkmann deleted the pr/tgraf/gcp-ipam-prep branch March 26, 2020 07:35
@christarazi christarazi mentioned this pull request Jun 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@errordeveloper errordeveloper errordeveloper approved these changes

Assignees
No one assigned
Labels
release-note/misc This PR makes changes that have no direct user impact. sig/ipam IP address management, including cloud IPAM
Projects
No open projects
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tgraf @coveralls @errordeveloper @borkmann