Skip to content

Commit 123e023

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3189 from shivi28/scidr
Test cases added for secondary cidr file
2 parents 0eee277 + d60c064 commit 123e023

File tree

2 files changed

+263
-8
lines changed

2 files changed

+263
-8
lines changed

‎pkg/cloud/services/network/secondarycidr.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,18 @@ limitations under the License.
1717
package network
1818

1919
import (
20+
"reflect"
21+
2022
"github.com/aws/aws-sdk-go/service/ec2"
2123
"github.com/pkg/errors"
2224

2325
"sigs.k8s.io/cluster-api-provider-aws/pkg/record"
2426
)
2527

28+
func isVPCPresent(vpcs *ec2.DescribeVpcsOutput) bool {
29+
return vpcs != nil && len(vpcs.Vpcs) > 0
30+
}
31+
2632
func (s *Service) associateSecondaryCidr() error {
2733
if s.scope.SecondaryCidrBlock() == nil {
2834
return nil
@@ -35,8 +41,8 @@ func (s *Service) associateSecondaryCidr() error {
3541
return err
3642
}
3743

38-
if len(vpcs.Vpcs) != 1 {
39-
return errors.Errorf("VPC not found")
44+
if !isVPCPresent(vpcs) {
45+
return errors.Errorf("failed to associateSecondaryCidr as there are no VPCs present")
4046
}
4147

4248
existingAssociations := vpcs.Vpcs[0].CidrBlockAssociationSet
@@ -55,6 +61,7 @@ func (s *Service) associateSecondaryCidr() error {
5561
return err
5662
}
5763

64+
// once IPv6 is supported, we need to modify out.CidrBlockAssociation.AssociationId to out.Ipv6CidrBlockAssociation.AssociationId
5865
record.Eventf(s.scope.InfraCluster(), "SuccessfulAssociateSecondaryCidr", "Associated secondary CIDR with VPC %q", *out.CidrBlockAssociation.AssociationId)
5966

6067
return nil
@@ -72,17 +79,16 @@ func (s *Service) disassociateSecondaryCidr() error {
7279
return err
7380
}
7481

75-
if len(vpcs.Vpcs) != 1 {
76-
return errors.Errorf("VPC not found")
82+
if !isVPCPresent(vpcs) {
83+
return errors.Errorf("failed to associateSecondaryCidr as there are no VPCs present")
7784
}
7885

7986
existingAssociations := vpcs.Vpcs[0].CidrBlockAssociationSet
8087
for _, existing := range existingAssociations {
81-
if existing.CidrBlock == s.scope.SecondaryCidrBlock() {
82-
_, err := s.EC2Client.DisassociateVpcCidrBlock(&ec2.DisassociateVpcCidrBlockInput{
88+
if reflect.DeepEqual(existing.CidrBlock, s.scope.SecondaryCidrBlock()) {
89+
if _, err := s.EC2Client.DisassociateVpcCidrBlock(&ec2.DisassociateVpcCidrBlockInput{
8390
AssociationId: existing.AssociationId,
84-
})
85-
if err != nil {
91+
}); err != nil {
8692
record.Warnf(s.scope.InfraCluster(), "FailedDisassociateSecondaryCidr", "Failed disassociating secondary CIDR with VPC %v", err)
8793
return err
8894
}

‎pkg/cloud/services/network/secondarycidr_test.go

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package network
18+
19+
import (
20+
"testing"
21+
22+
"github.com/aws/aws-sdk-go/aws"
23+
"github.com/aws/aws-sdk-go/service/ec2"
24+
"github.com/golang/mock/gomock"
25+
. "github.com/onsi/gomega"
26+
"k8s.io/apimachinery/pkg/runtime"
27+
"k8s.io/utils/pointer"
28+
"sigs.k8s.io/controller-runtime/pkg/client"
29+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
30+
31+
infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1beta1"
32+
ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/controlplane/eks/api/v1beta1"
33+
"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/awserrors"
34+
"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope"
35+
"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface"
36+
"sigs.k8s.io/cluster-api/api/v1beta1"
37+
)
38+
39+
func setupNewManagedControlPlaneScope(cl client.Client) (*scope.ManagedControlPlaneScope, error) {
40+
return scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{
41+
Client: cl,
42+
Cluster: &v1beta1.Cluster{},
43+
ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{
44+
Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{
45+
SecondaryCidrBlock: pointer.StringPtr("secondary-cidr"),
46+
NetworkSpec: infrav1.NetworkSpec{
47+
VPC: infrav1.VPCSpec{ID: "vpc-id"},
48+
},
49+
},
50+
},
51+
})
52+
}
53+
54+
func setupScheme() (*runtime.Scheme, error) {
55+
scheme := runtime.NewScheme()
56+
if err := ekscontrolplanev1.AddToScheme(scheme); err != nil {
57+
return nil, err
58+
}
59+
return scheme, nil
60+
}
61+
62+
func TestService_associateSecondaryCidr(t *testing.T) {
63+
mockCtrl := gomock.NewController(t)
64+
defer mockCtrl.Finish()
65+
66+
tests := []struct {
67+
name string
68+
haveSecondaryCIDR bool
69+
expect func(m *mock_ec2iface.MockEC2APIMockRecorder)
70+
wantErr bool
71+
}{
72+
{
73+
name: "Should not associate secondary CIDR if no secondary cidr block info present in control plane",
74+
},
75+
{
76+
name: "Should return error if unable to describe VPC",
77+
haveSecondaryCIDR: true,
78+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
79+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(nil, awserrors.NewFailedDependency("dependency-failure"))
80+
},
81+
wantErr: true,
82+
},
83+
{
84+
name: "Should not associate secondary cidr block if already exist in VPC",
85+
haveSecondaryCIDR: true,
86+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
87+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(&ec2.DescribeVpcsOutput{
88+
Vpcs: []*ec2.Vpc{
89+
{
90+
CidrBlockAssociationSet: []*ec2.VpcCidrBlockAssociation{
91+
{CidrBlock: aws.String("secondary-cidr")},
92+
},
93+
},
94+
}}, nil)
95+
},
96+
},
97+
{
98+
name: "Should return error if no VPC found",
99+
haveSecondaryCIDR: true,
100+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
101+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(nil, nil)
102+
},
103+
wantErr: true,
104+
},
105+
{
106+
name: "Should return error if failed during associating secondary cidr block",
107+
haveSecondaryCIDR: true,
108+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
109+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(&ec2.DescribeVpcsOutput{
110+
Vpcs: []*ec2.Vpc{
111+
{
112+
CidrBlockAssociationSet: []*ec2.VpcCidrBlockAssociation{
113+
{CidrBlock: aws.String("secondary-cidr-new")},
114+
},
115+
},
116+
}}, nil)
117+
m.AssociateVpcCidrBlock(gomock.AssignableToTypeOf(&ec2.AssociateVpcCidrBlockInput{})).Return(nil, awserrors.NewFailedDependency("dependency-failure"))
118+
},
119+
wantErr: true,
120+
},
121+
}
122+
for _, tt := range tests {
123+
t.Run(tt.name, func(t *testing.T) {
124+
g := NewWithT(t)
125+
126+
scheme, err := setupScheme()
127+
g.Expect(err).NotTo(HaveOccurred())
128+
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
129+
130+
ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl)
131+
132+
mcpScope, err := setupNewManagedControlPlaneScope(cl)
133+
g.Expect(err).NotTo(HaveOccurred())
134+
135+
if !tt.haveSecondaryCIDR {
136+
mcpScope.ControlPlane.Spec.SecondaryCidrBlock = nil
137+
}
138+
139+
s := NewService(mcpScope)
140+
s.EC2Client = ec2Mock
141+
142+
if tt.expect != nil {
143+
tt.expect(ec2Mock.EXPECT())
144+
}
145+
146+
err = s.associateSecondaryCidr()
147+
if tt.wantErr {
148+
g.Expect(err).To(HaveOccurred())
149+
return
150+
}
151+
g.Expect(err).NotTo(HaveOccurred())
152+
})
153+
}
154+
}
155+
156+
func TestService_diassociateSecondaryCidr(t *testing.T) {
157+
mockCtrl := gomock.NewController(t)
158+
defer mockCtrl.Finish()
159+
160+
tests := []struct {
161+
name string
162+
haveSecondaryCIDR bool
163+
expect func(m *mock_ec2iface.MockEC2APIMockRecorder)
164+
wantErr bool
165+
}{
166+
{
167+
name: "Should not disassociate secondary CIDR if no secondary cidr block info present in control plane",
168+
},
169+
{
170+
name: "Should return error if unable to describe VPC",
171+
haveSecondaryCIDR: true,
172+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
173+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(nil, awserrors.NewFailedDependency("dependency-failure"))
174+
},
175+
wantErr: true,
176+
},
177+
{
178+
name: "Should return error if no VPC found",
179+
haveSecondaryCIDR: true,
180+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
181+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(nil, nil)
182+
},
183+
wantErr: true,
184+
},
185+
{
186+
name: "Should diassociate secondary cidr block if already exist in VPC",
187+
haveSecondaryCIDR: true,
188+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
189+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(&ec2.DescribeVpcsOutput{
190+
Vpcs: []*ec2.Vpc{
191+
{
192+
CidrBlockAssociationSet: []*ec2.VpcCidrBlockAssociation{
193+
{CidrBlock: aws.String("secondary-cidr")},
194+
},
195+
},
196+
}}, nil)
197+
m.DisassociateVpcCidrBlock(gomock.AssignableToTypeOf(&ec2.DisassociateVpcCidrBlockInput{})).Return(nil, nil)
198+
},
199+
},
200+
{
201+
name: "Should return error if failed to diassociate secondary cidr block",
202+
haveSecondaryCIDR: true,
203+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
204+
m.DescribeVpcs(gomock.AssignableToTypeOf(&ec2.DescribeVpcsInput{})).Return(&ec2.DescribeVpcsOutput{
205+
Vpcs: []*ec2.Vpc{
206+
{
207+
CidrBlockAssociationSet: []*ec2.VpcCidrBlockAssociation{
208+
{CidrBlock: aws.String("secondary-cidr")},
209+
},
210+
},
211+
}}, nil)
212+
m.DisassociateVpcCidrBlock(gomock.AssignableToTypeOf(&ec2.DisassociateVpcCidrBlockInput{})).Return(nil, awserrors.NewFailedDependency("dependency-failure"))
213+
},
214+
wantErr: true,
215+
},
216+
}
217+
for _, tt := range tests {
218+
t.Run(tt.name, func(t *testing.T) {
219+
g := NewWithT(t)
220+
221+
scheme, err := setupScheme()
222+
g.Expect(err).NotTo(HaveOccurred())
223+
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
224+
225+
ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl)
226+
227+
mcpScope, err := setupNewManagedControlPlaneScope(cl)
228+
g.Expect(err).NotTo(HaveOccurred())
229+
230+
if !tt.haveSecondaryCIDR {
231+
mcpScope.ControlPlane.Spec.SecondaryCidrBlock = nil
232+
}
233+
234+
s := NewService(mcpScope)
235+
s.EC2Client = ec2Mock
236+
237+
if tt.expect != nil {
238+
tt.expect(ec2Mock.EXPECT())
239+
}
240+
241+
err = s.disassociateSecondaryCidr()
242+
if tt.wantErr {
243+
g.Expect(err).To(HaveOccurred())
244+
return
245+
}
246+
g.Expect(err).NotTo(HaveOccurred())
247+
})
248+
}
249+
}

0 commit comments

Comments
 (0)