From d6b19436bf8bf5e7893621c5d6ea59c8c8ae8197 Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Wed, 7 Nov 2018 12:01:18 +0000 Subject: [PATCH] Create `vpc.ImportSubnets` - refactor all VPC helpers into new package - make sure to only use the provider interface --- pkg/cfn/builder/api_test.go | 7 ++-- pkg/cfn/builder/cluster.go | 4 +-- pkg/ctl/create/cluster.go | 7 ++-- pkg/eks/api/vpc.go | 7 ++-- pkg/kops/kops.go | 43 ++++++++++-------------- pkg/{eks => vpc}/vpc.go | 66 ++++++++++++++++++++++++++++--------- 6 files changed, 83 insertions(+), 51 deletions(-) rename pkg/{eks => vpc}/vpc.go (52%) diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go index 0fb2ac8824..d0b7ab1741 100644 --- a/pkg/cfn/builder/api_test.go +++ b/pkg/cfn/builder/api_test.go @@ -13,10 +13,9 @@ import ( . "github.com/onsi/gomega" . "github.com/weaveworks/eksctl/pkg/cfn/builder" "github.com/weaveworks/eksctl/pkg/cloudconfig" - "github.com/weaveworks/eksctl/pkg/eks" "github.com/weaveworks/eksctl/pkg/eks/api" "github.com/weaveworks/eksctl/pkg/nodebootstrap" - "github.com/weaveworks/eksctl/pkg/testutils" + "github.com/weaveworks/eksctl/pkg/vpc" ) const ( @@ -200,10 +199,10 @@ var _ = Describe("CloudFormation template builder API", func() { } cfg := newClusterConfig() - ctl := eks.New(testutils.ProviderConfig, cfg) + //ctl := eks.New(testutils.ProviderConfig, cfg) It("should not error when calling SetSubnets", func() { - err := ctl.SetSubnets(cfg) + err := vpc.SetSubnets(cfg) Expect(err).ShouldNot(HaveOccurred()) }) diff --git a/pkg/cfn/builder/cluster.go b/pkg/cfn/builder/cluster.go index c3cd810cd6..07f92a9e27 100644 --- a/pkg/cfn/builder/cluster.go +++ b/pkg/cfn/builder/cluster.go @@ -101,11 +101,11 @@ func (c *ClusterResourceSet) GetAllOutputs(stack cfn.Stack) error { // TODO: shouldn't assume the order - https://github.com/weaveworks/eksctl/issues/293 for i, subnet := range c.outputs.SubnetsPrivate { - c.spec.ImportSubnet(api.SubnetTopologyPrivate, c.spec.AvailabilityZones[i], subnet) + c.spec.ImportSubnet(api.SubnetTopologyPrivate, c.spec.AvailabilityZones[i], subnet, "") } for i, subnet := range c.outputs.SubnetsPublic { - c.spec.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet) + c.spec.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet, "") } c.spec.ClusterStackName = c.outputs.ClusterStackName diff --git a/pkg/ctl/create/cluster.go b/pkg/ctl/create/cluster.go index 0a5f9c068d..1508fb7375 100644 --- a/pkg/ctl/create/cluster.go +++ b/pkg/ctl/create/cluster.go @@ -15,6 +15,7 @@ import ( "github.com/weaveworks/eksctl/pkg/kops" "github.com/weaveworks/eksctl/pkg/utils" "github.com/weaveworks/eksctl/pkg/utils/kubeconfig" + "github.com/weaveworks/eksctl/pkg/vpc" ) const ( @@ -157,7 +158,7 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.Node if err := ctl.SetAvailabilityZones(cfg, availabilityZones); err != nil { return err } - if err := ctl.SetSubnets(cfg); err != nil { + if err := vpc.SetSubnets(cfg); err != nil { return err } return nil @@ -178,7 +179,7 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.Node return err } - if err := kw.UseVPC(cfg); err != nil { + if err := kw.UseVPC(ctl.Provider, cfg); err != nil { return err } @@ -198,7 +199,7 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.Node } for topology := range subnets { - if err := ctl.UseSubnets(cfg, topology, *subnets[topology]); err != nil { + if err := vpc.UseSubnets(ctl.Provider, cfg, topology, *subnets[topology]); err != nil { return err } } diff --git a/pkg/eks/api/vpc.go b/pkg/eks/api/vpc.go index 49786e5711..d8bcd64cab 100644 --- a/pkg/eks/api/vpc.go +++ b/pkg/eks/api/vpc.go @@ -57,7 +57,9 @@ func (c *ClusterConfig) SubnetIDs(topology SubnetTopology) []string { } // ImportSubnet loads a given subnet into cluster config -func (c *ClusterConfig) ImportSubnet(topology SubnetTopology, az, subnetID string) { +func (c *ClusterConfig) ImportSubnet(topology SubnetTopology, az, subnetID, cidr string) { + _, subnetCIDR, _ := net.ParseCIDR(cidr) + if c.VPC.Subnets == nil { c.VPC.Subnets = make(map[SubnetTopology]map[string]Network) } @@ -65,9 +67,10 @@ func (c *ClusterConfig) ImportSubnet(topology SubnetTopology, az, subnetID strin c.VPC.Subnets[topology] = map[string]Network{} } if network, ok := c.VPC.Subnets[topology][az]; !ok { - c.VPC.Subnets[topology][az] = Network{ID: subnetID} + c.VPC.Subnets[topology][az] = Network{ID: subnetID, CIDR: subnetCIDR} } else { network.ID = subnetID + network.CIDR = subnetCIDR c.VPC.Subnets[topology][az] = network } } diff --git a/pkg/kops/kops.go b/pkg/kops/kops.go index adcf683960..8e6ac2e664 100644 --- a/pkg/kops/kops.go +++ b/pkg/kops/kops.go @@ -1,14 +1,16 @@ package kops import ( - "fmt" - - "github.com/aws/aws-sdk-go/service/ec2" "github.com/kubicorn/kubicorn/pkg/logger" "github.com/pkg/errors" + "github.com/weaveworks/eksctl/pkg/eks/api" + "github.com/weaveworks/eksctl/pkg/vpc" + "k8s.io/kops/pkg/resources/aws" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" + + "github.com/aws/aws-sdk-go/service/ec2" ) // Wrapper for interacting with a kops cluster @@ -41,41 +43,32 @@ func (k *Wrapper) topologyOf(s *ec2.Subnet) api.SubnetTopology { } // UseVPC finds VPC and subnets that give kops cluster uses and add those to EKS cluster config -func (k *Wrapper) UseVPC(spec *api.ClusterConfig) error { - allVPCs, err := aws.ListVPCs(k.cloud, k.clusterName) - if err != nil { - return err - } - +func (k *Wrapper) UseVPC(provider api.ClusterProvider, spec *api.ClusterConfig) error { allSubnets, err := aws.ListSubnets(k.cloud, k.clusterName) if err != nil { return err } - vpcs := []string{} - for _, vpc := range allVPCs { - vpc := vpc.Obj.(*ec2.Vpc) - for _, tag := range vpc.Tags { - if k.isOwned(tag) { - vpcs = append(vpcs, *vpc.VpcId) - } - } - } - logger.Debug("vpcs = %#v", vpcs) - if len(vpcs) > 1 { - return fmt.Errorf("more then one VPC found for kops cluster %q", k.clusterName) + subnetsByTopology := map[api.SubnetTopology][]*ec2.Subnet{ + api.SubnetTopologyPrivate: {}, + api.SubnetTopologyPublic: {}, } - spec.VPC.ID = vpcs[0] for _, subnet := range allSubnets { subnet := subnet.Obj.(*ec2.Subnet) for _, tag := range subnet.Tags { - if k.isOwned(tag) && *subnet.VpcId == spec.VPC.ID { - spec.ImportSubnet(k.topologyOf(subnet), *subnet.AvailabilityZone, *subnet.SubnetId) - spec.AppendAvailabilityZone(*subnet.AvailabilityZone) + if k.isOwned(tag) { + t := k.topologyOf(subnet) + subnetsByTopology[t] = append(subnetsByTopology[t], subnet) } } } + for t, subnets := range subnetsByTopology { + if err := vpc.ImportSubnets(provider, spec, t, subnets); err != nil { + return err + } + } + logger.Debug("subnets = %#v", spec.VPC.Subnets) if err := spec.HasSufficientSubnets(); err != nil { return errors.Wrapf(err, "using VPC from kops cluster %q", k.clusterName) diff --git a/pkg/eks/vpc.go b/pkg/vpc/vpc.go similarity index 52% rename from pkg/eks/vpc.go rename to pkg/vpc/vpc.go index 0f44583b7d..0ecab5a11f 100644 --- a/pkg/eks/vpc.go +++ b/pkg/vpc/vpc.go @@ -1,20 +1,21 @@ -package eks +package vpc import ( "fmt" - "strings" + "net" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/kubicorn/kubicorn/pkg/logger" "github.com/weaveworks/eksctl/pkg/eks/api" + "k8s.io/kops/pkg/util/subnet" ) // SetSubnets defines CIDRs for each of the subnets, // it must be called after SetAvailabilityZones -func (c *ClusterProvider) SetSubnets(spec *api.ClusterConfig) error { +func SetSubnets(spec *api.ClusterConfig) error { var err error vpc := spec.VPC @@ -53,29 +54,64 @@ func (c *ClusterProvider) SetSubnets(spec *api.ClusterConfig) error { return nil } -// UseSubnets imports -func (c *ClusterProvider) UseSubnets(spec *api.ClusterConfig, topology api.SubnetTopology, subnetIDs []string) error { - if len(subnetIDs) == 0 { - return nil - } +func describeSubnets(porvider api.ClusterProvider, subnetIDs ...string) ([]*ec2.Subnet, error) { input := &ec2.DescribeSubnetsInput{ SubnetIds: aws.StringSlice(subnetIDs), } - output, err := c.Provider.EC2().DescribeSubnets(input) + output, err := porvider.EC2().DescribeSubnets(input) if err != nil { - return err + return nil, err } + return output.Subnets, nil +} + +func describeVPC(povider api.ClusterProvider, vpcID string) (*ec2.Vpc, error) { + input := &ec2.DescribeVpcsInput{ + VpcIds: []*string{aws.String(vpcID)}, + } + output, err := povider.EC2().DescribeVpcs(input) + if err != nil { + return nil, err + } + return output.Vpcs[0], nil +} - for _, subnet := range output.Subnets { +// ImportSubnets will update spec with subnets, if VPC ID/CIDR is unknown +// it will use provider to call describeVPC based on the VPC ID of the +// first subnet; all subnets must be in the same VPC +func ImportSubnets(provider api.ClusterProvider, spec *api.ClusterConfig, topology api.SubnetTopology, subnets []*ec2.Subnet) error { + for _, subnet := range subnets { if spec.VPC.ID == "" { - spec.VPC.ID = *subnet.VpcId + vpc, err := describeVPC(provider, *subnet.VpcId) + if err != nil { + return err + } + spec.VPC.ID = *vpc.VpcId + _, spec.VPC.CIDR, err = net.ParseCIDR(*vpc.CidrBlock) + if err != nil { + return err + } } else if spec.VPC.ID != *subnet.VpcId { - return fmt.Errorf("given subnets (%s) are not in the same VPC", strings.Join(subnetIDs, ", ")) + return fmt.Errorf("given %s is in %s, not in %s", *subnet.SubnetId, *subnet.VpcId, spec.VPC.ID) } - spec.ImportSubnet(topology, *subnet.AvailabilityZone, *subnet.SubnetId) + spec.ImportSubnet(topology, *subnet.AvailabilityZone, *subnet.SubnetId, *subnet.CidrBlock) spec.AppendAvailabilityZone(*subnet.AvailabilityZone) } - return nil } + +// UseSubnets will update spec with subnets, it will call describeSubnets first, +// then pass resulting subnets to ImportSubnets +func UseSubnets(provider api.ClusterProvider, spec *api.ClusterConfig, topology api.SubnetTopology, subnetIDs []string) error { + if len(subnetIDs) == 0 { + return nil + } + + subnets, err := describeSubnets(provider, subnetIDs...) + if err != nil { + return err + } + + return ImportSubnets(provider, spec, topology, subnets) +}