From 8ca74c898f1b1a329fc31aa795f3bbf498120e64 Mon Sep 17 00:00:00 2001 From: Serghei Anicheev Date: Tue, 5 Jan 2021 17:38:41 +1100 Subject: [PATCH 1/3] aws-node does not perform ip unassignment properly Sometimes aws-node pod from aws-node daemonset does not perform unassignment of non-existing docker containers. This commit marks IPAMKey data structures where ContainerID is empty as unassigned. Example of ipamd output: "192.168.9.204": { "Address": "192.168.9.204", "IPAMKey": { "containerID": "d74e85090b40c25325b82289ad335287ab9467784ea4014052e49f3032c4e9e8", "ifName": "unknown", "networkName": "_migrated-from-cri" }, "UnassignedTime": "0001-01-01T00:00:00Z" }, "192.168.9.25": { "Address": "192.168.9.25", "IPAMKey": { "containerID": "", "ifName": "", "networkName": "" }, "UnassignedTime": "0001-01-01T00:00:00Z" }, "192.168.9.46": { "Address": "192.168.9.46", "IPAMKey": { "containerID": "", "ifName": "", "networkName": "" }, "UnassignedTime": "0001-01-01T00:00:00Z" }, Eventually we are facing situation when we have free IP addresses but because ipamd thinks it is still assigned we are not able to add new containers. After we re-deploy aws-node daemonset everything is back to normal again for couple of days. --- pkg/ipamd/datastore/data_store.go | 16 +++++++++++++--- pkg/ipamd/datastore/data_store_test.go | 7 ++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/pkg/ipamd/datastore/data_store.go b/pkg/ipamd/datastore/data_store.go index 962be14fde..81a70e18c6 100644 --- a/pkg/ipamd/datastore/data_store.go +++ b/pkg/ipamd/datastore/data_store.go @@ -123,11 +123,18 @@ type IPAMKey struct { IfName string `json:"ifName"` } -// IsZero returns true iff object is equal to the golang zero/null value. +// IsZero returns true if object is equal to the golang zero/null value. func (k IPAMKey) IsZero() bool { return k == IPAMKey{} } +// NoContainer returns true if IPAMKey exists but there is not ContainerID associated +// Eventually it should be t // should be processed by tryUnassignIPsFromAll in ipamd.go +// Which will decrease DS usage counter +func (k IPAMKey) NoContainer() bool { + return k.ContainerID == "" +} + // String() implements the fmt.Stringer interface. func (k IPAMKey) String() string { return fmt.Sprintf("%s/%s/%s", k.NetworkName, k.ContainerID, k.IfName) @@ -178,9 +185,12 @@ func (e *ENI) AssignedIPv4Addresses() int { return count } -// Assigned returns true iff the address is allocated to a pod/sandbox. +// Assigned returns true if the address is allocated to a pod/sandbox. func (addr AddressInfo) Assigned() bool { - return !addr.IPAMKey.IsZero() + if addr.IPAMKey.IsZero() { + return false + } + return !addr.IPAMKey.NoContainer() } // InCoolingPeriod checks whether an addr is in addressCoolingPeriod diff --git a/pkg/ipamd/datastore/data_store_test.go b/pkg/ipamd/datastore/data_store_test.go index 9200b47cfb..a7a300d2dc 100644 --- a/pkg/ipamd/datastore/data_store_test.go +++ b/pkg/ipamd/datastore/data_store_test.go @@ -300,10 +300,15 @@ func TestPodIPv4Address(t *testing.T) { key4 := IPAMKey{"net0", "sandbox-4", "eth0"} _, _, err = ds.AssignPodIPv4Address(key4) assert.Error(t, err) + // Unassign unknown Pod _, _, err = ds.UnassignPodIPv4Address(key4) assert.Error(t, err) + // IPAddress not associated with any container should be considered freeable + ds.eniPool["eni-1"].IPv4Addresses["1.1.1.2"].IPAMKey.ContainerID = "" + assert.Equal(t, ds.eniPool["eni-1"].AssignedIPv4Addresses(), 1) + _, deviceNum, err := ds.UnassignPodIPv4Address(key2) assert.NoError(t, err) assert.Equal(t, ds.total, 3) @@ -311,7 +316,7 @@ func TestPodIPv4Address(t *testing.T) { assert.Equal(t, deviceNum, pod1Ns2Device) assert.Equal(t, len(ds.eniPool["eni-2"].IPv4Addresses), 1) assert.Equal(t, ds.eniPool["eni-2"].AssignedIPv4Addresses(), 0) - assert.Equal(t, len(checkpoint.Data.(*CheckpointData).Allocations), 2) + assert.Equal(t, len(checkpoint.Data.(*CheckpointData).Allocations), 1) noWarmIPTarget := 0 noMinimumIPTarget := 0 From 03c53108b772a01436a15ecfdd90c480d9ba589f Mon Sep 17 00:00:00 2001 From: Serghei Anicheev Date: Tue, 5 Jan 2021 18:14:53 +1100 Subject: [PATCH 2/3] Applying correct code format --- pkg/ipamd/datastore/data_store.go | 2 +- pkg/ipamd/datastore/data_store_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ipamd/datastore/data_store.go b/pkg/ipamd/datastore/data_store.go index 81a70e18c6..1aa21c1acc 100644 --- a/pkg/ipamd/datastore/data_store.go +++ b/pkg/ipamd/datastore/data_store.go @@ -132,7 +132,7 @@ func (k IPAMKey) IsZero() bool { // Eventually it should be t // should be processed by tryUnassignIPsFromAll in ipamd.go // Which will decrease DS usage counter func (k IPAMKey) NoContainer() bool { - return k.ContainerID == "" + return k.ContainerID == "" } // String() implements the fmt.Stringer interface. diff --git a/pkg/ipamd/datastore/data_store_test.go b/pkg/ipamd/datastore/data_store_test.go index a7a300d2dc..c361fdf440 100644 --- a/pkg/ipamd/datastore/data_store_test.go +++ b/pkg/ipamd/datastore/data_store_test.go @@ -305,7 +305,7 @@ func TestPodIPv4Address(t *testing.T) { _, _, err = ds.UnassignPodIPv4Address(key4) assert.Error(t, err) - // IPAddress not associated with any container should be considered freeable + // IPAddress not associated with any container should be considered freeable ds.eniPool["eni-1"].IPv4Addresses["1.1.1.2"].IPAMKey.ContainerID = "" assert.Equal(t, ds.eniPool["eni-1"].AssignedIPv4Addresses(), 1) From 6ffec583c3050ac8cd22c1f931afc629c4d07630 Mon Sep 17 00:00:00 2001 From: Serghei Anicheev Date: Tue, 5 Jan 2021 18:21:43 +1100 Subject: [PATCH 3/3] Fixing method description --- pkg/ipamd/datastore/data_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ipamd/datastore/data_store.go b/pkg/ipamd/datastore/data_store.go index 1aa21c1acc..a12242f62f 100644 --- a/pkg/ipamd/datastore/data_store.go +++ b/pkg/ipamd/datastore/data_store.go @@ -129,7 +129,7 @@ func (k IPAMKey) IsZero() bool { } // NoContainer returns true if IPAMKey exists but there is not ContainerID associated -// Eventually it should be t // should be processed by tryUnassignIPsFromAll in ipamd.go +// Eventually it should be processed by tryUnassignIPsFromAll in ipamd.go // Which will decrease DS usage counter func (k IPAMKey) NoContainer() bool { return k.ContainerID == ""