Skip to content

Commit a08a9de

Browse files
yash97yash97
authored
cluster eni cleaner will use cluster tag filter of vpc-cni and vpc-rc (#550)
* keeping cluster tag same as vpc-cni * adding OR filter which will use vpc-rc and vpc-cni tag
1 parent 9b48a15 commit a08a9de

File tree

10 files changed

+104
-44
lines changed

10 files changed

+104
-44
lines changed

‎controllers/crds/cninode_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,13 @@ func (r *CNINodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
128128
shouldPatch := false
129129
cniNodeCopy := cniNode.DeepCopy()
130130
// Add cluster name tag if it does not exist
131-
val, ok := cniNode.Spec.Tags[config.CNINodeClusterNameKey]
131+
val, ok := cniNode.Spec.Tags[config.VPCCNIClusterNameKey]
132132
if !ok || val != r.clusterName {
133133
if len(cniNodeCopy.Spec.Tags) != 0 {
134-
cniNodeCopy.Spec.Tags[config.CNINodeClusterNameKey] = r.clusterName
134+
cniNodeCopy.Spec.Tags[config.VPCCNIClusterNameKey] = r.clusterName
135135
} else {
136136
cniNodeCopy.Spec.Tags = map[string]string{
137-
config.CNINodeClusterNameKey: r.clusterName,
137+
config.VPCCNIClusterNameKey: r.clusterName,
138138
}
139139
}
140140
shouldPatch = true

‎controllers/crds/cninode_controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func TestCNINodeReconcile(t *testing.T) {
8989
assert.NoError(t, err)
9090
assert.Equal(t, res, reconcile.Result{})
9191
assert.Equal(t, cniNode.Labels, map[string]string{config.NodeLabelOS: "linux"})
92-
assert.Equal(t, cniNode.Spec.Tags, map[string]string{config.CNINodeClusterNameKey: mockClusterName})
92+
assert.Equal(t, cniNode.Spec.Tags, map[string]string{config.VPCCNIClusterNameKey: mockClusterName})
9393
},
9494
},
9595
}

‎pkg/aws/ec2/api/cleanup/eni_cleanup.go‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
2424
rcHealthz "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/healthz"
2525
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils"
26-
"github.com/samber/lo"
2726

2827
ec2Errors "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/errors"
2928
"github.com/aws/aws-sdk-go-v2/aws"
@@ -38,6 +37,7 @@ import (
3837
// NetworkInterfaceManager interface allows to define the ENI filters and checks if ENI should be deleted for different callers like in the periodic cleanup routine or
3938
// during node termination
4039
type NetworkInterfaceManager interface {
40+
// If there are multiple filters then we will OR them.
4141
GetENITagFilters() []ec2types.Filter
4242
ShouldDeleteENI(eniID *string) bool
4343
UpdateAvailableENIsIfNeeded(eniMap *map[string]struct{})
@@ -126,18 +126,36 @@ func (e *ENICleaner) DeleteLeakedResources() error {
126126
}...)
127127

128128
// only apply extra filters when the controller is enabled which provides cninode resources
129+
var OrFilters []ec2types.Filter
130+
var err error
131+
var networkInterfaces []*ec2types.NetworkInterface
129132
if !e.ControllerDisabled {
130133
// get cleaner specific filters
131-
filters = append(filters, e.Manager.GetENITagFilters()...)
132-
}
133-
describeNetworkInterfaceIp := &ec2.DescribeNetworkInterfacesInput{
134-
Filters: filters,
135-
}
134+
OrFilters = e.Manager.GetENITagFilters()
135+
for _, OrFilter := range OrFilters {
136+
filterCopy := append([]ec2types.Filter{}, filters...)
137+
filterCopy = append(filterCopy, OrFilter)
138+
139+
describeNetworkInterfaceIp := &ec2.DescribeNetworkInterfacesInput{
140+
Filters: filterCopy,
141+
}
136142

137-
networkInterfaces, err := e.EC2Wrapper.DescribeNetworkInterfacesPagesWithRetry(describeNetworkInterfaceIp)
138-
if err != nil {
139-
e.Log.Error(err, "failed to describe network interfaces, cleanup will be retried in next cycle")
140-
return err
143+
tempNetworkInterfaces, err := e.EC2Wrapper.DescribeNetworkInterfacesPagesWithRetry(describeNetworkInterfaceIp)
144+
if err != nil {
145+
e.Log.Error(err, "failed to describe network interfaces, cleanup will be retried in next cycle")
146+
return err
147+
}
148+
networkInterfaces = append(networkInterfaces, tempNetworkInterfaces...)
149+
}
150+
} else {
151+
describeNetworkInterfaceIp := &ec2.DescribeNetworkInterfacesInput{
152+
Filters: filters,
153+
}
154+
networkInterfaces, err = e.EC2Wrapper.DescribeNetworkInterfacesPagesWithRetry(describeNetworkInterfaceIp)
155+
if err != nil {
156+
e.Log.Error(err, "failed to describe network interfaces, cleanup will be retried in next cycle")
157+
return err
158+
}
141159
}
142160

143161
for _, nwInterface := range networkInterfaces {
@@ -169,9 +187,12 @@ func (e *ENICleaner) DeleteLeakedResources() error {
169187
}
170188
continue
171189
}
172-
e.Log.Info("deleted leaked ENI successfully",
173-
"eniID", nwInterface.NetworkInterfaceId,
174-
"instanceID", lo.TernaryF(nwInterface.Attachment == nil, func() *string { return lo.ToPtr("") }, func() *string { return nwInterface.Attachment.InstanceId }))
190+
// It is possible for eni attachment to be nil, if it was never attached to instance
191+
instanceID := ""
192+
if nwInterface.Attachment != nil && nwInterface.Attachment.InstanceId != nil {
193+
instanceID = aws.ToString(nwInterface.Attachment.InstanceId)
194+
}
195+
e.Log.Info("deleted leaked ENI successfully", "eni id", *nwInterface.NetworkInterfaceId, "instance id", instanceID)
175196
} else {
176197
// Seeing the ENI for the first time, add it to the new list of available network interfaces
177198
availableENIs[*nwInterface.NetworkInterfaceId] = struct{}{}
@@ -184,11 +205,14 @@ func (e *ENICleaner) DeleteLeakedResources() error {
184205
}
185206

186207
func (e *ClusterENICleaner) GetENITagFilters() []ec2types.Filter {
187-
clusterNameTagKey := fmt.Sprintf(config.ClusterNameTagKeyFormat, e.ClusterName)
188208
return []ec2types.Filter{
189209
{
190-
Name: aws.String("tag:" + clusterNameTagKey),
191-
Values: []string{config.ClusterNameTagValue},
210+
Name: aws.String("tag:" + config.VPCCNIClusterNameKey),
211+
Values: []string{e.ClusterName},
212+
},
213+
{
214+
Name: aws.String("tag:" + fmt.Sprintf(config.VPCRCClusterNameTagKeyFormat, e.ClusterName)),
215+
Values: []string{config.VPCRCClusterNameTagValue},
192216
},
193217
}
194218
}

‎pkg/aws/ec2/api/cleanup/eni_cleanup_test.go‎

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import (
3333
var (
3434
mockClusterName = "cluster-name"
3535
mockNodeID = "i-00000000000000001"
36-
mockClusterNameTagKey = fmt.Sprintf(config.ClusterNameTagKeyFormat, mockClusterName)
36+
mockClusterNameTagKey = config.VPCCNIClusterNameKey
3737

3838
mockNetworkInterfaceId1 = "eni-000000000000000"
3939
mockNetworkInterfaceId2 = "eni-000000000000001"
@@ -60,7 +60,7 @@ var (
6060
},
6161
{
6262
Name: aws.String("tag:" + mockClusterNameTagKey),
63-
Values: []string{config.ClusterNameTagValue},
63+
Values: []string{mockClusterName},
6464
},
6565
},
6666
}
@@ -144,16 +144,53 @@ func TestENICleaner_DeleteLeakedResources(t *testing.T) {
144144
return mockClusterENICleaner.ENICleaner, mockClusterENICleaner
145145
},
146146
prepare: func(f *fields) {
147+
commonFilters := CommonNetworkInterfaceFilters
148+
vpcFilter := ec2types.Filter{
149+
Name: aws.String("vpc-id"),
150+
Values: []string{mockVpcId},
151+
}
152+
153+
firstOrFilter := ec2types.Filter{
154+
Name: aws.String("tag:" + config.VPCCNIClusterNameKey),
155+
Values: []string{mockClusterName},
156+
}
157+
158+
secondOrFilter := ec2types.Filter{
159+
Name: aws.String("tag:" + fmt.Sprintf(config.VPCRCClusterNameTagKeyFormat, mockClusterName)),
160+
Values: []string{config.VPCRCClusterNameTagValue},
161+
}
162+
163+
filtersWithFirstTag := append(append(commonFilters, vpcFilter), firstOrFilter)
164+
filtersWithSecondTag := append(append(commonFilters, vpcFilter), secondOrFilter)
147165
gomock.InOrder(
148-
// Return network interface 1 and 2 in first cycle
149-
f.mockEC2Wrapper.EXPECT().DescribeNetworkInterfacesPagesWithRetry(mockClusterTagInput).
150-
Return(NetworkInterfacesWith1And2, nil),
166+
f.mockEC2Wrapper.EXPECT().DescribeNetworkInterfacesPagesWithRetry(
167+
&ec2.DescribeNetworkInterfacesInput{
168+
Filters: filtersWithFirstTag,
169+
},
170+
).Return(NetworkInterfacesWith1And2, nil),
171+
172+
f.mockEC2Wrapper.EXPECT().DescribeNetworkInterfacesPagesWithRetry(
173+
&ec2.DescribeNetworkInterfacesInput{
174+
Filters: filtersWithSecondTag,
175+
},
176+
).Return([]*ec2types.NetworkInterface{}, nil),
177+
151178
// Return network interface 1 and 3 in the second cycle
152-
f.mockEC2Wrapper.EXPECT().DescribeNetworkInterfacesPagesWithRetry(mockClusterTagInput).
153-
Return(NetworkInterfacesWith1And3, nil),
154-
// Expect to delete the network interface 1
179+
f.mockEC2Wrapper.EXPECT().DescribeNetworkInterfacesPagesWithRetry(
180+
&ec2.DescribeNetworkInterfacesInput{
181+
Filters: filtersWithFirstTag,
182+
},
183+
).Return(NetworkInterfacesWith1And3, nil),
184+
185+
f.mockEC2Wrapper.EXPECT().DescribeNetworkInterfacesPagesWithRetry(
186+
&ec2.DescribeNetworkInterfacesInput{
187+
Filters: filtersWithSecondTag,
188+
},
189+
).Return([]*ec2types.NetworkInterface{}, nil),
190+
155191
f.mockEC2Wrapper.EXPECT().DeleteNetworkInterface(
156-
&ec2.DeleteNetworkInterfaceInput{NetworkInterfaceId: &mockNetworkInterfaceId1}).Return(nil, nil),
192+
&ec2.DeleteNetworkInterfaceInput{NetworkInterfaceId: &mockNetworkInterfaceId1},
193+
).Return(nil, nil),
157194
)
158195
},
159196
assertFirstCall: func(f *fields) {

‎pkg/aws/ec2/api/helper.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ func NewEC2APIHelper(ec2Wrapper EC2Wrapper, clusterName string) EC2APIHelper {
6969
// Set the key and value of the cluster name tag which will be used to tag all the network interfaces created by
7070
// the controller
7171
clusterNameTag = ec2types.Tag{
72-
Key: aws.String(fmt.Sprintf(config.ClusterNameTagKeyFormat, clusterName)),
73-
Value: aws.String(config.ClusterNameTagValue),
72+
Key: aws.String(fmt.Sprintf(config.VPCRCClusterNameTagKeyFormat, clusterName)),
73+
Value: aws.String(config.VPCRCClusterNameTagValue),
7474
}
7575
return &ec2APIHelper{ec2Wrapper: ec2Wrapper}
7676
}

‎pkg/aws/ec2/api/helper_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ var (
8383
}
8484

8585
defaultClusterNameTag = ec2types.Tag{
86-
Key: aws.String(fmt.Sprintf(config.ClusterNameTagKeyFormat, clusterName)),
87-
Value: aws.String(config.ClusterNameTagValue),
86+
Key: aws.String(fmt.Sprintf(config.VPCRCClusterNameTagKeyFormat, clusterName)),
87+
Value: aws.String(config.VPCRCClusterNameTagValue),
8888
}
8989

9090
createNetworkInterfaceInput = &ec2.CreateNetworkInterfaceInput{

‎pkg/aws/ec2/api/wrapper.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,7 @@ func (e *ec2Wrapper) DescribeNetworkInterfacesPagesWithRetry(input *ec2.Describe
761761
attemptInterfaces = append(attemptInterfaces, &ec2types.NetworkInterface{
762762
NetworkInterfaceId: nwInterface.NetworkInterfaceId,
763763
TagSet: nwInterface.TagSet,
764+
Attachment: nwInterface.Attachment,
764765
})
765766
}
766767

‎pkg/config/type.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,16 @@ const (
5757

5858
// EC2 Tags
5959
const (
60-
ControllerTagPrefix = "vpcresources.k8s.aws/"
61-
VLandIDTag = ControllerTagPrefix + "vlan-id"
62-
TrunkENIIDTag = ControllerTagPrefix + "trunk-eni-id"
63-
64-
ClusterNameTagKeyFormat = "kubernetes.io/cluster/%s"
65-
ClusterNameTagValue = "owned"
66-
60+
ControllerTagPrefix = "vpcresources.k8s.aws/"
61+
VLandIDTag = ControllerTagPrefix + "vlan-id"
62+
TrunkENIIDTag = ControllerTagPrefix + "trunk-eni-id"
63+
VPCRCClusterNameTagKeyFormat = "kubernetes.io/cluster/%s"
64+
VPCRCClusterNameTagValue = "owned"
6765
NetworkInterfaceOwnerTagKey = "eks:eni:owner"
6866
NetworkInterfaceOwnerTagValue = "eks-vpc-resource-controller"
6967
NetworkInterfaceOwnerVPCCNITagValue = "amazon-vpc-cni"
7068
NetworkInterfaceNodeIDKey = "node.k8s.amazonaws.com/instance_id"
71-
CNINodeClusterNameKey = "cluster.k8s.amazonaws.com/name"
69+
VPCCNIClusterNameKey = "cluster.k8s.amazonaws.com/name"
7270
)
7371

7472
const (
@@ -129,7 +127,7 @@ var (
129127
// CoolDownPeriod is the time to let kube-proxy propagates IP tables rules before assigning the resource back to new pod
130128
CoolDownPeriod = time.Second * 30
131129
// ENICleanUpInterval is the time interval between each dangling ENI clean up task
132-
ENICleanUpInterval = time.Minute * 30
130+
ENICleanUpInterval = time.Minute * 2
133131
)
134132

135133
// ResourceConfig is the configuration for each resource type

‎pkg/k8s/wrapper.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (k *k8sWrapper) CreateCNINode(node *v1.Node, clusterName string) error {
260260
},
261261
Spec: rcv1alpha1.CNINodeSpec{
262262
Tags: map[string]string{
263-
fmt.Sprintf(config.CNINodeClusterNameKey): clusterName,
263+
fmt.Sprintf(config.VPCCNIClusterNameKey): clusterName,
264264
},
265265
},
266266
}

‎test/integration/cninode/cninode_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func VerifyCNINodeFields(cniNode *v1alpha1.CNINode) {
190190
// For maps, ContainElement searches through the map's values.
191191
By("verifying cluster name tag is set")
192192
Expect(cniNode.Spec.Tags).To(ContainElement(frameWork.Options.ClusterName))
193-
Expect(config.CNINodeClusterNameKey).To(BeKeyOf(cniNode.Spec.Tags))
193+
Expect(config.VPCCNIClusterNameKey).To(BeKeyOf(cniNode.Spec.Tags))
194194

195195
By("verifying node OS label is set")
196196
Expect(cniNode.ObjectMeta.Labels).To(ContainElement(config.OSLinux))

0 commit comments

Comments
 (0)