From 5e262932ae792e40876f01866c1e7a64cd95a24d Mon Sep 17 00:00:00 2001 From: Rohith Date: Fri, 9 Nov 2018 21:21:46 +0000 Subject: [PATCH] - fixing up various linting issues and formatting --- pkg/model/awsmodel/autoscalinggroup.go | 32 +++++----- upup/pkg/fi/cloudup/apply_cluster.go | 20 +++--- .../fi/cloudup/awstasks/autoscalinggroup.go | 61 ++++++++++--------- .../cloudup/awstasks/launchconfiguration.go | 26 +++++--- 4 files changed, 72 insertions(+), 67 deletions(-) diff --git a/pkg/model/awsmodel/autoscalinggroup.go b/pkg/model/awsmodel/autoscalinggroup.go index 400986b278..41c51466ce 100644 --- a/pkg/model/awsmodel/autoscalinggroup.go +++ b/pkg/model/awsmodel/autoscalinggroup.go @@ -20,13 +20,13 @@ import ( "fmt" "strings" - "github.com/golang/glog" - "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/model" "k8s.io/kops/pkg/model/defaults" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awstasks" + + "github.com/golang/glog" ) const ( @@ -97,17 +97,14 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { Name: s(name), Lifecycle: b.Lifecycle, - SecurityGroups: []*awstasks.SecurityGroup{ - sgLink, - }, - IAMInstanceProfile: link, - ImageID: s(ig.Spec.Image), - InstanceType: s(strings.Split(ig.Spec.MachineType, ",")[0]), - InstanceMonitoring: ig.Spec.DetailedInstanceMonitoring, - + IAMInstanceProfile: link, + ImageID: s(ig.Spec.Image), + InstanceMonitoring: ig.Spec.DetailedInstanceMonitoring, + InstanceType: s(strings.Split(ig.Spec.MachineType, ",")[0]), + RootVolumeOptimization: ig.Spec.RootVolumeOptimization, RootVolumeSize: i64(int64(volumeSize)), RootVolumeType: s(volumeType), - RootVolumeOptimization: ig.Spec.RootVolumeOptimization, + SecurityGroups: []*awstasks.SecurityGroup{sgLink}, } if volumeType == "io1" { @@ -118,13 +115,13 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { t.Tenancy = s(ig.Spec.Tenancy) } + // @step: add any additional security groups to the instance for _, id := range ig.Spec.AdditionalSecurityGroups { sgTask := &awstasks.SecurityGroup{ - Name: fi.String(id), - ID: fi.String(id), - Shared: fi.Bool(true), - + Name: fi.String(id), + ID: fi.String(id), Lifecycle: b.SecurityLifecycle, + Shared: fi.Bool(true), } if err := c.EnsureTask(sgTask); err != nil { return err @@ -219,16 +216,15 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { Granularity: s("1Minute"), Metrics: []string{ - "GroupMinSize", - "GroupMaxSize", "GroupDesiredCapacity", "GroupInServiceInstances", + "GroupMaxSize", + "GroupMinSize", "GroupPendingInstances", "GroupStandbyInstances", "GroupTerminatingInstances", "GroupTotalInstances", }, - LaunchConfiguration: launchConfiguration, } diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index cfd5507975..aae3a38907 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -414,16 +414,16 @@ func (c *ApplyClusterCmd) Run() error { "iamRolePolicy": &awstasks.IAMRolePolicy{}, // VPC / Networking - "dhcpOptions": &awstasks.DHCPOptions{}, - "internetGateway": &awstasks.InternetGateway{}, - "route": &awstasks.Route{}, - "routeTable": &awstasks.RouteTable{}, - "routeTableAssociation": &awstasks.RouteTableAssociation{}, - "securityGroup": &awstasks.SecurityGroup{}, - "securityGroupRule": &awstasks.SecurityGroupRule{}, - "subnet": &awstasks.Subnet{}, - "vpc": &awstasks.VPC{}, - "ngw": &awstasks.NatGateway{}, + "dhcpOptions": &awstasks.DHCPOptions{}, + "internetGateway": &awstasks.InternetGateway{}, + "route": &awstasks.Route{}, + "routeTable": &awstasks.RouteTable{}, + "routeTableAssociation": &awstasks.RouteTableAssociation{}, + "securityGroup": &awstasks.SecurityGroup{}, + "securityGroupRule": &awstasks.SecurityGroupRule{}, + "subnet": &awstasks.Subnet{}, + "vpc": &awstasks.VPC{}, + "ngw": &awstasks.NatGateway{}, "vpcDHDCPOptionsAssociation": &awstasks.VPCDHCPOptionsAssociation{}, // ELB diff --git a/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go b/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go index accb7ac52c..3e84243c6d 100644 --- a/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go +++ b/upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go @@ -23,41 +23,41 @@ import ( "sort" "strings" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/autoscaling" - "github.com/golang/glog" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" "k8s.io/kops/upup/pkg/fi/cloudup/cloudformation" "k8s.io/kops/upup/pkg/fi/cloudup/terraform" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/golang/glog" ) const CloudTagInstanceGroupRolePrefix = "k8s.io/role/" -//go:generate fitask -type=AutoscalingGroup +// AutoscalingGroup defined the specification for a autoscaling group type AutoscalingGroup struct { Name *string Lifecycle *fi.Lifecycle - MinSize *int64 - MaxSize *int64 - Subnets []*Subnet - Tags map[string]string - - Granularity *string - Metrics []string - + Granularity *string LaunchConfiguration *LaunchConfiguration - - SuspendProcesses *[]string + MaxSize *int64 + Metrics []string + MinSize *int64 + Subnets []*Subnet + SuspendProcesses *[]string + Tags map[string]string } var _ fi.CompareWithID = &AutoscalingGroup{} +// CompareWithID is used to compare resource ids func (e *AutoscalingGroup) CompareWithID() *string { return e.Name } +// findAutoscalingGroup is responsible for finding the autoscaling group func findAutoscalingGroup(cloud awsup.AWSCloud, name string) (*autoscaling.Group, error) { request := &autoscaling.DescribeAutoScalingGroupsInput{ AutoScalingGroupNames: []*string{&name}, @@ -100,6 +100,7 @@ func findAutoscalingGroup(cloud awsup.AWSCloud, name string) (*autoscaling.Group return found[0], nil } +// Find is just a wrapper for the find method func (e *AutoscalingGroup) Find(c *fi.Context) (*AutoscalingGroup, error) { cloud := c.Cloud.(awsup.AWSCloud) @@ -171,6 +172,7 @@ func (e *AutoscalingGroup) Run(c *fi.Context) error { return err } c.Cloud.(awsup.AWSCloud).AddTags(e.Name, e.Tags) + return fi.DefaultDeltaRunMethod(e, c) } @@ -191,26 +193,30 @@ func (e *AutoscalingGroup) buildTags(cloud fi.Cloud) map[string]string { return tags } +// RenderAWS is responsible for creating the autoscaling group func (_ *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *AutoscalingGroup) error { tags := []*autoscaling.Tag{} + for k, v := range e.buildTags(t.Cloud) { tags = append(tags, &autoscaling.Tag{ Key: aws.String(k), - Value: aws.String(v), + PropagateAtLaunch: aws.Bool(true), ResourceId: e.Name, ResourceType: aws.String("auto-scaling-group"), - PropagateAtLaunch: aws.Bool(true), + Value: aws.String(v), }) } if a == nil { - glog.V(2).Infof("Creating autoscaling Group with Name:%q", *e.Name) + glog.V(2).Infof("Creating autoscaling Group with Name: %q", *e.Name) - request := &autoscaling.CreateAutoScalingGroupInput{} - request.AutoScalingGroupName = e.Name - request.LaunchConfigurationName = e.LaunchConfiguration.ID - request.MinSize = e.MinSize - request.MaxSize = e.MaxSize + request := &autoscaling.CreateAutoScalingGroupInput{ + AutoScalingGroupName: e.Name, + LaunchConfigurationName: e.LaunchConfiguration.ID, + MaxSize: e.MaxSize, + MinSize: e.MinSize, + Tags: tags, + } var subnetIDs []string for _, s := range e.Subnets { @@ -218,19 +224,15 @@ func (_ *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos } request.VPCZoneIdentifier = aws.String(strings.Join(subnetIDs, ",")) - request.Tags = tags - - _, err := t.Cloud.Autoscaling().CreateAutoScalingGroup(request) - if err != nil { + if _, err := t.Cloud.Autoscaling().CreateAutoScalingGroup(request); err != nil { return fmt.Errorf("error creating AutoscalingGroup: %v", err) } - _, err = t.Cloud.Autoscaling().EnableMetricsCollection(&autoscaling.EnableMetricsCollectionInput{ + if _, err := t.Cloud.Autoscaling().EnableMetricsCollection(&autoscaling.EnableMetricsCollectionInput{ AutoScalingGroupName: e.Name, Granularity: e.Granularity, Metrics: aws.StringSlice(e.Metrics), - }) - if err != nil { + }); err != nil { return fmt.Errorf("error enabling metrics collection for AutoscalingGroup: %v", err) } @@ -414,6 +416,7 @@ type terraformAutoscalingGroup struct { SuspendedProcesses []*string `json:"suspended_processes,omitempty"` } +// RenderTerraform is responsible for rendering the terraform func (_ *AutoscalingGroup) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *AutoscalingGroup) error { tf := &terraformAutoscalingGroup{ diff --git a/upup/pkg/fi/cloudup/awstasks/launchconfiguration.go b/upup/pkg/fi/cloudup/awstasks/launchconfiguration.go index 31074e5322..d1377bc0fe 100644 --- a/upup/pkg/fi/cloudup/awstasks/launchconfiguration.go +++ b/upup/pkg/fi/cloudup/awstasks/launchconfiguration.go @@ -25,15 +25,16 @@ import ( "strings" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/autoscaling" - "github.com/golang/glog" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kops/pkg/featureflag" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" "k8s.io/kops/upup/pkg/fi/cloudup/cloudformation" "k8s.io/kops/upup/pkg/fi/cloudup/terraform" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/golang/glog" + "k8s.io/apimachinery/pkg/util/sets" ) // defaultRetainLaunchConfigurationCount is the number of launch configurations (matching the name prefix) that we should @@ -134,6 +135,7 @@ func (e *LaunchConfiguration) findLaunchConfigurations(c *fi.Context) ([]*autosc return configurations, nil } +// Find is responsible for finding the launch configuration func (e *LaunchConfiguration) Find(c *fi.Context) (*LaunchConfiguration, error) { cloud := c.Cloud.(awsup.AWSCloud) @@ -317,23 +319,28 @@ func (s *LaunchConfiguration) CheckChanges(a, e, changes *LaunchConfiguration) e return nil } +// RenderAWS is responsible for creating the launchconfiguration via api func (_ *LaunchConfiguration) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *LaunchConfiguration) error { launchConfigurationName := *e.Name + "-" + fi.BuildTimestampString() + glog.V(2).Infof("Creating AutoscalingLaunchConfiguration with Name:%q", launchConfigurationName) if e.ImageID == nil { return fi.RequiredField("ImageID") } + image, err := t.Cloud.ResolveImage(*e.ImageID) if err != nil { return err } - request := &autoscaling.CreateLaunchConfigurationInput{} - request.LaunchConfigurationName = &launchConfigurationName - request.ImageId = image.ImageId - request.InstanceType = e.InstanceType - request.EbsOptimized = e.RootVolumeOptimization + request := &autoscaling.CreateLaunchConfigurationInput{ + AssociatePublicIpAddress: e.AssociatePublicIP, + EbsOptimized: e.RootVolumeOptimization, + ImageId: image.ImageId, + InstanceType: e.InstanceType, + LaunchConfigurationName: &launchConfigurationName, + } if e.SSHKey != nil { request.KeyName = e.SSHKey.Name @@ -349,7 +356,6 @@ func (_ *LaunchConfiguration) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *La } request.SecurityGroups = securityGroupIDs - request.AssociatePublicIpAddress = e.AssociatePublicIP if e.SpotPrice != "" { request.SpotPrice = aws.String(e.SpotPrice) }