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.

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

Make sure we have space for a trunk ENI #1210

Merged
merged 1 commit into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,6 @@ Default: `""`

Specifies the cluster name to tag allocated ENIs with. See the "Cluster Name tag" section below.


---

`ENABLE_POD_ENI` (Since v1.7.0)
Expand All @@ -422,8 +421,7 @@ Type: Boolean as a String
Default: `false`

To enable security groups for pods you need to have at least an EKS 1.17 eks.3 cluster. Setting `ENABLE_POD_ENI` to `true`
will add the `vpc.amazonaws.com/has-trunk-attached` label to the node, signifying that the feature is enabled.

will add the `vpc.amazonaws.com/has-trunk-attached` label to the node if it is possible to attach an additional ENI.

### ENI tags related to Allocation

Expand Down
6 changes: 5 additions & 1 deletion pkg/ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -827,13 +827,17 @@ func logPoolStats(total, used, maxAddrsPerENI int) {

func (c *IPAMContext) askForTrunkENIIfNeeded() {
if c.enablePodENI && c.dataStore.GetTrunkENI() == "" {
// Check that there is room for a trunk ENI to be attached:
if c.dataStore.GetENIs() >= (c.maxENI - c.unmanagedENI) {
log.Debug("No slot available for a trunk ENI to be attached. Not labeling the node")
return
}
// We need to signal that VPC Resource Controller needs to attach a trunk ENI
err := c.SetNodeLabel("vpc.amazonaws.com/has-trunk-attached", "false")
if err != nil {
podENIErrInc("askForTrunkENIIfNeeded")
log.Errorf("Failed to set node label", err)
}
return
}
}

Expand Down
45 changes: 45 additions & 0 deletions pkg/ipamd/ipamd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -814,5 +814,50 @@ func TestIPAMContext_setupENI(t *testing.T) {
err = mockContext.setupENI(newENIMetadata.ENIID, newENIMetadata, "")
assert.Error(t, err)
assert.Equal(t, 1, len(mockContext.primaryIP))
}

func TestIPAMContext_askForTrunkENIIfNeeded(t *testing.T) {
m := setup(t)
defer m.ctrl.Finish()

mockContext := &IPAMContext{
k8sClient: m.clientset,
dataStore: datastore.NewDataStore(log, datastore.NewTestCheckpoint(datastore.CheckpointData{Version: datastore.CheckpointFormatVersion})),
awsClient: m.awsutils,
networkClient: m.network,
primaryIP: make(map[string]string),
terminating: int32(0),
maxENI: 1,
myNodeName: myNodeName,
}

labels := make(map[string]string)
fakeNode := v1.Node{
TypeMeta: metav1.TypeMeta{Kind: "Node"},
ObjectMeta: metav1.ObjectMeta{Name: myNodeName, Labels: labels},
Spec: v1.NodeSpec{},
Status: v1.NodeStatus{},
}
_, _ = m.clientset.CoreV1().Nodes().Create(&fakeNode)

_ = mockContext.dataStore.AddENI("eni-1", 1, true, false)
// If ENABLE_POD_ENI is not set, nothing happens
mockContext.askForTrunkENIIfNeeded()

mockContext.enablePodENI = true
// Enabled, we should try to set the label if there is room
mockContext.askForTrunkENIIfNeeded()
notUpdatedNode, err := m.clientset.CoreV1().Nodes().Get(myNodeName, metav1.GetOptions{})
// Since there was no room, no label should be added
assert.NoError(t, err)
assert.Equal(t, 0, len(notUpdatedNode.Labels))

mockContext.maxENI = 4
// Now there is room!
mockContext.askForTrunkENIIfNeeded()

// Fetch the updated node and verify that the label is set
updatedNode, err := m.clientset.CoreV1().Nodes().Get(myNodeName, metav1.GetOptions{})
assert.NoError(t, err)
assert.Equal(t, "false", updatedNode.Labels["vpc.amazonaws.com/has-trunk-attached"])
}