Refactor and fix NLB subnet change checks

Move checks for valid subnet operations into CheckChanges. This also fixes
a bug where changes would cause immutable field errors while it's actually
perfectly fine to add new subnets (only detaching is forbidden).

This also commit changes the actualSubnets and expectedSubnets lists to
be maps of *string. This is in preparation for the next commit that then
relies on it being a map.
This commit is contained in:
Alexander Block 2021-02-05 12:39:47 +01:00
parent beb8b62746
commit 190073e766
1 changed files with 22 additions and 17 deletions

View File

@ -26,13 +26,11 @@ import (
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/elbv2"
"github.com/aws/aws-sdk-go/service/route53"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
"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"
"k8s.io/kops/util/pkg/slice"
)
// NetworkLoadBalancer manages an NLB. We find the existing NLB using the Name tag.
@ -517,7 +515,16 @@ func (s *NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) e
}
} else {
if len(changes.Subnets) > 0 {
return fi.FieldIsImmutable(e.Subnets, a.Subnets, field.NewPath("Subnets"))
expectedSubnets := make(map[string]*string)
for _, s := range e.Subnets {
expectedSubnets[*s.ID] = nil
}
for _, s := range a.Subnets {
if _, ok := expectedSubnets[*s.ID]; !ok {
return fmt.Errorf("network load balancers do not support detaching subnets")
}
}
}
}
return nil
@ -596,27 +603,25 @@ func (_ *NetworkLoadBalancer) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Ne
loadBalancerArn = fi.StringValue(lb.LoadBalancerArn)
if changes.Subnets != nil {
var expectedSubnets []string
for _, s := range e.Subnets {
expectedSubnets = append(expectedSubnets, fi.StringValue(s.ID))
}
var actualSubnets []string
actualSubnets := make(map[string]*string)
for _, s := range a.Subnets {
actualSubnets = append(actualSubnets, fi.StringValue(s.ID))
actualSubnets[*s.ID] = nil
}
oldSubnetIDs := slice.GetUniqueStrings(expectedSubnets, actualSubnets)
if len(oldSubnetIDs) > 0 {
return fmt.Errorf("network load balancers do not support detaching subnets")
var awsSubnets []*string
hasChanges := false
for _, s := range e.Subnets {
_, ok := actualSubnets[*s.ID]
if !ok {
hasChanges = true
}
awsSubnets = append(awsSubnets, s.ID)
}
newSubnetIDs := slice.GetUniqueStrings(actualSubnets, expectedSubnets)
if len(newSubnetIDs) > 0 {
if hasChanges {
request := &elbv2.SetSubnetsInput{}
request.SetLoadBalancerArn(loadBalancerArn)
request.SetSubnets(aws.StringSlice(append(actualSubnets, newSubnetIDs...)))
request.SetSubnets(awsSubnets)
klog.V(2).Infof("Attaching Load Balancer to new subnets")
if _, err := t.Cloud.ELBV2().SetSubnets(request); err != nil {