Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc task code cleanups #4660

Merged
merged 1 commit into from
Mar 12, 2018
Merged

Misc task code cleanups #4660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions upup/pkg/fi/cloudup/awstasks/elastic_ip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@ limitations under the License.
package awstasks

import (
//"fmt"
//
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"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"
Expand All @@ -32,8 +31,7 @@ import (

//go:generate fitask -type=ElasticIP

// Elastic IP
// Representation the EIP AWS task
// ElasticIP manages an AWS Address (ElasticIP)
type ElasticIP struct {
Name *string
Lifecycle *fi.Lifecycle
Expand Down Expand Up @@ -71,7 +69,7 @@ func (e *ElasticIP) FindIPAddress(context *fi.Context) (*string, error) {
return actual.PublicIP, nil
}

// Find is a public wrapper for find()
// Find returns the actual ElasticIP state, or nil if not found
func (e *ElasticIP) Find(context *fi.Context) (*ElasticIP, error) {
return e.find(context.Cloud.(awsup.AWSCloud))
}
Expand Down Expand Up @@ -175,7 +173,7 @@ func (e *ElasticIP) find(cloud awsup.AWSCloud) (*ElasticIP, error) {
return nil, nil
}

// The Run() function is called to execute this task.
// Run is called to execute this task.
// This is the main entry point of the task, and will actually
// connect our internal resource representation to an actual
// resource in AWS
Expand Down Expand Up @@ -233,7 +231,7 @@ func (_ *ElasticIP) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *ElasticIP) e
eipId = a.ID
err := t.AddAWSTags(*a.ID, changes.Tags)
if err != nil {
return fmt.Errorf("Unable to tag eip %v", err)
return fmt.Errorf("unable to tag ElasticIP: %v", err)
}
}

Expand Down
20 changes: 14 additions & 6 deletions upup/pkg/fi/cloudup/awstasks/natgateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/golang/glog"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
"k8s.io/kops/upup/pkg/fi/cloudup/cloudformation"
Expand Down Expand Up @@ -78,14 +79,13 @@ func (e *NatGateway) Find(c *fi.Context) (*NatGateway, error) {
}

if len(response.NatGateways) != 1 {
return nil, fmt.Errorf("found %d Nat Gateways, expected 1", len(response.NatGateways))
return nil, fmt.Errorf("found %d Nat Gateways with ID %q, expected 1", len(response.NatGateways), fi.StringValue(e.ID))
}
ngw = response.NatGateways[0]

if len(ngw.NatGatewayAddresses) != 1 {
return nil, fmt.Errorf("found %d EIP Addresses for 1 NATGateway, expected 1", len(ngw.NatGatewayAddresses))
}
actual.ElasticIP = &ElasticIP{ID: ngw.NatGatewayAddresses[0].AllocationId}
} else {
// This is the normal/default path
var err error
Expand Down Expand Up @@ -113,10 +113,10 @@ func (e *NatGateway) Find(c *fi.Context) (*NatGateway, error) {
// NATGateways now have names and tags so lets pull from there instead.
actual.Name = findNameTag(ngw.Tags)
actual.Tags = intersectTags(ngw.Tags, e.Tags)
actual.Lifecycle = e.Lifecycle

// Avoid spurious changes
actual.Lifecycle = e.Lifecycle
actual.Shared = e.Shared

actual.AssociatedRouteTable = e.AssociatedRouteTable

e.ID = actual.ID
Expand Down Expand Up @@ -231,7 +231,7 @@ func (s *NatGateway) CheckChanges(a, e, changes *NatGateway) error {
if a == nil {
if !fi.BoolValue(e.Shared) {
if e.ElasticIP == nil {
return fi.RequiredField("ElasticIp")
return fi.RequiredField("ElasticIP")
}
if e.Subnet == nil {
return fi.RequiredField("Subnet")
Expand All @@ -245,7 +245,15 @@ func (s *NatGateway) CheckChanges(a, e, changes *NatGateway) error {
// Delta
if a != nil {
if changes.ElasticIP != nil {
return fi.CannotChangeField("ElasticIp")
eID := ""
if e.ElasticIP != nil {
eID = fi.StringValue(e.ElasticIP.ID)
}
aID := ""
if a.ElasticIP != nil {
aID = fi.StringValue(a.ElasticIP.ID)
}
return fi.FieldIsImmutable(eID, aID, field.NewPath("ElasticIP"))
}
if changes.Subnet != nil {
return fi.CannotChangeField("Subnet")
Expand Down
10 changes: 9 additions & 1 deletion upup/pkg/fi/cloudup/awstasks/subnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,15 @@ func (s *Subnet) CheckChanges(a, e, changes *Subnet) error {
if a != nil {
// TODO: Do we want to destroy & recreate the subnet when theses immutable fields change?
if changes.VPC != nil {
errors = append(errors, fi.FieldIsImmutable(a.VPC, e.VPC, fieldPath.Child("VPC")))
var aID *string
if a.VPC != nil {
aID = a.VPC.ID
}
var eID *string
if e.VPC != nil {
eID = e.VPC.ID
}
errors = append(errors, fi.FieldIsImmutable(eID, aID, fieldPath.Child("VPC")))
}
if changes.AvailabilityZone != nil {
errors = append(errors, fi.FieldIsImmutable(a.AvailabilityZone, e.AvailabilityZone, fieldPath.Child("AvailabilityZone")))
Expand Down
3 changes: 2 additions & 1 deletion upup/pkg/fi/cloudup/awstasks/vpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/golang/glog"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
Expand Down Expand Up @@ -124,7 +125,7 @@ func (s *VPC) CheckChanges(a, e, changes *VPC) error {
if a != nil {
if changes.CIDR != nil {
// TODO: Do we want to destroy & recreate the VPC?
return fi.CannotChangeField("CIDR")
return fi.FieldIsImmutable(e.CIDR, a.CIDR, field.NewPath("CIDR"))
}
}
return nil
Expand Down