Allow configuration of the subnet the LB is placed in (#287)

* Allow configuration of the subnet the LB is placed in

This is an optional parameter that can either be left uninitialized to
keep the old behavior or be set as a helm value or per LB.

README.md

@@ -178,6 +178,12 @@ yawolFloatingID: <floating-id>
 # Placed in LoadBalancer.spec.infrastructure.networkID
 yawolNetworkID: <network-id>
 
+# OpenStack subnet ID in which the Load Balancer is placed.
+# If not set, the subnet is chosen automatically.
+#
+# Placed in LoadBalancer.spec.infrastructure.subnetID
+yawolSubnetID: <subnet-id>
+
 # default value for flavor that yawol Load Balancer instances should use
 # can be overridden by annotation
 #

api/v1beta1/loadbalancer_types.go

@@ -15,6 +15,8 @@ const (
 	// If this is set to a different network ID than defined as default in the yawol-cloud-controller
 	// the default from the yawol-cloud-controller will be added to the additionalNetworks
 	ServiceDefaultNetworkID = "yawol.stackit.cloud/defaultNetworkID"
+	// ServiceDefaultSubnetID overwrites the default openstack subnet for the loadbalancer
+	ServiceDefaultSubnetID = "yawol.stackit.cloud/defaultSubnetID"
 	// ServiceSkipCloudControllerDefaultNetworkID if set to true it do not add the default network ID from
 	// the yawol-cloud-controller to the additionalNetworks
 	ServiceSkipCloudControllerDefaultNetworkID = "yawol.stackit.cloud/skipCloudControllerDefaultNetworkID"
@@ -245,6 +247,9 @@ type LoadBalancerDefaultNetwork struct {
 	FloatingNetID *string `json:"floatingNetID,omitempty"`
 	// NetworkID defines an openstack ID for the network.
 	NetworkID string `json:"networkID"`
+	// SubnetID defines an openstack ID for the subnet.
+	// +optional
+	SubnetID *string `json:"subnetID,omitempty"`
 }
 
 // OpenstackImageRef defines a reference to a Openstack image.

charts/yawol-controller/README.md

@@ -51,5 +51,6 @@ Helm chart for yawol-controller
 | yawolFloatingID | string | `nil` |  |
 | yawolImageID | string | `nil` |  |
 | yawolNetworkID | string | `nil` |  |
+| yawolSubnetID | string | `nil` |  |
 | yawolOSSecretName | string | `nil` |  |
 

charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml

@@ -100,6 +100,10 @@ spec:
                       networkID:
                         description: NetworkID defines an openstack ID for the network.
                         type: string
+                      subnetID:
+                        description: SubnetID defines an openstack ID for the
+                          subnet.
+                        type: string
                     required:
                     - networkID
                     type: object

charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml

@@ -131,6 +131,10 @@ spec:
                       networkID:
                         description: NetworkID defines an openstack ID for the network.
                         type: string
+                      subnetID:
+                        description: SubnetID defines an openstack ID for the
+                          subnet.
+                        type: string
                     required:
                     - networkID
                     type: object

charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml

@@ -165,6 +165,10 @@ spec:
                                 description: NetworkID defines an openstack ID for
                                   the network.
                                 type: string
+                              subnetID:
+                                description: SubnetID defines an openstack ID
+                                  for the subnet.
+                                type: string
                             required:
                             - networkID
                             type: object

charts/yawol-controller/templates/yawol-cloud-controller.yaml

@@ -61,6 +61,10 @@ spec:
         - name: NETWORK_ID
           value: {{ .Values.yawolNetworkID }}
         {{- end }}
+        {{- if .Values.yawolSubnetID }}
+        - name: SUBNET_ID
+          value: {{ .Values.yawolSubnetID }}
+        {{- end }}
         {{- if .Values.yawolFlavorID }}
         - name: FLAVOR_ID
           value: {{ .Values.yawolFlavorID }}

charts/yawol-controller/values.yaml

@@ -90,6 +90,12 @@ yawolFloatingID:
 # Placed in LoadBalancer.spec.infrastructure.networkID
 yawolNetworkID:
 
+# OpenStack subnet ID in which the Load Balancer is placed.
+# If not set, the subnet is chosen automatically.
+#
+# Placed in LoadBalancer.spec.infrastructure.subnetID
+yawolSubnetID:
+
 # default value for flavor that yawol Load Balancer instances should use
 # can be overridden by annotation
 #

cmd/yawol-cloud-controller/main.go

@@ -47,6 +47,8 @@ const (
 	EnvFloatingNetID = "FLOATING_NET_ID"
 	// Openstack NetworkID for LB
 	EnvNetworkID = "NETWORK_ID"
+	// OpenStack SubnetID for LB
+	EnvSubnetID = "SUBNET_ID"
 	// Flavor Information
 	// one must be set
 	EnvFlavorID     = "FLAVOR_ID"
@@ -301,6 +303,11 @@ func getInfrastructureDefaultsFromEnvOrDie() targetcontroller.InfrastructureDefa
 		panic("could not read env " + EnvNetworkID)
 	}
 
+	var subnetID *string
+	if subnetID = ptr.To(os.Getenv(EnvSubnetID)); *subnetID == "" {
+		subnetID = nil
+	}
+
 	var clusterNamespace string
 	if clusterNamespace = os.Getenv(EnvClusterNamespace); clusterNamespace == "" {
 		panic("could not read env " + EnvClusterNamespace)
@@ -358,6 +365,7 @@ func getInfrastructureDefaultsFromEnvOrDie() targetcontroller.InfrastructureDefa
 		AuthSecretName:    ptr.To(authSecretName),
 		FloatingNetworkID: ptr.To(floatingNetworkID),
 		NetworkID:         ptr.To(networkID),
+		SubnetID:          subnetID,
 		Namespace:         ptr.To(clusterNamespace),
 		FlavorRef: &yawolv1beta1.OpenstackFlavorRef{
 			FlavorID:     flavorID,

controllers/yawol-cloud-controller/targetcontroller/infrastructure_defaults.go

@@ -12,6 +12,7 @@ type InfrastructureDefaults struct {
 	AuthSecretName    *string
 	FloatingNetworkID *string
 	NetworkID         *string
+	SubnetID          *string
 	Namespace         *string
 	FlavorRef         *yawolv1beta1.OpenstackFlavorRef
 	ImageRef          *yawolv1beta1.OpenstackImageRef

controllers/yawol-cloud-controller/targetcontroller/service_controller.go

@@ -634,6 +634,7 @@ func getDefaultNetwork(
 	defaultNetwork := yawolv1beta1.LoadBalancerDefaultNetwork{
 		FloatingNetID: infraConfig.FloatingNetworkID,
 		NetworkID:     *infraConfig.NetworkID,
+		SubnetID:      infraConfig.SubnetID,
 	}
 
 	if networkID, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok {
@@ -643,6 +644,10 @@ func getDefaultNetwork(
 	if floatingID, ok := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID]; ok {
 		defaultNetwork.FloatingNetID = &floatingID
 	}
+
+	if subnetID, ok := svc.Annotations[yawolv1beta1.ServiceDefaultSubnetID]; ok {
+		defaultNetwork.SubnetID = &subnetID
+	}
 	return defaultNetwork
 }
 

controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go

@@ -2482,5 +2482,62 @@ var _ = Describe("Check loadbalancer reconcile", Serial, Ordered, func() {
 				return fmt.Errorf("projectID is not correctly set in infrastructure %v", lb.Spec.Infrastructure.ProjectID)
 			}, time.Second*5, time.Millisecond*500).Should(Succeed())
 		})
+
+		It("should update subnet from annotation", func() {
+			By("creating a service without overwritten subnet id")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "service-test34",
+					Namespace: "default",
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolTCP,
+							Port:       12345,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+							NodePort:   31034,
+						},
+					},
+					Type: "LoadBalancer",
+				},
+			}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("checking that the defaultNetwork SubnetID is set")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test34", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if (testInfraDefaults.SubnetID == nil && lb.Spec.Infrastructure.DefaultNetwork.SubnetID == nil) ||
+					(lb.Spec.Infrastructure.DefaultNetwork.SubnetID != nil && testInfraDefaults.SubnetID != nil &&
+						*lb.Spec.Infrastructure.DefaultNetwork.SubnetID == *testInfraDefaults.SubnetID) {
+					return nil
+				}
+				return fmt.Errorf("defaultNetwork subbnetID is not correct %v", lb.Spec.Infrastructure.DefaultNetwork.SubnetID)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("update svc to overwrite subbnetwork ID")
+			Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed())
+			service.ObjectMeta.Annotations = map[string]string{
+				yawolv1beta1.ServiceDefaultSubnetID: "newSubnetID",
+			}
+			Expect(k8sClient.Update(ctx, &service)).Should(Succeed())
+
+			By("check if lb gets new subnet ID")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test34", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if lb.Spec.Infrastructure.DefaultNetwork.SubnetID != nil &&
+					*lb.Spec.Infrastructure.DefaultNetwork.SubnetID == "newSubnetID" {
+					return nil
+				}
+				return fmt.Errorf("defaultNetwork SubnetID is not correct %v", lb.Spec.Infrastructure.DefaultNetwork.NetworkID)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+		})
 	})
 })

