Skip to content

Commit

Permalink
Log if availability zone does not exist
Browse files Browse the repository at this point in the history
Improve user experience by logging availability zone
does not exist instead of logging subnet not found if
availability zone does not exist.
  • Loading branch information
ingvagabund committed Jan 15, 2019
1 parent 97771a0 commit b6b8de3
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/actuators/machine/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ func TestAvailabiltyZone(t *testing.T) {

mockAWSClient.EXPECT().TerminateInstances(gomock.Any()).Return(&ec2.TerminateInstancesOutput{}, nil)
mockAWSClient.EXPECT().RegisterInstancesWithLoadBalancer(gomock.Any()).AnyTimes()

mockAWSClient.EXPECT().DescribeAvailabilityZones(gomock.Any()).Return(nil, nil).AnyTimes()
mockAWSClient.EXPECT().DescribeSubnets(gomock.Any()).Return(&ec2.DescribeSubnetsOutput{}, nil)

actuator.Create(context.TODO(), cluster, machine)
Expand Down
9 changes: 9 additions & 0 deletions pkg/actuators/machine/instaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ func TestLaunchInstance(t *testing.T) {
securityGroupErr error
subnetOutput *ec2.DescribeSubnetsOutput
subnetErr error
azErr error
imageOutput *ec2.DescribeImagesOutput
imageErr error
}{
Expand Down Expand Up @@ -327,6 +328,13 @@ func TestLaunchInstance(t *testing.T) {
}),
subnetErr: fmt.Errorf("error"),
},
{
name: "Subnet with availability zone with error",
providerConfig: stubPCSubnet(providerconfigv1.AWSResourceReference{
Filters: []providerconfigv1.Filter{},
}),
azErr: fmt.Errorf("error"),
},
{
name: "AMI with filters",
providerConfig: stubPCAMI(providerconfigv1.AWSResourceReference{
Expand Down Expand Up @@ -396,6 +404,7 @@ func TestLaunchInstance(t *testing.T) {
mockAWSClient := mockaws.NewMockClient(mockCtrl)

mockAWSClient.EXPECT().DescribeSecurityGroups(gomock.Any()).Return(tc.securityGroupOutput, tc.securityGroupErr).AnyTimes()
mockAWSClient.EXPECT().DescribeAvailabilityZones(gomock.Any()).Return(nil, tc.azErr).AnyTimes()
mockAWSClient.EXPECT().DescribeSubnets(gomock.Any()).Return(tc.subnetOutput, tc.subnetErr).AnyTimes()
mockAWSClient.EXPECT().DescribeImages(gomock.Any()).Return(tc.imageOutput, tc.imageErr).AnyTimes()
mockAWSClient.EXPECT().RunInstances(gomock.Any())
Expand Down
10 changes: 10 additions & 0 deletions pkg/actuators/machine/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ func getSubnetIDs(subnet providerconfigv1.AWSResourceReference, availabilityZone
} else {
var filters []providerconfigv1.Filter
if availabilityZone != "" {
// Improve error logging for better user experience.
// Otherwise, during the process of minimizing API calls, this is a good
// candidate for removal.
_, err := client.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{
ZoneNames: []*string{aws.String(availabilityZone)},
})
if err != nil {
glog.Errorf("error describing availability zones: %v", err)
return nil, fmt.Errorf("error describing availability zones: %v", err)
}
filters = append(filters, providerconfigv1.Filter{Name: "availabilityZone", Values: []string{availabilityZone}})
}
filters = append(filters, subnet.Filters...)
Expand Down
5 changes: 5 additions & 0 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type Client interface {
DescribeImages(*ec2.DescribeImagesInput) (*ec2.DescribeImagesOutput, error)
DescribeVpcs(*ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error)
DescribeSubnets(*ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error)
DescribeAvailabilityZones(*ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error)
DescribeSecurityGroups(*ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error)
RunInstances(*ec2.RunInstancesInput) (*ec2.Reservation, error)
DescribeInstances(*ec2.DescribeInstancesInput) (*ec2.DescribeInstancesOutput, error)
Expand Down Expand Up @@ -80,6 +81,10 @@ func (c *awsClient) DescribeSubnets(input *ec2.DescribeSubnetsInput) (*ec2.Descr
return c.ec2Client.DescribeSubnets(input)
}

func (c *awsClient) DescribeAvailabilityZones(input *ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error) {
return c.ec2Client.DescribeAvailabilityZones(input)
}

func (c *awsClient) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) {
return c.ec2Client.DescribeSecurityGroups(input)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/client/fake/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ func (c *awsClient) DescribeSubnets(input *ec2.DescribeSubnetsInput) (*ec2.Descr
}, nil
}

func (c *awsClient) DescribeAvailabilityZones(*ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error) {
return &ec2.DescribeAvailabilityZonesOutput{}, nil
}

func (c *awsClient) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInput) (*ec2.DescribeSecurityGroupsOutput, error) {
return &ec2.DescribeSecurityGroupsOutput{
SecurityGroups: []*ec2.SecurityGroup{
Expand Down
13 changes: 13 additions & 0 deletions pkg/client/mock/client_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b6b8de3

Please sign in to comment.