Merge pull request #293 from oracle/mp/torontoSupport

Remove check for number of subnets

Author: MadalinaPatrichi

pkg/cloudprovider/providers/oci/load_balancer.go

@@ -323,7 +323,12 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
 		secretBackendSetString := service.Annotations[ServiceAnnotationLoadBalancerTLSBackendSetSecret]
 		sslConfig = NewSSLConfig(secretListenerString, secretBackendSetString, ports, cp)
 	}
-	subnets := []string{cp.config.LoadBalancer.Subnet1, cp.config.LoadBalancer.Subnet2}
+	var subnets []string
+	if cp.config.LoadBalancer.Subnet2 != "" {
+		subnets = []string{cp.config.LoadBalancer.Subnet1, cp.config.LoadBalancer.Subnet2}
+	} else {
+		subnets = []string{cp.config.LoadBalancer.Subnet1}
+	}
 	spec, err := NewLBSpec(service, nodes, subnets, sslConfig, cp.securityListManagerFactory)
 	if err != nil {
 		logger.With(zap.Error(err)).Error("Failed to derive LBSpec")

pkg/cloudprovider/providers/oci/load_balancer_spec.go

@@ -95,9 +95,11 @@ type LBSpec struct {
 
 // NewLBSpec creates a LB Spec from a Kubernetes service and a slice of nodes.
 func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, sslConfig *SSLConfig, secListFactory securityListManagerFactory) (*LBSpec, error) {
-	if len(defaultSubnets) != 2 {
-		return nil, errors.New("default subnets incorrectly configured")
-	}
+	// Disable check for whether there are two subnets, rely on OCI to decide whether the number of subnets is correct
+	// This allows LoadBalancers to be created in single AD regions
+	// if len(defaultSubnets) != 2 {
+	// 	return nil, errors.New("default subnets incorrectly configured")
+	// }
 
 	if err := validateService(svc); err != nil {
 		return nil, errors.Wrap(err, "invalid service")
@@ -134,11 +136,14 @@ func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, sslCo
 			return nil, errors.Errorf("a configuration for subnet1 must be specified for an internal load balancer")
 		}
 		subnets = subnets[:1]
-	} else {
-		if subnets[0] == "" || subnets[1] == "" {
-			return nil, errors.Errorf("a configuration for both subnets must be specified")
-		}
 	}
+	// Disable check for whether there are two subnets, rely on OCI to decide whether the number of subnets is correct
+	// This allows LoadBalancers to be created in single AD regions
+	// else {
+	// 	if subnets[0] == "" || subnets[1] == "" {
+	// 		return nil, errors.Errorf("a configuration for both subnets must be specified")
+	// 	}
+	// }
 
 	listeners, err := getListeners(svc, sslConfig)
 	if err != nil {

pkg/cloudprovider/providers/oci/load_balancer_spec_test.go

@@ -503,6 +503,90 @@ func TestNewLBSpecSuccess(t *testing.T) {
 	}
 }
 
+func TestNewLBSpecSingleAD(t *testing.T) {
+	testCases := map[string]struct {
+		defaultSubnetOne string
+		defaultSubnetTwo string
+		nodes            []*v1.Node
+		service          *v1.Service
+		expected         *LBSpec
+	}{
+		"single subnet for single AD": {
+			defaultSubnetOne: "one",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "kube-system",
+					Name:      "testservice",
+					UID:       "test-uid",
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerBEProtocol: "",
+						ServiceAnnotationLoadBalancerSubnet1:    "annotation-one",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					SessionAffinity: v1.ServiceAffinityNone,
+					Ports: []v1.ServicePort{
+						v1.ServicePort{
+							Protocol: v1.ProtocolTCP,
+							Port:     int32(80),
+						},
+					},
+				},
+			},
+			expected: &LBSpec{
+				Name:     "test-uid",
+				Shape:    "100Mbps",
+				Internal: false,
+				Subnets:  []string{"annotation-one"},
+				Listeners: map[string]loadbalancer.ListenerDetails{
+					"TCP-80": loadbalancer.ListenerDetails{
+						DefaultBackendSetName: common.String("TCP-80"),
+						Port:                  common.Int(80),
+						Protocol:              common.String("TCP"),
+					},
+				},
+				BackendSets: map[string]loadbalancer.BackendSetDetails{
+					"TCP-80": loadbalancer.BackendSetDetails{
+						Backends: []loadbalancer.BackendDetails{},
+						HealthChecker: &loadbalancer.HealthCheckerDetails{
+							Protocol: common.String("HTTP"),
+							Port:     common.Int(10256),
+							UrlPath:  common.String("/healthz"),
+						},
+						Policy: common.String("ROUND_ROBIN"),
+					},
+				},
+				SourceCIDRs: []string{"0.0.0.0/0"},
+				Ports: map[string]portSpec{
+					"TCP-80": portSpec{
+						ListenerPort:      80,
+						HealthCheckerPort: 10256,
+					},
+				},
+				securityListManager: newSecurityListManagerNOOP(),
+			},
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			// we expect the service to be unchanged
+			tc.expected.service = tc.service
+			subnets := []string{tc.defaultSubnetOne}
+			slManagerFactory := func(mode string) securityListManager {
+				return newSecurityListManagerNOOP()
+			}
+			result, err := NewLBSpec(tc.service, tc.nodes, subnets, nil, slManagerFactory)
+			if err != nil {
+				t.Error(err)
+			}
+
+			if !reflect.DeepEqual(result, tc.expected) {
+				t.Errorf("Expected load balancer spec\n%+v\nbut got\n%+v", tc.expected, result)
+			}
+		})
+	}
+}
+
 func TestNewLBSpecFailure(t *testing.T) {
 	testCases := map[string]struct {
 		defaultSubnetOne string
@@ -566,21 +650,6 @@ func TestNewLBSpecFailure(t *testing.T) {
 			},
 			expectedErrMsg: "error parsing service annotation: service.beta.kubernetes.io/oci-load-balancer-connection-idle-timeout=whoops",
 		},
-		"missing subnet defaults and annotations": {
-			service: &v1.Service{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace:   "kube-system",
-					Name:        "testservice",
-					UID:         "test-uid",
-					Annotations: map[string]string{},
-				},
-				Spec: v1.ServiceSpec{
-					SessionAffinity: v1.ServiceAffinityNone,
-					Ports:           []v1.ServicePort{},
-				},
-			},
-			expectedErrMsg: "a configuration for both subnets must be specified",
-		},
 		"internal lb missing subnet1": {
 			defaultSubnetTwo: "two",
 			service: &v1.Service{