controllers/yawol-controller/loadbalancer/loadbalancer_controller.go

@@ -575,11 +575,17 @@ func (r *Reconciler) reconcilePort( //nolint: gocyclo // TODO reduce complexity
 			networkID = lb.Spec.Infrastructure.DefaultNetwork.NetworkID
 		}
 
+		var subnetID string
+		if lb.Spec.Infrastructure.DefaultNetwork.SubnetID != nil {
+			subnetID = *lb.Spec.Infrastructure.DefaultNetwork.SubnetID
+		}
+
 		port, err = openstackhelper.CreatePort(
 			ctx,
 			portClient,
 			*lb.Status.PortName,
 			networkID,
+			subnetID,
 		)
 		if err != nil {
 			r.Log.Info("unexpected error occurred claiming a port", "lb", req.NamespacedName)

controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go

@@ -1016,6 +1016,7 @@ func getMockLB(lbNN types.NamespacedName) *LB {
 				DefaultNetwork: yawolv1beta1.LoadBalancerDefaultNetwork{
 					FloatingNetID: ptr.To("floatingnet-id"),
 					NetworkID:     "network-id",
+					SubnetID:      ptr.To("subnet-id"),
 				},
 				Flavor: yawolv1beta1.OpenstackFlavorRef{
 					FlavorID: ptr.To("flavor-id"),

controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go

@@ -471,6 +471,11 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( //nolint: gocyclo // TODO
 		return helper.ErrNoNetworkID
 	}
 
+	var subnetID string
+	if lbm.Spec.Infrastructure.DefaultNetwork.SubnetID != nil {
+		subnetID = *lbm.Spec.Infrastructure.DefaultNetwork.SubnetID
+	}
+
 	var portClient os.PortClient
 	portClient, err = osClient.PortClient(ctx)
 	if err != nil {
@@ -509,7 +514,8 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( //nolint: gocyclo // TODO
 				ctx,
 				portClient,
 				*lbm.Status.DefaultPortName,
-				networkID)
+				networkID,
+				subnetID)
 			if err != nil {
 				r.Log.Info("unexpected error occurred claiming a port", "lbm", lbm.Name)
 				return kubernetes.SendErrorAsEvent(r.RecorderLB, err, lbm)

controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go

@@ -240,6 +240,60 @@ var _ = Describe("load balancer machine", Serial, Ordered, func() {
 		})
 	}) // openstack not working
 
+	Context("customizable subnetID features", func() {
+		BeforeEach(func() {
+			lbm.Spec.Infrastructure.DefaultNetwork.SubnetID = ptr.To("lb-subnetID")
+		})
+
+		It("should create openstack resources", func() {
+			lbmNN := runtimeClient.ObjectKeyFromObject(lbm)
+
+			Eventually(func(g Gomega) {
+				var actual yawolv1beta1.LoadBalancerMachine
+				g.Expect(k8sClient.Get(ctx, lbmNN, &actual)).To(Succeed())
+
+				g.Expect(actual.Status.ServerID).ToNot(BeNil())
+				g.Expect(actual.Status.DefaultPortID).ToNot(BeNil())
+
+				_, err := client.ServerClientObj.Get(ctx, *actual.Status.ServerID)
+				g.Expect(err).To(Succeed())
+
+				port, err := client.PortClientObj.Get(ctx, *actual.Status.DefaultPortID)
+				g.Expect(err).To(Succeed())
+				g.Expect(port.FixedIPs).To(HaveLen(1))
+
+				g.Expect(port.FixedIPs[0].SubnetID).To(Equal("lb-subnetID"))
+			}, timeout, interval).Should(Succeed())
+		})
+	}) // customizable subnetID features
+
+	Context("fallback to default subnetID", func() {
+		BeforeEach(func() {
+			lbm.Spec.Infrastructure.DefaultNetwork.SubnetID = nil // unset
+		})
+
+		It("should create openstack resources", func() {
+			lbmNN := runtimeClient.ObjectKeyFromObject(lbm)
+
+			Eventually(func(g Gomega) {
+				var actual yawolv1beta1.LoadBalancerMachine
+				g.Expect(k8sClient.Get(ctx, lbmNN, &actual)).To(Succeed())
+
+				g.Expect(actual.Status.ServerID).ToNot(BeNil())
+				g.Expect(actual.Status.DefaultPortID).ToNot(BeNil())
+
+				_, err := client.ServerClientObj.Get(ctx, *actual.Status.ServerID)
+				g.Expect(err).To(Succeed())
+
+				port, err := client.PortClientObj.Get(ctx, *actual.Status.DefaultPortID)
+				g.Expect(err).To(Succeed())
+				g.Expect(len(port.FixedIPs)).To(Equal(1))
+
+				g.Expect(port.FixedIPs[0].SubnetID).To(Equal("default-subnet-id"))
+			}, timeout, interval).Should(Succeed())
+		})
+	}) // fallback to default subnetID
+
 	Context("additionalNetworks features", func() {
 		BeforeEach(func() {
 			lbm.Spec.Infrastructure.AdditionalNetworks = []yawolv1beta1.LoadBalancerAdditionalNetwork{

internal/helper/openstack/port.go

@@ -38,11 +38,17 @@ func CreatePort(
 	portClient openstack.PortClient,
 	portName string,
 	networkID string,
+	subnetID string,
 ) (*ports.Port, error) {
 	opts := ports.CreateOpts{
 		Name:      portName,
 		NetworkID: networkID,
 	}
+	if subnetID != "" {
+		opts.FixedIPs = []ports.IP{
+			{SubnetID: subnetID},
+		}
+	}
 	port, err := portClient.Create(ctx, opts)
 	if err != nil {
 		return nil, err

internal/openstack/testing/fake.go

@@ -237,12 +237,20 @@ func GetFakeClient() *MockClient {
 		},
 		CreateFunc: func(ctx context.Context, optsBuilder ports.CreateOptsBuilder) (*ports.Port, error) {
 			opts := optsBuilder.(ports.CreateOpts)
+			var fixedIPs []ports.IP
+			if opts.FixedIPs != nil {
+				fixedIPs = opts.FixedIPs.([]ports.IP)
+			}
 
+			subnetID := "default-subnet-id"
+			if len(fixedIPs) > 0 {
+				subnetID = fixedIPs[0].SubnetID
+			}
 			port := &ports.Port{
 				ID:        getID(&client),
 				Name:      opts.Name,
 				NetworkID: opts.NetworkID,
-				FixedIPs:  []ports.IP{{IPAddress: generateIP()}},
+				FixedIPs:  []ports.IP{{SubnetID: subnetID, IPAddress: generateIP()}},
 			}
 
 			if opts.SecurityGroups != nil {