From cf9e73629f6331692487740b50ad787cbabfc84c Mon Sep 17 00:00:00 2001 From: Malte Poll <1780588+malt3@users.noreply.github.com> Date: Tue, 14 May 2024 13:39:28 +0200 Subject: [PATCH] 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 | 6 ++ api/v1beta1/loadbalancer_types.go | 5 ++ api/v1beta1/zz_generated.deepcopy.go | 5 ++ charts/yawol-controller/README.md | 1 + ...ol.stackit.cloud_loadbalancermachines.yaml | 4 ++ .../yawol.stackit.cloud_loadbalancers.yaml | 4 ++ .../yawol.stackit.cloud_loadbalancersets.yaml | 4 ++ .../templates/yawol-cloud-controller.yaml | 4 ++ charts/yawol-controller/values.yaml | 6 ++ cmd/yawol-cloud-controller/main.go | 8 +++ .../infrastructure_defaults.go | 1 + .../targetcontroller/service_controller.go | 5 ++ .../service_controller_test.go | 57 +++++++++++++++++++ .../loadbalancer/loadbalancer_controller.go | 6 ++ .../loadbalancer_controller_test.go | 1 + .../loadbalancermachine_controller.go | 8 ++- .../loadbalancermachine_controller_test.go | 54 ++++++++++++++++++ internal/helper/openstack/port.go | 6 ++ internal/openstack/testing/fake.go | 10 +++- 19 files changed, 193 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 76630d7b..55e3da95 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,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 # diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index b69975d7..0556340f 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/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. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 838df928..e1d63741 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -75,6 +75,11 @@ func (in *LoadBalancerDefaultNetwork) DeepCopyInto(out *LoadBalancerDefaultNetwo *out = new(string) **out = **in } + if in.SubnetID != nil { + in, out := &in.SubnetID, &out.SubnetID + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerDefaultNetwork. diff --git a/charts/yawol-controller/README.md b/charts/yawol-controller/README.md index 43a44e84..77e4e4cd 100644 --- a/charts/yawol-controller/README.md +++ b/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` | | diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml index f221a033..144dda98 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml +++ b/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 diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml index 71109279..4f858936 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml +++ b/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 diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml index 73121874..67879fbc 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml +++ b/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 diff --git a/charts/yawol-controller/templates/yawol-cloud-controller.yaml b/charts/yawol-controller/templates/yawol-cloud-controller.yaml index 5c4189c8..1da48573 100644 --- a/charts/yawol-controller/templates/yawol-cloud-controller.yaml +++ b/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 }} diff --git a/charts/yawol-controller/values.yaml b/charts/yawol-controller/values.yaml index 2e2bd886..60ade61a 100644 --- a/charts/yawol-controller/values.yaml +++ b/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 # diff --git a/cmd/yawol-cloud-controller/main.go b/cmd/yawol-cloud-controller/main.go index 4c019d32..983e5e33 100644 --- a/cmd/yawol-cloud-controller/main.go +++ b/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, diff --git a/controllers/yawol-cloud-controller/targetcontroller/infrastructure_defaults.go b/controllers/yawol-cloud-controller/targetcontroller/infrastructure_defaults.go index 28b63e3b..9f85d881 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/infrastructure_defaults.go +++ b/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 diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index 801e33e3..e1a143aa 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/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 } diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go index ea09c68b..1152cd7a 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go +++ b/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()) + }) }) }) diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go index 4c74ae63..549c1c81 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go +++ b/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) diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go index 8b9cca23..f4b807e8 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go +++ b/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"), diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index dbdd972a..6fd35989 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/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) diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go index 1fc9bbc9..4ac4584d 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go +++ b/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{ diff --git a/internal/helper/openstack/port.go b/internal/helper/openstack/port.go index 3081ae93..33f89618 100644 --- a/internal/helper/openstack/port.go +++ b/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 diff --git a/internal/openstack/testing/fake.go b/internal/openstack/testing/fake.go index 74bfab26..c89c7a11 100644 --- a/internal/openstack/testing/fake.go +++ b/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 {