This allows the cni to return a previous allocation
for a pod with the same podRef and interface name. This
is needed on networks with limited IPs.
Signed-off-by: Marcelo Guerrero <marguerr@redhat.com>
Move everything related to watching / updating the configuration to a
separate pkg. That makes the code easier to follow, and allows us to
properly unit test watching the configuration.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
This change will allow us to unit test the configuration updates without
having to create the CNI configuration files (they're unrelated to this
unit).
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Refactor both IterateForAssignment and GetIPRange.
Make sure that IterateForAssignment adheres to a consistent set of
rules for both IPv4 and IPv6:
* Valid IPs are contained within the ipnet, excluding the network and
broadcast address
* If rangeStart is specified, it is respected if it lies within the
ipnet
* If rangeEnd is specified, it is respected if it lies within the
ipnet and if it is >= rangeStart
* reserveList holds a list of reserved IPs.
* excludeRanges holds a list of subnets to be excluded (meaning the
full subnet, including the network and broadcast IP)
Add more unit tests to make sure the above conditions are met.
Signed-off-by: Andreas Karis <ak.karis@gmail.com>
The podInformerFactory uses filter key spec.nodeName to filter the pods
that it should monitor. Up until now, this filter was set to the value
of HOSTNAME. However, this is not reliable, as spec.nodeName can be
overridden in kubernetes with --hostname-override and thus HOSTNAME and
spec.nodeName do not necessarily always match. Instead, rely on a new
custom environment variable NODENAME which is populated by the downward
API.
Signed-off-by: Andreas Karis <ak.karis@gmail.com>
Update 'allocate.AssignIP' to use 'RangeConfiguration' instead of
'IPAMConfig'. It makes no change in the IP assignment logic.
Update Kubernetes' IP Management to handle multiple IP ranges present in
'IPAM.IPRanges' and return a list for new IPs instead of single new IP.
Update whereabouts' logic to assign multiple new IPs to a pod based on a
list of new IPs returned by IP Management module rather than a single
new IP address.
Update 'net.IPNet' to '[]net.IPNet' in return parameters of
'garbageCollector' to match with the changes in definition of
'wbclient.IPManagement'.
Removed old "Range" from all the places and use IPRanges' 1st element
instead of it
Signed-off-by: Ayush Patel <patel.ayush08@gmail.com>
Some tests that didn't make sense anymore were deleted, while some
were ported to the `config` pkg unit tests.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
We will now rely on the fake kubernetes clients to hold the state of the
pools / pods / etc, instead of the testing `controller-runtime` objects.
Thus, we need to perform dependency injection to inject the "correct"
K8s client to the CNI cmd ADD / DEL / CHECK functions.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
This prepares the ground for a coordinated shutdown of the reconciler
worker queue triggered from the main reconciler func.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Kubebuilder's API server is now serving via an https endpoint; rather
than figuring out how to build a suitable kubeconfig for that, copy the
one the `testEnv` is generating.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* build: generate ip pool clientSet/informers/listers
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* vendor: update vendor stuff
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* build: vendor net-attach-def-client types
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* config: look for the whereabouts config file in multiple places
The reconciler controller will have access to the whereabouts
configuration via a mount point. As such, we need a way to specify its
path.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* reconcile-loop: requires the IP ranges in normalized format
The IP reconcile loop also requires the IP ranges in a normalized
format; as such, we export it into a function, which will be used in a
follow-up commit.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* config: allow IPAM config parsing from a NetConfList
Currently whereabouts is only able to parse network configurations in
the strict [0] format - i.e. **do not accept** a plugin list - [1].
The `ip-control-loop` must recover the full plugin configuration, which
may be in the network configuration format.
This commit allows whereabouts to now understand both formats.
Furthermore, the current CNI release - v1.0.Z - removed the support for
[0], meaning that only the configuration list format is now supported
[2].
[0] - https://github.com/containernetworking/cni/blob/v0.8.1/SPEC.md#network-configuration
[1] - https://github.com/containernetworking/cni/blob/v0.8.1/SPEC.md#network-configuration-lists
[2] - https://github.com/containernetworking/cni/blob/master/SPEC.md#released-versions
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* reconcile-loop: add a controller
Listen to pod deletion, and for every deleted pod, assure their IPs
are gone.
The rough algorithm goes like this:
- for every network-status in the pod's annotations:
- read associated net-attach-def from the k8s API
- extract the range from the net-attach-def
- find the corresponding IP pool
- look for allocations belonging to the deleted pod
- delete them using `IPManagement(..., types.Deallocate, ...)`
All the API reads go through the informer cache, which is kept updated
whenever the objects are updated on the API.
The dockerfiles are also updated, to ship this new binary.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* e2e tests: remove manual cluster reconciliation
This would leave the `ip-control-loop` as the reconciliation tool.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* unit tests: assure stale IPAllocation cleanup
This commit adds a unit where it is checked that the pod deletion leads
to the cleanup of a stale IP address.
This commit features the automatic provisioning of the controller informer cache
with the data present on the fake clientset tracker (the "fake" datastore).
This way, users can just create the client with provisioned data, and
that'll trickle down to the informer cache of the pod controller.
Because the `network-attachment-definitions` resources feature dashes,
the heuristic function that guesses - yes, guesses. very deterministic
... - the name of the resource can't be used - [0]. As such, it was
needed to create an alternate `newFakeNetAttachDefClient` where it is
possible to specify the correct resource name.
[0] - 2fd7267afc/vendor/k8s.io/client-go/testing/fixture.go (L331)
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* unit tests: move helper funcs to other files
The helper files are tagged with the `test` build tag, to prevent them
from being shipped on the production code binary.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* control loop, queueing: use a rate-limiting queue
Using a queue allows us to re-queue errors.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* control loop: add IPAllocation cleanup related events
Adds two new events related to garbage collection of the whereabouts IP
addresses:
- when an IP address is garbage collected
- when a cleanup operation fails and is not re-queued
The former event looks like:
```
116s Normal IPAddressGarbageCollected pod/macvlan1-worker1 \
successful cleanup of IP address [192.168.2.1] from network \
whereabouts-conf
```
The latter event looks like:
```
10s Warning IPAddressGarbageCollectionFailed failed to garbage \
collect addresses for pod default/macvlan1-worker1
```
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* e2e tests: check out statefulset scenarios
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* e2e tests: test different scale up/down order and instance deltas
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* ci: test e2e bash scripts last
These ugly tests do not cleanup after themselves; this way, the golang
based tests (which **do** cleanup after themselves) will not be impacted by
these left-overs.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* ip control loop, unit tests: test negative scenarios
Check the event thrown when a request is dropped from the queue, and
assure reconciling an allocation is impossible without having access to
the attachment configuration data.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* e2e tests: test fix for issue #182
Issue [0] reports an error when a pod associated to a `StatefulSet`
whose IPPool is already full is deleted. According to it, the new pod -
scheduled by the `StatefulSet` - cannot run because the IPPool is
already full, and the old pod's IP cannot be garbage collected because
we match by pod reference - and the "new" pod is stuck in `creating`
phase.
[0] - https://github.com/k8snetworkplumbingwg/whereabouts/issues/182
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* ip-control-loop: strip pod before queueing it
The ip reconcile loop only requires the pod metadata and its network
status annotatations to garbage collect the stale IP addresses.
As such, we remove the status and spec parameters from the pod before
queueing it.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
* reconcile-loop: focus on networks w/ whereabouts IPAM type
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
These are the errors listed on the first `staticcheck` run:
```
+ staticcheck ./...
cmd/whereabouts.go:64:10: error strings should not be capitalized (ST1005)
cmd/whereabouts_test.go:613:17: unnecessary use of fmt.Sprintf (S1039)
pkg/allocate/allocate.go:59:28: error strings should not be capitalized (ST1005)
pkg/allocate/allocate.go:94:3: should merge variable declaration with assignment on next line (S1021)
pkg/allocate/allocate.go:266:20: error strings should not be capitalized (ST1005)
pkg/allocate/allocate_test.go:169:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:181:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:195:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:208:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:224:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:239:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:262:3: this value of err is never used (SA4006)
pkg/allocate/allocate_test.go:277:3: this value of err is never used (SA4006)
pkg/allocate/allocate_test.go:292:3: this value of err is never used (SA4006)
pkg/allocate/allocate_test.go:309:3: this value of err is never used (SA4006)
pkg/allocate/allocate_test.go:324:3: this value of err is never used (SA4006)
pkg/allocate/allocate_test.go:331:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:345:3: this value of ip is never used (SA4006)
pkg/allocate/allocate_test.go:359:3: this value of badip is never used (SA4006)
pkg/allocate/allocate_test.go:369:3: this value of badip is never used (SA4006)
pkg/config/config.go:73:21: error strings should not be capitalized (ST1005)
pkg/config/config.go:163:20: error strings should not be capitalized (ST1005)
pkg/reconciler/iploop_test.go:155:6: func generatePodRef is unused (U1000)
pkg/storage/etcd.go:175:17: error strings should not be capitalized (ST1005)
pkg/storage/kubernetes/client.go:112:2: should replace loop with podEntries = append(podEntries, podList.Items...) (S1011)
pkg/storage/kubernetes/client.go:125:2: should replace loop with clusterWiderReservations = append(clusterWiderReservations, overlappingIPsList.Items...) (S1011)
pkg/storage/kubernetes/ipam.go:415:17: error strings should not be capitalized (ST1005)
pkg/storage/kubernetes/ipam.go:499:7: should omit comparison to bool constant, can be simplified to !rl.IsAllocated (S1002)
```
All of these were fixed in this commit.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
These contexts are all cancellable, and have a timeout.
The timeout can be specified via the reconciler command line arguments,
and defaults to 30 seconds.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Follow the golang convention regarding `context.Context` - [0]
According to it:
"""
Do not store Contexts inside a struct type; instead, pass a Context explicitly
to each function that needs it. The Context should be the first parameter,
typically named ctx:
...
"""
[0] - https://pkg.go.dev/context
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
With this change the reconciler can run in two different ways:
- when ran in a k8s pod, it does not require to be told how to
connect to the cluster.
- when ran as a binary, it does require to know how to connect to
the cluster, via the -kubeconfig config option.
The reconciler cron spec is updated to use the correct service
account name, and also is updated to run in the `kube-system`
namespace.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
The ip-reconciler tool currently feature the logging level
hard-coded to `debug`, which is too verbose.
This commit makes it configurable, defaulting it to error level.
The effective logging level is set to `verbose` in the daemonset
spec.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
During scale testing, we found that if an instance
of whereabouts did not accomplish its task within a
period of time, the kubelet spawned an additional
whereabouts instance and the initial instance
remained active.
For pod creation, the Kubelet will try to
start a pod and wait for ~8 minutes before
attempting to kill it. See kubelets "runPod"
function.
For pod deletion, kubelet will attempt to kill a pod
and if it fails it will attempt after one minute one
more time. Unable to see this in kubelet code but
observed it using K8 1.22.
Signed-off-by: Martin Kennelly <mkennell@redhat.com>
The hack/build-go.sh script is modified to pass version information
during the build through LDFLAGS, which is a pretty standard technique.
This information can then be used to report the version of the
whereabouts CNI plugin at runtime; before this change the plugin would
just print "TODO".
By not returning an `IPReservation` entry - we now return an IP
address instead - we simplify the code.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Only mark a pod as "alive" when the pod's annotations feature the
IP being de-allocated.
This makes the reconciler binary *dependent* on multus, which adds
these `network-status` annotations into the pod.
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
