From b541c9d898d3dde983de2e0ea80a40e17d4c9b9f Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curii.com>
Date: Mon, 18 Mar 2024 15:01:16 -0400
Subject: [PATCH] 21603: Recognize subnet error despite generic error code.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---
 lib/cloud/ec2/ec2.go      | 10 +++++++-
 lib/cloud/ec2/ec2_test.go | 51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index 9a3f784b51..6251f18df0 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -13,6 +13,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"math/big"
+	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -713,6 +714,8 @@ func isErrorQuota(err error) bool {
 	return false
 }
 
+var reSubnetSpecificInvalidParameterMessage = regexp.MustCompile(`(?ms).*( subnet |sufficient free [Ii]pv[46] addresses).*`)
+
 // isErrorSubnetSpecific returns true if the problem encountered by
 // RunInstances might be avoided by trying a different subnet.
 func isErrorSubnetSpecific(err error) bool {
@@ -724,7 +727,12 @@ func isErrorSubnetSpecific(err error) bool {
 	return strings.Contains(code, "Subnet") ||
 		code == "InsufficientInstanceCapacity" ||
 		code == "InsufficientVolumeCapacity" ||
-		code == "Unsupported"
+		code == "Unsupported" ||
+		// See TestIsErrorSubnetSpecific for examples of why
+		// we look for substrings in code/message instead of
+		// only using specific codes here.
+		(strings.Contains(code, "InvalidParameter") &&
+			reSubnetSpecificInvalidParameterMessage.MatchString(aerr.Message()))
 }
 
 // isErrorCapacity returns true if the error indicates lack of
diff --git a/lib/cloud/ec2/ec2_test.go b/lib/cloud/ec2/ec2_test.go
index d342f0fb30..5e6cf2c82b 100644
--- a/lib/cloud/ec2/ec2_test.go
+++ b/lib/cloud/ec2/ec2_test.go
@@ -363,6 +363,57 @@ func (*EC2InstanceSetSuite) TestCreateFailoverSecondSubnet(c *check.C) {
 		`.*`)
 }
 
+func (*EC2InstanceSetSuite) TestIsErrorSubnetSpecific(c *check.C) {
+	c.Check(isErrorSubnetSpecific(nil), check.Equals, false)
+	c.Check(isErrorSubnetSpecific(errors.New("misc error")), check.Equals, false)
+
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code: "InsufficientInstanceCapacity",
+	}), check.Equals, true)
+
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code: "InsufficientVolumeCapacity",
+	}), check.Equals, true)
+
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code:    "InsufficientFreeAddressesInSubnet",
+		message: "Not enough free addresses in subnet subnet-abcdefg\n\tstatus code: 400, request id: abcdef01-2345-6789-abcd-ef0123456789",
+	}), check.Equals, true)
+
+	// #21603: (Sometimes?) EC2 returns code InvalidParameterValue
+	// even though the code "InsufficientFreeAddressesInSubnet"
+	// seems like it must be meant for exactly this error.
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code:    "InvalidParameterValue",
+		message: "Not enough free addresses in subnet subnet-abcdefg\n\tstatus code: 400, request id: abcdef01-2345-6789-abcd-ef0123456789",
+	}), check.Equals, true)
+
+	// Similarly, AWS docs
+	// (https://repost.aws/knowledge-center/vpc-insufficient-ip-errors)
+	// suggest the following code/message combinations also exist.
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code:    "Client.InvalidParameterValue",
+		message: "There aren't sufficient free Ipv4 addresses or prefixes",
+	}), check.Equals, true)
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code:    "InvalidParameterValue",
+		message: "There aren't sufficient free Ipv4 addresses or prefixes",
+	}), check.Equals, true)
+	// Meanwhile, other AWS docs
+	// (https://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html)
+	// suggest Client.InvalidParameterValue is not a real code but
+	// ClientInvalidParameterValue is.
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code:    "ClientInvalidParameterValue",
+		message: "There aren't sufficient free Ipv4 addresses or prefixes",
+	}), check.Equals, true)
+
+	c.Check(isErrorSubnetSpecific(&ec2stubError{
+		code:    "InvalidParameterValue",
+		message: "Some other invalid parameter error",
+	}), check.Equals, false)
+}
+
 func (*EC2InstanceSetSuite) TestCreateAllSubnetsFailing(c *check.C) {
 	if *live != "" {
 		c.Skip("not applicable in live mode")
-- 
2.30.2