Skip to content

Commit

Permalink
handle launch template latest version in observer (#62)
Browse files Browse the repository at this point in the history
* handle launch template latest version in observer
* update logger not working in cloud provider
  • Loading branch information
Jacobious52 authored Jul 25, 2023
1 parent b5b3824 commit d84d128
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 10 deletions.
2 changes: 1 addition & 1 deletion cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion cmd/observer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion docs/deployment/cloud-providers/aws/iam_policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"autoscaling:DetachInstances",
"autoscaling:AttachInstances",
"ec2:TerminateInstances",
"ec2:DescribeInstances"
"ec2:DescribeInstances",
"ec2:DescribeLaunchTemplateVersions"
],
"Resource": "*"
}
Expand Down
47 changes: 46 additions & 1 deletion pkg/cloudprovider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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-]+)`)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
49 changes: 48 additions & 1 deletion pkg/cloudprovider/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions pkg/cloudprovider/aws/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions pkg/cloudprovider/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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)
}

0 comments on commit d84d128

Please sign in to comment.