Skip to content

Commit 0934a16

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3298 from Ankitasw/bastion-ami-lookup
Removed hardcoding for AMIs in bastion host and added latest AMI lookup
2 parents f99d577 + 605fb04 commit 0934a16

File tree

4 files changed

+294
-43
lines changed

4 files changed

+294
-43
lines changed

‎docs/book/src/topics/accessing-ec2-instances.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ spec:
3131
bastion:
3232
enabled: true
3333
```
34+
If this field is set and a specific AMI ID is not provided for the bastion (by setting spec.bastion.ami) then by default the latest AMI(Ubuntu 20.04 LTS OS) is looked up from [Ubuntu cloud images](https://ubuntu.com/server/docs/cloud-images/amazon-ec2) by CAPA controller and used in bastion host creation.
3435
3536
#### Obtain public IP address of the bastion node
3637

‎pkg/cloud/services/ec2/ami.go

Lines changed: 43 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ const (
4040
// https://github.com/kubernetes-sigs/cluster-api-provider-aws/issues/487
4141
DefaultMachineAMIOwnerID = "258751437250"
4242

43+
// ubuntuOwnerID is Ubuntu owned account. Please see:
44+
// https://ubuntu.com/server/docs/cloud-images/amazon-ec2
45+
ubuntuOwnerID = "099720109477"
46+
47+
// Description regex for fetching Ubuntu AMIs for bastion host.
48+
ubuntuImageDescription = "Canonical??Ubuntu??20.04?LTS??amd64?focal?image*"
49+
4350
// defaultMachineAMILookupBaseOS is the default base operating system to use
4451
// when looking up machine AMIs.
4552
defaultMachineAMILookupBaseOS = "ubuntu-18.04"
@@ -188,45 +195,43 @@ func GetLatestImage(imgs []*ec2.Image) (*ec2.Image, error) {
188195
return imgs[len(imgs)-1], nil
189196
}
190197

191-
func (s *Service) defaultBastionAMILookup(region string) string {
192-
switch region {
193-
case "ap-northeast-1":
194-
return "ami-09b86f9709b3c33d4"
195-
case "ap-northeast-2":
196-
return "ami-044057cb1bc4ce527"
197-
case "ap-south-1":
198-
return "ami-0cda377a1b884a1bc"
199-
case "ap-southeast-1":
200-
return "ami-093da183b859d5a4b"
201-
case "ap-southeast-2":
202-
return "ami-0f158b0f26f18e619"
203-
case "ca-central-1":
204-
return "ami-0edab43b6fa892279"
205-
case "eu-central-1":
206-
return "ami-0c960b947cbb2dd16"
207-
case "eu-west-1":
208-
return "ami-06fd8a495a537da8b"
209-
case "eu-west-2":
210-
return "ami-05c424d59413a2876"
211-
case "eu-west-3":
212-
return "ami-078db6d55a16afc82"
213-
case "sa-east-1":
214-
return "ami-02dc8ad50da58fffd"
215-
case "us-east-1":
216-
return "ami-0dba2cb6798deb6d8"
217-
case "us-east-2":
218-
return "ami-07efac79022b86107"
219-
case "us-west-1":
220-
return "ami-021809d9177640a20"
221-
case "us-west-2":
222-
return "ami-06e54d05255faf8f6"
223-
case "eu-north-1":
224-
return "ami-008dea09a148cea39"
225-
case "eu-south-1":
226-
return "ami-01eec6bdfa20f008e"
227-
default:
228-
return "unknown region"
198+
func (s *Service) defaultBastionAMILookup() (string, error) {
199+
describeImageInput := &ec2.DescribeImagesInput{
200+
Filters: []*ec2.Filter{
201+
{
202+
Name: aws.String("owner-id"),
203+
Values: []*string{aws.String(ubuntuOwnerID)},
204+
},
205+
{
206+
Name: aws.String("architecture"),
207+
Values: []*string{aws.String("x86_64")},
208+
},
209+
{
210+
Name: aws.String("state"),
211+
Values: []*string{aws.String("available")},
212+
},
213+
{
214+
Name: aws.String("virtualization-type"),
215+
Values: []*string{aws.String("hvm")},
216+
},
217+
{
218+
Name: aws.String("description"),
219+
Values: aws.StringSlice([]string{ubuntuImageDescription}),
220+
},
221+
},
222+
}
223+
out, err := s.EC2Client.DescribeImages(describeImageInput)
224+
if err != nil {
225+
return "", errors.Wrapf(err, "failed to describe images within region: %q", s.scope.Region())
226+
}
227+
if len(out.Images) == 0 {
228+
return "", errors.Errorf("found no AMIs within the region: %q", s.scope.Region())
229+
}
230+
latestImage, err := GetLatestImage(out.Images)
231+
if err != nil {
232+
return "", err
229233
}
234+
return *latestImage.ImageId, nil
230235
}
231236

232237
func (s *Service) eksAMILookup(kubernetesVersion string, amiType *infrav1.EKSAMILookupType) (string, error) {

‎pkg/cloud/services/ec2/bastion.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ func (s *Service) ReconcileBastion() error {
7676
return errors.Wrap(err, "failed to patch conditions")
7777
}
7878
}
79-
instance, err = s.runInstance("bastion", s.getDefaultBastion(s.scope.Bastion().InstanceType, s.scope.Bastion().AMI))
79+
defaultBastion, err := s.getDefaultBastion(s.scope.Bastion().InstanceType, s.scope.Bastion().AMI)
80+
if err != nil {
81+
record.Warnf(s.scope.InfraCluster(), "FailedFetchingBastion", "Failed to fetch default bastion instance: %v", err)
82+
return err
83+
}
84+
instance, err = s.runInstance("bastion", defaultBastion)
8085
if err != nil {
8186
record.Warnf(s.scope.InfraCluster(), "FailedCreateBastion", "Failed to create bastion instance: %v", err)
8287
return err
@@ -161,7 +166,7 @@ func (s *Service) describeBastionInstance() (*infrav1.Instance, error) {
161166
return nil, awserrors.NewNotFound("bastion host not found")
162167
}
163168

164-
func (s *Service) getDefaultBastion(instanceType, ami string) *infrav1.Instance {
169+
func (s *Service) getDefaultBastion(instanceType, ami string) (*infrav1.Instance, error) {
165170
name := fmt.Sprintf("%s-bastion", s.scope.Name())
166171
userData, _ := userdata.NewBastion(&userdata.BastionInput{})
167172

@@ -182,7 +187,11 @@ func (s *Service) getDefaultBastion(instanceType, ami string) *infrav1.Instance
182187
}
183188

184189
if ami == "" {
185-
ami = s.defaultBastionAMILookup(s.scope.Region())
190+
var err error
191+
ami, err = s.defaultBastionAMILookup()
192+
if err != nil {
193+
return nil, err
194+
}
186195
}
187196

188197
i := &infrav1.Instance{
@@ -203,5 +212,5 @@ func (s *Service) getDefaultBastion(instanceType, ami string) *infrav1.Instance
203212
}),
204213
}
205214

206-
return i
215+
return i, nil
207216
}

‎pkg/cloud/services/ec2/bastion_test.go

Lines changed: 237 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3737
)
3838

39-
func TestDeleteBastion(t *testing.T) {
39+
func TestService_DeleteBastion(t *testing.T) {
4040
clusterName := "cluster"
4141

4242
describeInput := &ec2.DescribeInstancesInput{
@@ -225,3 +225,239 @@ func TestDeleteBastion(t *testing.T) {
225225
}
226226
}
227227
}
228+
229+
func TestService_ReconcileBastion(t *testing.T) {
230+
clusterName := "cluster"
231+
232+
describeInput := &ec2.DescribeInstancesInput{
233+
Filters: []*ec2.Filter{
234+
filter.EC2.ProviderRole(infrav1.BastionRoleTagValue),
235+
filter.EC2.Cluster(clusterName),
236+
filter.EC2.InstanceStates(
237+
ec2.InstanceStateNamePending,
238+
ec2.InstanceStateNameRunning,
239+
ec2.InstanceStateNameStopping,
240+
ec2.InstanceStateNameStopped,
241+
),
242+
},
243+
}
244+
245+
foundOutput := &ec2.DescribeInstancesOutput{
246+
Reservations: []*ec2.Reservation{
247+
{
248+
Instances: []*ec2.Instance{
249+
{
250+
InstanceId: aws.String("id123"),
251+
State: &ec2.InstanceState{
252+
Name: aws.String(ec2.InstanceStateNameRunning),
253+
},
254+
Placement: &ec2.Placement{
255+
AvailabilityZone: aws.String("us-east-1"),
256+
},
257+
},
258+
},
259+
},
260+
},
261+
}
262+
263+
tests := []struct {
264+
name string
265+
bastionEnabled bool
266+
expect func(m *mock_ec2iface.MockEC2APIMockRecorder)
267+
expectError bool
268+
bastionStatus *infrav1.Instance
269+
}{
270+
{
271+
name: "Should ignore reconciliation if instance not found",
272+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
273+
m.
274+
DescribeInstances(gomock.Eq(describeInput)).
275+
Return(&ec2.DescribeInstancesOutput{}, nil)
276+
},
277+
expectError: false,
278+
},
279+
{
280+
name: "Should fail reconcile if describe instance fails",
281+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
282+
m.
283+
DescribeInstances(gomock.Eq(describeInput)).
284+
Return(nil, errors.New("some error"))
285+
},
286+
expectError: true,
287+
},
288+
{
289+
name: "Should fail reconcile if terminate instance fails",
290+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
291+
m.
292+
DescribeInstances(gomock.Eq(describeInput)).
293+
Return(foundOutput, nil).MinTimes(1)
294+
m.
295+
TerminateInstances(
296+
gomock.Eq(&ec2.TerminateInstancesInput{
297+
InstanceIds: aws.StringSlice([]string{"id123"}),
298+
}),
299+
).
300+
Return(nil, errors.New("some error"))
301+
},
302+
expectError: true,
303+
},
304+
{
305+
name: "Should create bastion successfully",
306+
expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) {
307+
m.DescribeInstances(gomock.Eq(describeInput)).
308+
Return(&ec2.DescribeInstancesOutput{}, nil).MinTimes(1)
309+
m.DescribeImages(gomock.Eq(&ec2.DescribeImagesInput{Filters: []*ec2.Filter{
310+
{
311+
Name: aws.String("owner-id"),
312+
Values: aws.StringSlice([]string{ubuntuOwnerID}),
313+
},
314+
{
315+
Name: aws.String("architecture"),
316+
Values: aws.StringSlice([]string{"x86_64"}),
317+
},
318+
{
319+
Name: aws.String("state"),
320+
Values: aws.StringSlice([]string{"available"}),
321+
},
322+
{
323+
Name: aws.String("virtualization-type"),
324+
Values: aws.StringSlice([]string{"hvm"}),
325+
},
326+
{
327+
Name: aws.String("description"),
328+
Values: aws.StringSlice([]string{ubuntuImageDescription}),
329+
},
330+
}})).Return(&ec2.DescribeImagesOutput{Images: images{
331+
{
332+
ImageId: aws.String("ubuntu-ami-id-latest"),
333+
CreationDate: aws.String("2019-02-08T17:02:31.000Z"),
334+
},
335+
{
336+
ImageId: aws.String("ubuntu-ami-id-old"),
337+
CreationDate: aws.String("2014-02-08T17:02:31.000Z"),
338+
},
339+
}}, nil)
340+
m.RunInstances(gomock.Any()).
341+
Return(&ec2.Reservation{
342+
Instances: []*ec2.Instance{
343+
{
344+
State: &ec2.InstanceState{
345+
Name: aws.String(ec2.InstanceStateNameRunning),
346+
},
347+
IamInstanceProfile: &ec2.IamInstanceProfile{
348+
Arn: aws.String("arn:aws:iam::123456789012:instance-profile/foo"),
349+
},
350+
InstanceId: aws.String("id123"),
351+
InstanceType: aws.String("t3.micro"),
352+
SubnetId: aws.String("subnet-1"),
353+
ImageId: aws.String("ubuntu-ami-id-latest"),
354+
RootDeviceName: aws.String("device-1"),
355+
BlockDeviceMappings: []*ec2.InstanceBlockDeviceMapping{
356+
{
357+
DeviceName: aws.String("device-1"),
358+
Ebs: &ec2.EbsInstanceBlockDevice{
359+
VolumeId: aws.String("volume-1"),
360+
},
361+
},
362+
},
363+
Placement: &ec2.Placement{
364+
AvailabilityZone: aws.String("us-east-1"),
365+
},
366+
},
367+
},
368+
}, nil)
369+
m.WaitUntilInstanceRunningWithContext(gomock.Any(), gomock.Any(), gomock.Any()).
370+
Return(nil)
371+
},
372+
bastionEnabled: true,
373+
expectError: false,
374+
bastionStatus: &infrav1.Instance{
375+
ID: "id123",
376+
State: "running",
377+
Type: "t3.micro",
378+
SubnetID: "subnet-1",
379+
ImageID: "ubuntu-ami-id-latest",
380+
IAMProfile: "foo",
381+
Addresses: []clusterv1.MachineAddress{},
382+
AvailabilityZone: "us-east-1",
383+
VolumeIDs: []string{"volume-1"},
384+
},
385+
},
386+
}
387+
388+
for _, tc := range tests {
389+
managedValues := []bool{false, true}
390+
for i := range managedValues {
391+
managed := managedValues[i]
392+
393+
t.Run(fmt.Sprintf("managed=%t %s", managed, tc.name), func(t *testing.T) {
394+
g := NewWithT(t)
395+
396+
mockControl := gomock.NewController(t)
397+
defer mockControl.Finish()
398+
399+
ec2Mock := mock_ec2iface.NewMockEC2API(mockControl)
400+
401+
scheme, err := setupScheme()
402+
g.Expect(err).To(BeNil())
403+
404+
awsCluster := &infrav1.AWSCluster{
405+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
406+
Spec: infrav1.AWSClusterSpec{
407+
NetworkSpec: infrav1.NetworkSpec{
408+
VPC: infrav1.VPCSpec{
409+
ID: "vpcID",
410+
},
411+
Subnets: infrav1.Subnets{
412+
{
413+
ID: "subnet-1",
414+
},
415+
{
416+
ID: "subnet-2",
417+
IsPublic: true,
418+
},
419+
},
420+
},
421+
Bastion: infrav1.Bastion{Enabled: tc.bastionEnabled},
422+
},
423+
}
424+
425+
client := fake.NewClientBuilder().WithScheme(scheme).Build()
426+
ctx := context.TODO()
427+
client.Create(ctx, awsCluster)
428+
429+
scope, err := scope.NewClusterScope(scope.ClusterScopeParams{
430+
Cluster: &clusterv1.Cluster{
431+
ObjectMeta: metav1.ObjectMeta{
432+
Namespace: "ns",
433+
Name: clusterName,
434+
},
435+
},
436+
AWSCluster: awsCluster,
437+
Client: client,
438+
})
439+
g.Expect(err).To(BeNil())
440+
441+
if managed {
442+
scope.AWSCluster.Spec.NetworkSpec.VPC.Tags = infrav1.Tags{
443+
infrav1.ClusterTagKey(clusterName): string(infrav1.ResourceLifecycleOwned),
444+
}
445+
}
446+
447+
tc.expect(ec2Mock.EXPECT())
448+
s := NewService(scope)
449+
s.EC2Client = ec2Mock
450+
451+
err = s.ReconcileBastion()
452+
if tc.expectError {
453+
g.Expect(err).NotTo(BeNil())
454+
return
455+
}
456+
457+
g.Expect(err).To(BeNil())
458+
459+
g.Expect(scope.AWSCluster.Status.Bastion).To(BeEquivalentTo(tc.bastionStatus))
460+
})
461+
}
462+
}
463+
}

0 commit comments

Comments
 (0)