diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 1517877..ba2a5a7 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -95,7 +95,7 @@ func main() { metrics.Register(mgr.GetClient(), log, *namespace) // Setup the cloud provider - cloudProvider, err := builder.BuildCloudProvider(*cloudProviderName) + cloudProvider, err := builder.BuildCloudProvider(*cloudProviderName, logger) if err != nil { log.Error(err, "Unable to build cloud provider") os.Exit(1) diff --git a/cmd/observer/main.go b/cmd/observer/main.go index acafa07..9b2a277 100644 --- a/cmd/observer/main.go +++ b/cmd/observer/main.go @@ -14,6 +14,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/cache" "k8s.io/klog" + "k8s.io/klog/klogr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/atlassian-labs/cyclops/pkg/apis" @@ -27,6 +28,7 @@ import ( var ( // replaced by ldflags at buildtime version = "undefined" //nolint:golint,varcheck,deadcode,unused + klogger = klogr.New() ) // app type holds options for the application from cobra @@ -168,7 +170,7 @@ func (a *app) createK8SObserver(nodeLister k8s.NodeLister, podLister k8s.PodList // createCloudObserver creates a new cloud.Observer with the given cloud provider name func (a *app) createCloudObserver(nodeLister k8s.NodeLister) observer.Observer { // Setup the backend cloud provider - cloudProvider, err := builder.BuildCloudProvider(*a.cloudProviderName) + cloudProvider, err := builder.BuildCloudProvider(*a.cloudProviderName, klogger) if err != nil { klog.Error(err, "Unable to build cloud provider") os.Exit(1) diff --git a/docs/deployment/cloud-providers/aws/iam_policy.json b/docs/deployment/cloud-providers/aws/iam_policy.json index cdc186c..1a9a164 100644 --- a/docs/deployment/cloud-providers/aws/iam_policy.json +++ b/docs/deployment/cloud-providers/aws/iam_policy.json @@ -8,7 +8,8 @@ "autoscaling:DetachInstances", "autoscaling:AttachInstances", "ec2:TerminateInstances", - "ec2:DescribeInstances" + "ec2:DescribeInstances", + "ec2:DescribeLaunchTemplateVersions" ], "Resource": "*" } diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index 0a30d8d..182e724 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -3,14 +3,18 @@ package aws import ( "fmt" "regexp" + "strconv" "strings" "github.com/atlassian-labs/cyclops/pkg/cloudprovider" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/go-logr/logr" + "github.com/pkg/errors" ) const ( @@ -20,6 +24,9 @@ const ( validationErrorCode = "ValidationError" alreadyAttachedMessage = "is already part of AutoScalingGroup" alreadyDetachingMessage = "is not in InService or Standby" + + // launchTemplateLatestVersion defines the launching of the latest version of the template. + launchTemplateLatestVersion = "$Latest" ) var providerIDRegex = regexp.MustCompile(`aws:\/\/\/[\w-]+\/([\w-]+)`) @@ -67,7 +74,8 @@ type provider struct { } type autoscalingGroups struct { - autoScalingService *autoscaling.AutoScaling + autoScalingService autoscalingiface.AutoScalingAPI + ec2Service ec2iface.EC2API groups []*autoscaling.Group logger logr.Logger } @@ -100,6 +108,8 @@ func (p *provider) GetNodeGroups(names []string) (cloudprovider.NodeGroups, erro return &autoscalingGroups{ groups: groups, autoScalingService: p.autoScalingService, + ec2Service: p.ec2Service, + logger: p.logger, }, nil } @@ -311,6 +321,18 @@ func (a *autoscalingGroups) instanceOutOfDate(instance *autoscaling.Instance) bo groupVersion = aws.StringValue(group.LaunchConfigurationName) case group.LaunchTemplate != nil: groupVersion = aws.StringValue(group.LaunchTemplate.Version) + + if groupVersion == launchTemplateLatestVersion { + groupVersion, err = a.getLaunchTemplateLatestVersion(aws.StringValue(group.LaunchTemplate.LaunchTemplateId)) + if err != nil { + a.logger.WithValues( + "lt-id", aws.StringValue(group.LaunchTemplate.LaunchTemplateId), + "lt-name", aws.StringValue(group.LaunchTemplate.LaunchTemplateName), + ).Error(err, "[ASG] failed to get latest asg version") + return false + } + } + case group.MixedInstancesPolicy != nil: if policy := group.MixedInstancesPolicy; policy.LaunchTemplate != nil && policy.LaunchTemplate.LaunchTemplateSpecification != nil { groupVersion = aws.StringValue(policy.LaunchTemplate.LaunchTemplateSpecification.Version) @@ -325,9 +347,32 @@ func (a *autoscalingGroups) instanceOutOfDate(instance *autoscaling.Instance) bo instanceVersion = aws.StringValue(instance.LaunchTemplate.Version) } + a.logger.WithValues( + "instance", instanceVersion, + "asg", groupVersion, + "asg-name", aws.StringValue(group.AutoScalingGroupName), + ).Info("[ASG] out of date version check") + return groupVersion != instanceVersion } +func (a *autoscalingGroups) getLaunchTemplateLatestVersion(id string) (string, error) { + input := &ec2.DescribeLaunchTemplateVersionsInput{ + LaunchTemplateId: aws.String(id), + Versions: aws.StringSlice([]string{launchTemplateLatestVersion}), + } + out, err := a.ec2Service.DescribeLaunchTemplateVersions(input) + if err != nil { + return "", err + } + + if len(out.LaunchTemplateVersions) == 0 { + return "", errors.Wrapf(err, "[ASG] failed to get latest launch template version %q", id) + } + + return strconv.Itoa(int(*out.LaunchTemplateVersions[0].VersionNumber)), nil +} + // ID returns the ID for the instance func (i *instance) ID() string { return aws.StringValue(i.instance.InstanceId) diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index 434c485..587cc98 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -3,12 +3,25 @@ package aws import ( "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" ) +type mockedEC2 struct { + ec2iface.EC2API + Resp ec2.DescribeLaunchTemplateVersionsOutput +} + +func (m mockedEC2) DescribeLaunchTemplateVersions(in *ec2.DescribeLaunchTemplateVersionsInput) (*ec2.DescribeLaunchTemplateVersionsOutput, error) { + // Only need to return mocked response output + return &m.Resp, nil +} + // Test_providerIDToInstanceID is checking that the regex used is correctly matching the providerID to instanceID format // rather than ensuring the correct instanceID format exactly func Test_providerIDToInstanceID(t *testing.T) { @@ -124,6 +137,16 @@ func TestInstance_OutOfDate(t *testing.T) { okConfig, notOkConfig, emptyConfig := "ok-config-name", "not-ok-config-name", "" configV2, configV3 := "2", "3" + mockedEC2ServiceLatest := &mockedEC2{ + Resp: ec2.DescribeLaunchTemplateVersionsOutput{ + LaunchTemplateVersions: []*ec2.LaunchTemplateVersion{ + { + VersionNumber: aws.Int64(3), + }, + }, + }, + } + tests := []struct { name string group *autoscaling.Group @@ -386,12 +409,36 @@ func TestInstance_OutOfDate(t *testing.T) { buildInstance(&instanceID, &okConfig), true, }, + { + "test asg set to latest should match if instance is latest", + &autoscaling.Group{ + Instances: []*autoscaling.Instance{buildLTInstance(&instanceID, &configV3)}, + LaunchTemplate: &autoscaling.LaunchTemplateSpecification{ + Version: aws.String(launchTemplateLatestVersion), + }, + }, + buildLTInstance(&instanceID, &configV3), + false, + }, + { + "test asg set to latest should not match if instance not latest", + &autoscaling.Group{ + Instances: []*autoscaling.Instance{buildLTInstance(&instanceID, &configV2)}, + LaunchTemplate: &autoscaling.LaunchTemplateSpecification{ + Version: aws.String(launchTemplateLatestVersion), + }, + }, + buildLTInstance(&instanceID, &configV2), + true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { asg := &autoscalingGroups{ - groups: []*autoscaling.Group{tt.group}, + groups: []*autoscaling.Group{tt.group}, + ec2Service: mockedEC2ServiceLatest, + logger: logr.Discard(), } outOfDate := asg.instanceOutOfDate(tt.instance) assert.Equal(t, tt.expect, outOfDate) diff --git a/pkg/cloudprovider/aws/builder.go b/pkg/cloudprovider/aws/builder.go index bd9bd3e..a1b189b 100644 --- a/pkg/cloudprovider/aws/builder.go +++ b/pkg/cloudprovider/aws/builder.go @@ -9,13 +9,14 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/autoscaling" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/go-logr/logr" logf "sigs.k8s.io/controller-runtime/pkg/log" ) -var log = logf.Log.WithName("aws") +var defaultLogger = logf.Log.WithName("aws") // NewCloudProvider returns a new AWS cloud provider -func NewCloudProvider() (cloudprovider.CloudProvider, error) { +func NewCloudProvider(logger logr.Logger) (cloudprovider.CloudProvider, error) { sess, err := session.NewSession() if err != nil { return nil, err @@ -27,6 +28,11 @@ func NewCloudProvider() (cloudprovider.CloudProvider, error) { ec2Service := ec2.New(sess, config) autoScalingService := autoscaling.New(sess, config) + var log = defaultLogger + if logger != nil { + log = logger + } + p := &provider{ autoScalingService: autoScalingService, ec2Service: ec2Service, diff --git a/pkg/cloudprovider/builder/builder.go b/pkg/cloudprovider/builder/builder.go index b81ab53..13d8c44 100644 --- a/pkg/cloudprovider/builder/builder.go +++ b/pkg/cloudprovider/builder/builder.go @@ -5,12 +5,13 @@ import ( "github.com/atlassian-labs/cyclops/pkg/cloudprovider" "github.com/atlassian-labs/cyclops/pkg/cloudprovider/aws" + "github.com/go-logr/logr" ) -type builderFunc func() (cloudprovider.CloudProvider, error) +type builderFunc func(logger logr.Logger) (cloudprovider.CloudProvider, error) // BuildCloudProvider returns a cloud provider based on the provided name -func BuildCloudProvider(name string) (cloudprovider.CloudProvider, error) { +func BuildCloudProvider(name string, logger logr.Logger) (cloudprovider.CloudProvider, error) { buildFuncs := map[string]builderFunc{ aws.ProviderName: aws.NewCloudProvider, } @@ -20,5 +21,5 @@ func BuildCloudProvider(name string) (cloudprovider.CloudProvider, error) { return nil, fmt.Errorf("builder for cloud provider %v not found", name) } - return builder() + return builder(logger) }