Make NetcfgSubnetRangeOutsideVnet error retryable for VirtualNetworksSubnet (#4931)

* Initial plan

* Implement ErrorClassifier for VirtualNetworksSubnet to make NetcfgSubnetRangeOutsideVnet retryable

Co-authored-by: theunrepentantgeek <[email protected]>

* Format code for copilot

* Simplify testing

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: theunrepentantgeek <[email protected]>
Co-authored-by: Bevan Arps <[email protected]>

Author: Copilot

v2/api/network/customizations/virtual_network_subnet_extensions.go

@@ -13,10 +13,14 @@ import (
 	"github.com/Azure/azure-service-operator/v2/internal/genericarmclient"
 	"github.com/Azure/azure-service-operator/v2/internal/resolver"
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/core"
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
 )
 
-var _ extensions.PostReconciliationChecker = &PrivateEndpointExtension{}
+var (
+	_ extensions.PostReconciliationChecker = &VirtualNetworksSubnetExtension{}
+	_ extensions.ErrorClassifier           = &VirtualNetworksSubnetExtension{}
+)
 
 func (extension *VirtualNetworksSubnetExtension) PostReconcileCheck(
 	_ context.Context,
@@ -50,3 +54,36 @@ func (extension *VirtualNetworksSubnetExtension) PostReconcileCheck(
 
 	return extensions.PostReconcileCheckResultSuccess(), nil
 }
+
+func (extension *VirtualNetworksSubnetExtension) ClassifyError(
+	cloudError *genericarmclient.CloudError,
+	apiVersion string,
+	log logr.Logger,
+	next extensions.ErrorClassifierFunc,
+) (core.CloudErrorDetails, error) {
+	details, err := next(cloudError)
+	if err != nil {
+		return core.CloudErrorDetails{}, err
+	}
+
+	if isRetryableSubnetError(cloudError) {
+		details.Classification = core.ErrorRetryable
+	}
+
+	return details, nil
+}
+
+func isRetryableSubnetError(err *genericarmclient.CloudError) bool {
+	if err == nil {
+		return false
+	}
+
+	// NetcfgSubnetRangeOutsideVnet occurs when a subnet's IP range is outside the VNet's allowed ranges.
+	// This can happen during simultaneous VNet and Subnet changes when the subnet is reconciled before the VNet.
+	// This should be retryable as the VNet may be updated soon to include the required range.
+	if err.Code() == "NetcfgSubnetRangeOutsideVnet" {
+		return true
+	}
+
+	return false
+}

v2/api/network/customizations/virtual_network_subnet_extensions_test.go

@@ -0,0 +1,78 @@
+/*
+Copyright (c) Microsoft Corporation.
+Licensed under the MIT license.
+*/
+
+package customizations
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+
+	"github.com/go-logr/logr"
+
+	"github.com/Azure/azure-service-operator/v2/internal/genericarmclient"
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/core"
+)
+
+func Test_VirtualNetworksSubnetExtension_ClassifyError_NetcfgSubnetRangeOutsideVnet_IsRetryable(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		errorCode    string
+		errorMessage string
+		mockReturns  core.ErrorClassification
+		expected     core.ErrorClassification
+	}{
+		"Fatal error is still fatal": {
+			errorCode:    "BadRequest",
+			errorMessage: "Invalid parameter value",
+			mockReturns:  core.ErrorFatal,
+			expected:     core.ErrorFatal,
+		},
+		"Other error is still retryable": {
+			errorCode:    "OtherError",
+			errorMessage: "Some other error occurred",
+			mockReturns:  core.ErrorRetryable,
+			expected:     core.ErrorRetryable,
+		},
+		"SubnetRangeOutsideVnet error is retryable": {
+			errorCode:    "NetcfgSubnetRangeOutsideVnet",
+			errorMessage: "Subnet 'testsubnet' is not valid because its IP address range is outside the IP address range of virtual network 'testvnet'.",
+			mockReturns:  core.ErrorRetryable,
+			expected:     core.ErrorRetryable,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			g := NewGomegaWithT(t)
+
+			extension := &VirtualNetworksSubnetExtension{}
+
+			// Create a test error with the specific code
+			err := genericarmclient.NewTestCloudError(c.errorCode, c.errorMessage)
+
+			// Mock the next classifier function - this would normally classify as fatal
+			mockCalled := false
+			next := func(cloudError *genericarmclient.CloudError) (core.CloudErrorDetails, error) {
+				mockCalled = true
+				return core.CloudErrorDetails{
+					Classification: c.mockReturns,
+					Code:           cloudError.Code(),
+					Message:        cloudError.Message(),
+				}, nil
+			}
+
+			details, classifyErr := extension.ClassifyError(err, "2024-03-01", logr.Discard(), next)
+
+			g.Expect(classifyErr).ToNot(HaveOccurred())
+			g.Expect(details.Classification).To(Equal(c.expected))
+			g.Expect(details.Code).To(Equal(c.errorCode))
+			g.Expect(details.Message).To(ContainSubstring(c.errorMessage))
+			g.Expect(mockCalled).To(BeTrue())
+		})
+	}
+}