From 0978cd97e840bafd1e46d9248768acddc075e7fe Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Sun, 27 Sep 2020 20:08:09 -0500 Subject: [PATCH 1/3] Add second additionalNetworkCIDR w/ incorrect test output --- .../update_cluster/complex/cloudformation.json | 11 ++++++++++- .../update_cluster/complex/in-legacy-v1alpha2.yaml | 1 + .../update_cluster/complex/in-v1alpha2.yaml | 1 + .../integration/update_cluster/complex/kubernetes.tf | 7 ++++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/integration/update_cluster/complex/cloudformation.json b/tests/integration/update_cluster/complex/cloudformation.json index 9e40b2886e..65175b18ab 100644 --- a/tests/integration/update_cluster/complex/cloudformation.json +++ b/tests/integration/update_cluster/complex/cloudformation.json @@ -931,7 +931,16 @@ "VpcId": { "Ref": "AWSEC2VPCcomplexexamplecom" }, - "CidrBlock": "10.1.0.0/16" + "CidrBlock": "10.2.0.0/16" + } + }, + "AWSEC2VPCCidrBlock1020016": { + "Type": "AWS::EC2::VPCCidrBlock", + "Properties": { + "VpcId": { + "Ref": "AWSEC2VPCcomplexexamplecom" + }, + "CidrBlock": "10.2.0.0/16" } }, "AWSEC2VPCDHCPOptionsAssociationcomplexexamplecom": { diff --git a/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml b/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml index d9f1182bec..5092141aef 100644 --- a/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml +++ b/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml @@ -42,6 +42,7 @@ spec: networkCIDR: 172.20.0.0/16 additionalNetworkCIDRs: - 10.1.0.0/16 + - 10.2.0.0/16 networking: kubenet: {} nodePortAccess: diff --git a/tests/integration/update_cluster/complex/in-v1alpha2.yaml b/tests/integration/update_cluster/complex/in-v1alpha2.yaml index 9386163c96..34272bff8e 100644 --- a/tests/integration/update_cluster/complex/in-v1alpha2.yaml +++ b/tests/integration/update_cluster/complex/in-v1alpha2.yaml @@ -42,6 +42,7 @@ spec: networkCIDR: 172.20.0.0/16 additionalNetworkCIDRs: - 10.1.0.0/16 + - 10.2.0.0/16 networking: kubenet: {} nodePortAccess: diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf index 98ed770b94..7c3d53a6ea 100644 --- a/tests/integration/update_cluster/complex/kubernetes.tf +++ b/tests/integration/update_cluster/complex/kubernetes.tf @@ -732,7 +732,12 @@ resource "aws_vpc_dhcp_options" "complex-example-com" { } resource "aws_vpc_ipv4_cidr_block_association" "cidr-10-1-0-0--16" { - cidr_block = "10.1.0.0/16" + cidr_block = "10.2.0.0/16" + vpc_id = aws_vpc.complex-example-com.id +} + +resource "aws_vpc_ipv4_cidr_block_association" "cidr-10-2-0-0--16" { + cidr_block = "10.2.0.0/16" vpc_id = aws_vpc.complex-example-com.id } From 4bcfebebcce88ccf063148d8b29dc0074b7aca41 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Sun, 27 Sep 2020 20:12:09 -0500 Subject: [PATCH 2/3] Fix the detection and rendering of multiple additionalNetworkCIDR blocks --- pkg/model/network.go | 2 +- .../update_cluster/complex/cloudformation.json | 2 +- .../update_cluster/complex/kubernetes.tf | 2 +- upup/pkg/fi/cloudup/awstasks/vpccidrblock.go | 13 ++++++++++++- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/model/network.go b/pkg/model/network.go index 314189e5be..747cfd846e 100644 --- a/pkg/model/network.go +++ b/pkg/model/network.go @@ -93,7 +93,7 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error { Lifecycle: b.Lifecycle, VPC: b.LinkToVPC(), Shared: fi.Bool(sharedVPC), - CIDRBlock: &cidr, + CIDRBlock: s(cidr), }) } } diff --git a/tests/integration/update_cluster/complex/cloudformation.json b/tests/integration/update_cluster/complex/cloudformation.json index 65175b18ab..fbf0f4094b 100644 --- a/tests/integration/update_cluster/complex/cloudformation.json +++ b/tests/integration/update_cluster/complex/cloudformation.json @@ -931,7 +931,7 @@ "VpcId": { "Ref": "AWSEC2VPCcomplexexamplecom" }, - "CidrBlock": "10.2.0.0/16" + "CidrBlock": "10.1.0.0/16" } }, "AWSEC2VPCCidrBlock1020016": { diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf index 7c3d53a6ea..edf1cfe06d 100644 --- a/tests/integration/update_cluster/complex/kubernetes.tf +++ b/tests/integration/update_cluster/complex/kubernetes.tf @@ -732,7 +732,7 @@ resource "aws_vpc_dhcp_options" "complex-example-com" { } resource "aws_vpc_ipv4_cidr_block_association" "cidr-10-1-0-0--16" { - cidr_block = "10.2.0.0/16" + cidr_block = "10.1.0.0/16" vpc_id = aws_vpc.complex-example-com.id } diff --git a/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go b/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go index 37b35e01aa..8f92e3dd3e 100644 --- a/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go +++ b/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go @@ -48,10 +48,21 @@ func (e *VPCCIDRBlock) Find(c *fi.Context) (*VPCCIDRBlock, error) { return nil, err } + found := false + for _, cba := range vpc.CidrBlockAssociationSet { + if fi.StringValue(cba.CidrBlock) == fi.StringValue(e.CIDRBlock) { + found = true + break + } + } + if !found { + return nil, nil + } + actual := &VPCCIDRBlock{ CIDRBlock: e.CIDRBlock, + VPC: &VPC{ID: vpc.VpcId}, } - actual.VPC = &VPC{ID: vpc.VpcId} // Prevent spurious changes actual.Shared = e.Shared From db1b4e301c7b1a6040cd0734b8ba95cb8a55f3c7 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Tue, 29 Sep 2020 22:02:48 -0500 Subject: [PATCH 3/3] Reconcile deletion of VPC CIDR block associations --- pkg/model/network.go | 1 + upup/pkg/fi/cloudup/awstasks/vpc.go | 83 ++++++++++++++++++++ upup/pkg/fi/cloudup/awstasks/vpccidrblock.go | 3 +- upup/pkg/fi/cloudup/awsup/aws_cloud.go | 8 +- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/pkg/model/network.go b/pkg/model/network.go index 747cfd846e..4cabcc73c2 100644 --- a/pkg/model/network.go +++ b/pkg/model/network.go @@ -73,6 +73,7 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error { // but seems safer to stick with existing behaviour t.EnableDNSHostnames = fi.Bool(true) + t.AssociateExtraCIDRBlocks = b.Cluster.Spec.AdditionalNetworkCIDRs } if b.Cluster.Spec.NetworkID != "" { diff --git a/upup/pkg/fi/cloudup/awstasks/vpc.go b/upup/pkg/fi/cloudup/awstasks/vpc.go index d7d9d6f383..d92ceadeb0 100644 --- a/upup/pkg/fi/cloudup/awstasks/vpc.go +++ b/upup/pkg/fi/cloudup/awstasks/vpc.go @@ -44,9 +44,15 @@ type VPC struct { Shared *bool Tags map[string]string + + // AssociateExtraCIDRBlocks contains a list of cidr blocks that should be + // associated with the VPC; any other CIDR blocks should be disassociated. + // The associations themselves are created through the VPCCIDRBlock awstask. + AssociateExtraCIDRBlocks []string } var _ fi.CompareWithID = &VPC{} +var _ fi.ProducesDeletions = &VPC{} func (e *VPC) CompareWithID() *string { return e.ID @@ -109,6 +115,7 @@ func (e *VPC) Find(c *fi.Context) (*VPC, error) { } actual.Lifecycle = e.Lifecycle actual.Name = e.Name // Name is part of Tags + actual.AssociateExtraCIDRBlocks = e.AssociateExtraCIDRBlocks return actual, nil } @@ -194,6 +201,53 @@ func (_ *VPC) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *VPC) error { return t.AddAWSTags(*e.ID, e.Tags) } +func (e *VPC) FindDeletions(c *fi.Context) ([]fi.Deletion, error) { + if fi.StringValue(e.ID) == "" { + return nil, nil + } + + var removals []fi.Deletion + request := &ec2.DescribeVpcsInput{ + VpcIds: []*string{e.ID}, + } + cloud := c.Cloud.(awsup.AWSCloud) + response, err := cloud.EC2().DescribeVpcs(request) + if err != nil { + return nil, err + } + if response == nil || len(response.Vpcs) == 0 { + return nil, nil + } + + if len(response.Vpcs) != 1 { + return nil, fmt.Errorf("found multiple VPCs matching tags") + } + vpc := response.Vpcs[0] + for _, association := range vpc.CidrBlockAssociationSet { + // We'll only delete CIDR associations that are not the primary association + // and that have a state of "associated" + if fi.StringValue(association.CidrBlock) == fi.StringValue(vpc.CidrBlock) || + association.CidrBlockState != nil && fi.StringValue(association.CidrBlockState.State) != ec2.VpcCidrBlockStateCodeAssociated { + continue + } + match := false + for _, cidr := range e.AssociateExtraCIDRBlocks { + if fi.StringValue(association.CidrBlock) == cidr { + match = true + break + } + } + if !match { + removals = append(removals, &deleteVPCCIDRBlock{ + vpcID: vpc.VpcId, + cidrBlock: association.CidrBlock, + associationID: association.AssociationId, + }) + } + } + return removals, nil +} + type terraformVPC struct { CIDR *string `json:"cidr_block,omitempty" cty:"cidr_block"` EnableDNSHostnames *bool `json:"enable_dns_hostnames,omitempty" cty:"enable_dns_hostnames"` @@ -280,3 +334,32 @@ func (e *VPC) CloudformationLink() *cloudformation.Literal { return cloudformation.Ref("AWS::EC2::VPC", *e.Name) } + +type deleteVPCCIDRBlock struct { + vpcID *string + cidrBlock *string + associationID *string +} + +var _ fi.Deletion = &deleteVPCCIDRBlock{} + +func (d *deleteVPCCIDRBlock) Delete(t fi.Target) error { + + awsTarget, ok := t.(*awsup.AWSAPITarget) + if !ok { + return fmt.Errorf("unexpected target type for deletion: %T", t) + } + request := &ec2.DisassociateVpcCidrBlockInput{ + AssociationId: d.associationID, + } + _, err := awsTarget.Cloud.EC2().DisassociateVpcCidrBlock(request) + return err +} + +func (d *deleteVPCCIDRBlock) TaskName() string { + return "VPCCIDRBlock" +} + +func (d *deleteVPCCIDRBlock) Item() string { + return fmt.Sprintf("%v: cidr=%v", *d.vpcID, *d.cidrBlock) +} diff --git a/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go b/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go index 8f92e3dd3e..9b7981b808 100644 --- a/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go +++ b/upup/pkg/fi/cloudup/awstasks/vpccidrblock.go @@ -50,7 +50,8 @@ func (e *VPCCIDRBlock) Find(c *fi.Context) (*VPCCIDRBlock, error) { found := false for _, cba := range vpc.CidrBlockAssociationSet { - if fi.StringValue(cba.CidrBlock) == fi.StringValue(e.CIDRBlock) { + if fi.StringValue(cba.CidrBlock) == fi.StringValue(e.CIDRBlock) && + cba.CidrBlockState != nil && fi.StringValue(cba.CidrBlockState.State) == ec2.VpcCidrBlockStateCodeAssociated { found = true break } diff --git a/upup/pkg/fi/cloudup/awsup/aws_cloud.go b/upup/pkg/fi/cloudup/awsup/aws_cloud.go index ff18398bd1..5000bc4c7d 100644 --- a/upup/pkg/fi/cloudup/awsup/aws_cloud.go +++ b/upup/pkg/fi/cloudup/awsup/aws_cloud.go @@ -1470,7 +1470,7 @@ func ValidateZones(zones []string, cloud AWSCloud) error { klog.Warningf("Zone %q has message: %q", zone, aws.StringValue(message.Message)) } - if aws.StringValue(z.State) != "available" { + if aws.StringValue(z.State) != ec2.AvailabilityZoneStateAvailable { klog.Warningf("Zone %q has state %q", zone, aws.StringValue(z.State)) } } @@ -1608,9 +1608,9 @@ func (c *awsCloudImplementation) zonesWithInstanceType(instanceType string) (set request := &ec2.DescribeReservedInstancesOfferingsInput{} request.InstanceTenancy = aws.String("default") request.IncludeMarketplace = aws.Bool(false) - request.OfferingClass = aws.String("standard") - request.OfferingType = aws.String("No Upfront") - request.ProductDescription = aws.String("Linux/UNIX (Amazon VPC)") + request.OfferingClass = aws.String(ec2.OfferingClassTypeStandard) + request.OfferingType = aws.String(ec2.OfferingTypeValuesNoUpfront) + request.ProductDescription = aws.String(ec2.RIProductDescriptionLinuxUnixamazonVpc) request.InstanceType = aws.String(instanceType) zones := sets.NewString()