search

lyft / cni-ipvlan-vpc-k8s

AWS VPC Kubernetes CNI driver using IPvlan
Apache License 2.0
360 stars 58 forks source link

fixes nil pointer when other VPCs with peering connections are found #27

Closed chris-h-phillips closed 6 years ago

chris-h-phillips commented 6 years ago

We run multiple VPCs in our AWS accounts and most of them have some peering connection of some kind. Attempting to use the routeToVpcPeers option failed like this:

github.com/lyft/cni-ipvlan-vpc-k8s/aws.(*vpcclient).DescribeVPCPeerCIDRs(0xc42000e3a8, 0xc42021d850, 0xc, 0xc420336900, 0x2, 0xc420314360, 0x16, 0xc4203c2000)
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/aws/vpc.go:115 +0x298
github.com/lyft/cni-ipvlan-vpc-k8s/aws.(*vpcCacheClient).DescribeVPCPeerCIDRs(0xc4202bab60, 0xc42021d850, 0xc, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/aws/vpc.go:45 +0x152
main.actionVpcPeerCidr(0xc42008cb00, 0x0, 0xc42008cb00)
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/cmd/cni-ipvlan-vpc-k8s-tool/cni-ipvlan-vpc-k8s-tool.go:244 +0x1c5
github.com/lyft/cni-ipvlan-vpc-k8s/vendor/github.com/urfave/cli.HandleAction(0xa85ac0, 0xc09c70, 0xc42008cb00, 0xc420248d00, 0x0)
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/vendor/github.com/urfave/cli/app.go:490 +0xd2
github.com/lyft/cni-ipvlan-vpc-k8s/vendor/github.com/urfave/cli.Command.Run(0xbec0aa, 0xb, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc02b07, 0x3c, 0x0, ...)
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/vendor/github.com/urfave/cli/command.go:210 +0xa95
github.com/lyft/cni-ipvlan-vpc-k8s/vendor/github.com/urfave/cli.(*App).Run(0xc420138ea0, 0xc42000c060, 0x2, 0x2, 0x0, 0x0)
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/vendor/github.com/urfave/cli/app.go:255 +0x6f8
main.main()
    /home/atrus/gopath/src/github.com/lyft/cni-ipvlan-vpc-k8s/cmd/cni-ipvlan-vpc-k8s-tool/cni-ipvlan-vpc-k8s-tool.go:433 +0x690

Allowing DescribeVPCPeerCIDRs to ignore peering connections where the requester and accepter are both unrelated vpcs fixes this bug

theatrus commented 6 years ago

Change looks good - can take a look at our CLA (https://oss.lyft.com/cla/#!/signature )? Thanks :)

chris-h-phillips commented 6 years ago

No problem I signed just now 👍

theatrus commented 6 years ago

Sorry for the delay merging - we broke some things internally but now good to go. I'll cut a point release for convenience shortly

chris-h-phillips commented 6 years ago

No worries thanks for the update

theatrus commented 6 years ago

Release posted. Thanks