From 85d07aff5f172f6c24bb768a47540181844f09e7 Mon Sep 17 00:00:00 2001 From: Maximilian Geberl Date: Mon, 5 Dec 2022 17:25:40 +0100 Subject: [PATCH 01/11] wip: add additional networks to lbm controller and yawol-cloud-controller --- README.md | 45 ++-- api/v1beta1/loadbalancer_types.go | 41 +++- api/v1beta1/loadbalancermachine_types.go | 7 + api/v1beta1/zz_generated.deepcopy.go | 51 ++++ ...ol.stackit.cloud_loadbalancermachines.yaml | 47 +++- .../yawol.stackit.cloud_loadbalancers.yaml | 36 ++- .../yawol.stackit.cloud_loadbalancersets.yaml | 40 +++- .../targetcontroller/service_controller.go | 92 +++++++- .../service_controller_test.go | 222 +++++++++++++++++- .../loadbalancer/loadbalancer_controller.go | 14 +- .../loadbalancer_controller_test.go | 6 +- .../loadbalancerset_status_controller_test.go | 6 +- .../loadbalancermachine_controller.go | 168 +++++++++---- .../loadbalancermachine_controller_test.go | 47 +++- .../loadbalancerset_controller_test.go | 6 +- go.mod | 2 +- go.sum | 6 +- internal/helper/errors.go | 1 + internal/helper/loadbalancermachine.go | 4 +- internal/helper/openstack/floatingip.go | 11 +- internal/openstack/testing/fake.go | 20 +- 21 files changed, 754 insertions(+), 118 deletions(-) diff --git a/README.md b/README.md index 22fdd721..80d0342e 100644 --- a/README.md +++ b/README.md @@ -186,41 +186,50 @@ kind: Service metadata: name: loadbalancer annotations: - # override the default OpenStack image ID + # Override the default OpenStack image ID. yawol.stackit.cloud/imageId: "OS-imageId" - # override the default OpenStack machine flavor + # Override the default OpenStack machine flavor. yawol.stackit.cloud/flavorId: "OS-flavorId" - # override the default OpenStack availability zone + # Overwrites the default openstack network for the loadbalancer. + # 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. + yawol.stackit.cloud/defaultNetworkID: "OS-networkID" + # Overwrites the openstack floating network for the loadbalancer. + yawol.stackit.cloud/floatingNetworkID: "OS-floatingNetID" + # Override the default OpenStack availability zone. yawol.stackit.cloud/availabilityZone: "OS-AZ" - # specify if this should be an internal LoadBalancer + # Specify if this should be an internal LoadBalancer . yawol.stackit.cloud/internalLB: "false" - # run yawollet in debug mode + # Run yawollet in debug mode. yawol.stackit.cloud/debug: "false" - # reference the name of the SSH key provided to OpenStack for debugging + # Reference the name of the SSH key provided to OpenStack for debugging . yawol.stackit.cloud/debugsshkey: "OS-keyName" - # allows filtering services in cloud-controller + # Allows filtering services in cloud-controller. yawol.stackit.cloud/className: "test" - # specify the number of LoadBalancer machines to deploy (default 1) + # Specify the number of LoadBalancer machines to deploy (default 1). yawol.stackit.cloud/replicas: "3" - # specify an existing floating IP for yawol to use + # Specify an existing floating IP for yawol to use. yawol.stackit.cloud/existingFloatingIP: "193.148.175.46" - # enable/disable envoy support for proxy protocol + # Enable/disable envoy support for proxy protocol. yawol.stackit.cloud/tcpProxyProtocol: "false" - # defines proxy protocol ports (comma separated list) + # Defines proxy protocol ports (comma separated list). yawol.stackit.cloud/tcpProxyProtocolPortsFilter: "80,443" - # enables log forwarding + # Enables log forwarding. yawol.stackit.cloud/logForward: "true" - # defines loki URL for the log forwarding + # Defines loki URL for the log forwarding. yawol.stackit.cloud/logForwardLokiURL: "http://example.com:3100/loki/api/v1/push" - # defines the TCP idle Timeout as duration, default is 1h - # make sure there is a valid unit (like "s", "m", "h"), otherwise this option is ignored + # Defines the TCP idle Timeout as duration, default is 1h. + # Make sure there is a valid unit (like "s", "m", "h"), otherwise this option is ignored. yawol.stackit.cloud/tcpIdleTimeout: "5m30s" - # defines the UDP idle Timeout as duration, default is 1m - # make sure there is a valid unit (like "s", "m", "h"), otherwise this option is ignored + # Defines the UDP idle Timeout as duration, default is 1m. + # Make sure there is a valid unit (like "s", "m", "h"), otherwise this option is ignored. yawol.stackit.cloud/udpIdleTimeout: "5m" - # can be 'affinity', 'anti-affinity' 'soft-affinity', 'soft-anti-affinity' depending on the OpenStack Infrastructure. + # Defines the openstack server group policy for a LoadBalancer. + # Can be 'affinity', 'anti-affinity' 'soft-affinity', 'soft-anti-affinity' depending on the OpenStack Infrastructure. # If not set openstack server group is disabled. yawol.stackit.cloud/serverGroupPolicy: anti-affinity + # Defines additional openstack networks for the loadbalancer (comma separated list). + yawol.stackit.cloud/additionalNetworks: "OS-networkID1,OS-networkID2" ``` See [our example service](example-setup/yawol-cloud-controller/service.yaml) diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index c2a0d45c..ede2868a 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/api/v1beta1/loadbalancer_types.go @@ -11,7 +11,13 @@ const ( ServiceImageID = "yawol.stackit.cloud/imageId" // ServiceFlavorID overwrite default flavorID ServiceFlavorID = "yawol.stackit.cloud/flavorId" - // AvailabilityZoneID set availability zone for specific service + // ServiceDefaultNetworkID overwrites the default openstack network for the loadbalancer + // 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" + // ServiceFloatingNetworkID overwrites the openstack floating network for the loadbalancer + ServiceFloatingNetworkID = "yawol.stackit.cloud/floatingNetworkID" + // ServiceAvailabilityZone set availability zone for specific service ServiceAvailabilityZone = "yawol.stackit.cloud/availabilityZone" // ServiceInternalLoadbalancer sets the internal flag in LB objects ServiceInternalLoadbalancer = "yawol.stackit.cloud/internalLB" @@ -39,6 +45,8 @@ const ( ServiceLogForwardLokiURL = "yawol.stackit.cloud/logForwardLokiURL" // ServiceServerGroupPolicy set openstack server group policy for a LoadBalancer ServiceServerGroupPolicy = "yawol.stackit.cloud/serverGroupPolicy" + // ServiceAdditionalNetworks adds additional openstack networks for the loadbalancer (comma separated list) + ServiceAdditionalNetworks = "yawol.stackit.cloud/additionalNetworks" ) // +kubebuilder:object:root=true @@ -160,17 +168,27 @@ type LoadBalancerEndpoint struct { // LoadBalancerInfrastructure defines infrastructure defaults for the LoadBalancer type LoadBalancerInfrastructure struct { + // Deprecated: use defaultNetwork instead // FloatingNetID defines a openstack ID for the floatingNet. // +optional FloatingNetID *string `json:"floatingNetID,omitempty"` + // Deprecated: use defaultNetwork instead // NetworkID defines a openstack ID for the network. - NetworkID string `json:"networkID"` + // +optional + NetworkID string `json:"networkID,omitempty"` + // DefaultNetwork defines the default/listener network for the Loadbalancer. + // +optional + // TODO Remove optional when Deprecations are removed + DefaultNetwork LoadBalancerDefaultNetwork `json:"defaultNetwork"` + // AdditionalNetworks defines additional networks that will be added to the LoadBalancerMachines. + // +optional + AdditionalNetworks []LoadBalancerAdditionalNetwork `json:"additionalNetworks"` // Flavor defines openstack flavor for the LoadBalancer. Uses a default if not defined. // +optional - Flavor *OpenstackFlavorRef `json:"flavor,omitempty"` + Flavor *OpenstackFlavorRef `json:"flavor"` // Image defines openstack image for the LoadBalancer. Uses a default if not defined. // +optional - Image *OpenstackImageRef `json:"image,omitempty"` + Image *OpenstackImageRef `json:"image"` // AvailabilityZone defines the openstack availability zone for the LoadBalancer. // +optional AvailabilityZone string `json:"availabilityZone"` @@ -178,6 +196,21 @@ type LoadBalancerInfrastructure struct { AuthSecretRef corev1.SecretReference `json:"authSecretRef"` } +// LoadBalancerAdditionalNetwork defines additional networks for the LoadBalancer +type LoadBalancerAdditionalNetwork struct { + // NetworkID defines an openstack ID for the network. + NetworkID string `json:"networkID"` +} + +// LoadBalancerDefaultNetwork defines the default/listener network for the Loadbalancer +type LoadBalancerDefaultNetwork struct { + // FloatingNetID defines an openstack ID for the floatingNet. + // +optional + FloatingNetID *string `json:"floatingNetID,omitempty"` + // NetworkID defines an openstack ID for the network. + NetworkID string `json:"networkID"` +} + // OpenstackImageRef defines a reference to a Openstack image. type OpenstackImageRef struct { // ImageID is the image ID used for requesting virtual machines. diff --git a/api/v1beta1/loadbalancermachine_types.go b/api/v1beta1/loadbalancermachine_types.go index 01b25baa..710b8237 100644 --- a/api/v1beta1/loadbalancermachine_types.go +++ b/api/v1beta1/loadbalancermachine_types.go @@ -68,9 +68,16 @@ type LoadBalancerMachineStatus struct { // ServerID contains the openstack server ID for a LoadBalancerMachine. // +optional ServerID *string `json:"serverID,omitempty"` + // Deprecated: use defaultPortID instead // PortID contains the openstack port ID for a LoadBalancerMachine. // +optional PortID *string `json:"portID,omitempty"` + // DefaultPortID contains the default openstack port ID for a LoadBalancerMachine. + // +optional + DefaultPortID *string `json:"defaultPortID,omitempty"` + // DefaultPortID contains the default openstack port ID for a LoadBalancerMachine. + // +optional + DefaultPortName *string `json:"defaultPortName,omitempty"` // ServiceAccountName contains the namespacedName from the ServiceAccount for a LoadBalancerMachine. // +optional ServiceAccountName *string `json:"serviceAccountName,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 8e88f4de..c7b6a724 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -38,6 +38,21 @@ func (in *LoadBalancer) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LoadBalancerAdditionalNetwork) DeepCopyInto(out *LoadBalancerAdditionalNetwork) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerAdditionalNetwork. +func (in *LoadBalancerAdditionalNetwork) DeepCopy() *LoadBalancerAdditionalNetwork { + if in == nil { + return nil + } + out := new(LoadBalancerAdditionalNetwork) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerDebugSettings) DeepCopyInto(out *LoadBalancerDebugSettings) { *out = *in @@ -53,6 +68,26 @@ func (in *LoadBalancerDebugSettings) DeepCopy() *LoadBalancerDebugSettings { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LoadBalancerDefaultNetwork) DeepCopyInto(out *LoadBalancerDefaultNetwork) { + *out = *in + if in.FloatingNetID != nil { + in, out := &in.FloatingNetID, &out.FloatingNetID + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerDefaultNetwork. +func (in *LoadBalancerDefaultNetwork) DeepCopy() *LoadBalancerDefaultNetwork { + if in == nil { + return nil + } + out := new(LoadBalancerDefaultNetwork) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancerEndpoint) DeepCopyInto(out *LoadBalancerEndpoint) { *out = *in @@ -81,6 +116,12 @@ func (in *LoadBalancerInfrastructure) DeepCopyInto(out *LoadBalancerInfrastructu *out = new(string) **out = **in } + in.DefaultNetwork.DeepCopyInto(&out.DefaultNetwork) + if in.AdditionalNetworks != nil { + in, out := &in.AdditionalNetworks, &out.AdditionalNetworks + *out = make([]LoadBalancerAdditionalNetwork, len(*in)) + copy(*out, *in) + } if in.Flavor != nil { in, out := &in.Flavor, &out.Flavor *out = new(OpenstackFlavorRef) @@ -286,6 +327,16 @@ func (in *LoadBalancerMachineStatus) DeepCopyInto(out *LoadBalancerMachineStatus *out = new(string) **out = **in } + if in.DefaultPortID != nil { + in, out := &in.DefaultPortID, &out.DefaultPortID + *out = new(string) + **out = **in + } + if in.DefaultPortName != nil { + in, out := &in.DefaultPortName, &out.DefaultPortName + *out = new(string) + **out = **in + } if in.ServiceAccountName != nil { in, out := &in.ServiceAccountName, &out.ServiceAccountName *out = new(string) diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml index f4c3add6..a449021a 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml @@ -57,6 +57,20 @@ spec: infrastructure: description: Infrastructure defines parameters for the Infrastructure. properties: + additionalNetworks: + description: AdditionalNetworks defines additional networks that + will be added to the LoadBalancerMachines. + items: + description: LoadBalancerAdditionalNetwork defines additional + networks for the LoadBalancer + properties: + networkID: + description: NetworkID defines an openstack ID for the network. + type: string + required: + - networkID + type: object + type: array authSecretRef: description: AuthSecretRef defines a secretRef for the openstack secret. @@ -75,6 +89,21 @@ spec: description: AvailabilityZone defines the openstack availability zone for the LoadBalancer. type: string + defaultNetwork: + description: DefaultNetwork defines the default/listener network + for the Loadbalancer. TODO Remove optional when Deprecations + are removed + properties: + floatingNetID: + description: FloatingNetID defines an openstack ID for the + floatingNet. + type: string + networkID: + description: NetworkID defines an openstack ID for the network. + type: string + required: + - networkID + type: object flavor: description: Flavor defines openstack flavor for the LoadBalancer. Uses a default if not defined. @@ -112,7 +141,8 @@ spec: type: string type: object floatingNetID: - description: FloatingNetID defines a openstack ID for the floatingNet. + description: 'Deprecated: use defaultNetwork instead FloatingNetID + defines a openstack ID for the floatingNet.' type: string image: description: Image defines openstack image for the LoadBalancer. @@ -151,11 +181,11 @@ spec: type: string type: object networkID: - description: NetworkID defines a openstack ID for the network. + description: 'Deprecated: use defaultNetwork instead NetworkID + defines a openstack ID for the network.' type: string required: - authSecretRef - - networkID type: object loadBalancerRef: description: LoadBalancerRef defines a reference to the LoadBalancer @@ -227,6 +257,14 @@ spec: description: CreationTimestamp contains the creation timestamp a LoadBalancerMachine. format: date-time type: string + defaultPortID: + description: DefaultPortID contains the default openstack port ID + for a LoadBalancerMachine. + type: string + defaultPortName: + description: DefaultPortID contains the default openstack port ID + for a LoadBalancerMachine. + type: string lastOpenstackReconcile: description: LastOpenstackReconcile contains the timestamp of the last openstack reconciliation. @@ -255,7 +293,8 @@ spec: type: object type: array portID: - description: PortID contains the openstack port ID for a LoadBalancerMachine. + description: 'Deprecated: use defaultPortID instead PortID contains + the openstack port ID for a LoadBalancerMachine.' type: string roleBindingName: description: RoleBindingName contains the namespacedName from the diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml index bf2d0e01..8df78225 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml @@ -88,6 +88,20 @@ spec: infrastructure: description: Infrastructure defines parameters for the Infrastructure properties: + additionalNetworks: + description: AdditionalNetworks defines additional networks that + will be added to the LoadBalancerMachines. + items: + description: LoadBalancerAdditionalNetwork defines additional + networks for the LoadBalancer + properties: + networkID: + description: NetworkID defines an openstack ID for the network. + type: string + required: + - networkID + type: object + type: array authSecretRef: description: AuthSecretRef defines a secretRef for the openstack secret. @@ -106,6 +120,21 @@ spec: description: AvailabilityZone defines the openstack availability zone for the LoadBalancer. type: string + defaultNetwork: + description: DefaultNetwork defines the default/listener network + for the Loadbalancer. TODO Remove optional when Deprecations + are removed + properties: + floatingNetID: + description: FloatingNetID defines an openstack ID for the + floatingNet. + type: string + networkID: + description: NetworkID defines an openstack ID for the network. + type: string + required: + - networkID + type: object flavor: description: Flavor defines openstack flavor for the LoadBalancer. Uses a default if not defined. @@ -143,7 +172,8 @@ spec: type: string type: object floatingNetID: - description: FloatingNetID defines a openstack ID for the floatingNet. + description: 'Deprecated: use defaultNetwork instead FloatingNetID + defines a openstack ID for the floatingNet.' type: string image: description: Image defines openstack image for the LoadBalancer. @@ -182,11 +212,11 @@ spec: type: string type: object networkID: - description: NetworkID defines a openstack ID for the network. + description: 'Deprecated: use defaultNetwork instead NetworkID + defines a openstack ID for the network.' type: string required: - authSecretRef - - networkID type: object options: description: Options for additional LoadBalancer settings diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml index 07d69fb0..43d2a307 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml @@ -117,6 +117,21 @@ spec: infrastructure: description: Infrastructure defines parameters for the Infrastructure. properties: + additionalNetworks: + description: AdditionalNetworks defines additional networks + that will be added to the LoadBalancerMachines. + items: + description: LoadBalancerAdditionalNetwork defines additional + networks for the LoadBalancer + properties: + networkID: + description: NetworkID defines an openstack ID for + the network. + type: string + required: + - networkID + type: object + type: array authSecretRef: description: AuthSecretRef defines a secretRef for the openstack secret. @@ -135,6 +150,22 @@ spec: description: AvailabilityZone defines the openstack availability zone for the LoadBalancer. type: string + defaultNetwork: + description: DefaultNetwork defines the default/listener + network for the Loadbalancer. TODO Remove optional when + Deprecations are removed + properties: + floatingNetID: + description: FloatingNetID defines an openstack ID + for the floatingNet. + type: string + networkID: + description: NetworkID defines an openstack ID for + the network. + type: string + required: + - networkID + type: object flavor: description: Flavor defines openstack flavor for the LoadBalancer. Uses a default if not defined. @@ -174,8 +205,8 @@ spec: type: string type: object floatingNetID: - description: FloatingNetID defines a openstack ID for - the floatingNet. + description: 'Deprecated: use defaultNetwork instead FloatingNetID + defines a openstack ID for the floatingNet.' type: string image: description: Image defines openstack image for the LoadBalancer. @@ -216,12 +247,11 @@ spec: type: string type: object networkID: - description: NetworkID defines a openstack ID for the - network. + description: 'Deprecated: use defaultNetwork instead NetworkID + defines a openstack ID for the network.' type: string required: - authSecretRef - - networkID type: object loadBalancerRef: description: LoadBalancerRef defines a reference to the LoadBalancer diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index f6dbf55e..5346734e 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go @@ -3,7 +3,6 @@ package targetcontroller import ( "context" "crypto/sha256" - "encoding/base32" "encoding/json" "fmt" @@ -147,7 +146,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, err } - err = r.reconcileInfrastructure(ctx, loadBalancer, infraDefaults) + err = r.reconcileInfrastructure(ctx, loadBalancer, svc, infraDefaults) if err != nil { return ctrl.Result{}, err } @@ -202,11 +201,11 @@ func (r *ServiceReconciler) createLoadBalancer( }, ExistingFloatingIP: helper.GetExistingFloatingIPFromAnnotation(svc), Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: infraConfig.FloatingNetworkID, - NetworkID: *infraConfig.NetworkID, - Flavor: infraConfig.FlavorRef, - Image: infraConfig.ImageRef, - AvailabilityZone: *infraConfig.AvailabilityZone, + DefaultNetwork: getDefaultNetwork(svc, infraConfig), + AdditionalNetworks: getAdditionalNetworks(svc, infraConfig), + Flavor: infraConfig.FlavorRef, + Image: infraConfig.ImageRef, + AvailabilityZone: *infraConfig.AvailabilityZone, AuthSecretRef: coreV1.SecretReference{ Name: *infraConfig.AuthSecretName, Namespace: *infraConfig.Namespace, @@ -223,14 +222,15 @@ func (r *ServiceReconciler) createLoadBalancer( func (r *ServiceReconciler) reconcileInfrastructure( ctx context.Context, lb *yawolv1beta1.LoadBalancer, + svc *coreV1.Service, infraConfig InfrastructureDefaults, ) error { newInfra := yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: infraConfig.FloatingNetworkID, - NetworkID: *infraConfig.NetworkID, - Flavor: infraConfig.FlavorRef, - Image: infraConfig.ImageRef, - AvailabilityZone: *infraConfig.AvailabilityZone, + DefaultNetwork: getDefaultNetwork(svc, infraConfig), + AdditionalNetworks: getAdditionalNetworks(svc, infraConfig), + Flavor: infraConfig.FlavorRef, + Image: infraConfig.ImageRef, + AvailabilityZone: *infraConfig.AvailabilityZone, AuthSecretRef: coreV1.SecretReference{ Name: *infraConfig.AuthSecretName, Namespace: *infraConfig.Namespace, @@ -255,6 +255,23 @@ func (r *ServiceReconciler) reconcileInfrastructure( return err } } + + // TODO remove after remove deprecation + if lb.Spec.Infrastructure.NetworkID != "" { //nolint: staticcheck // needed to be backwards compatible + patch := []byte(`{"spec":{"infrastructure": { "networkID": "" }}}`) + err := r.ControlClient.Patch(ctx, lb, client.RawPatch(types.MergePatchType, patch)) + if err != nil { + return err + } + } + if lb.Spec.Infrastructure.FloatingNetID != nil { //nolint: staticcheck // needed to be backwards compatible + patch := []byte(`{"spec":{"infrastructure": { "floatingNetID": null }}}`) + err := r.ControlClient.Patch(ctx, lb, client.RawPatch(types.MergePatchType, patch)) + if err != nil { + return err + } + } + return nil } @@ -583,3 +600,54 @@ func (r *ServiceReconciler) addAnnotation( patch := []byte(`{"metadata": {"annotations": {` + string(keyJSON) + `: ` + string(valueJSON) + `}}}`) return r.ControlClient.Patch(ctx, lb, client.RawPatch(types.MergePatchType, patch)) } + +// getAdditionalNetworks returns LoadBalancerAdditionalNetwork based on the ServiceAdditionalNetworks annotation +// If the default network is overwritten with the ServiceDefaultNetworkID annotation it will add the +// networkID from the InfrastructureDefaults +func getAdditionalNetworks( + svc *coreV1.Service, + infraConfig InfrastructureDefaults, +) []yawolv1beta1.LoadBalancerAdditionalNetwork { + // use struct to make sure there are no duplicates + networkIDs := make(map[string]struct{}) + + if _, ok := svc.Annotations[yawolv1beta1.ServiceAdditionalNetworks]; ok { + for _, networkID := range strings.Split(svc.Annotations[yawolv1beta1.ServiceAdditionalNetworks], ",") { + networkIDs[networkID] = struct{}{} + } + } + + if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { + if svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID] != *infraConfig.NetworkID { + networkIDs[*infraConfig.NetworkID] = struct{}{} + } + } + + var additionalNetworks []yawolv1beta1.LoadBalancerAdditionalNetwork + for networkID := range networkIDs { + additionalNetworks = append(additionalNetworks, yawolv1beta1.LoadBalancerAdditionalNetwork{NetworkID: networkID}) + } + + return additionalNetworks +} + +func getDefaultNetwork( + svc *coreV1.Service, + infraConfig InfrastructureDefaults, +) yawolv1beta1.LoadBalancerDefaultNetwork { + defaultNetwork := yawolv1beta1.LoadBalancerDefaultNetwork{ + FloatingNetID: infraConfig.FloatingNetworkID, + NetworkID: *infraConfig.NetworkID, + } + + if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { + defaultNetwork.NetworkID = svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID] + } + + if _, ok := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID]; ok { + floatingNetID := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID] + defaultNetwork.FloatingNetID = &floatingNetID + } + + 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 fa750d3c..b4e31758 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go @@ -221,8 +221,9 @@ var _ = Describe("Check loadbalancer reconcile", func() { patch := []byte(`{"spec":{` + `"infrastructure":{` + + `"defaultNetwork":{` + `"floatingNetID":"CHANGED",` + - `"networkID":"CHANGED",` + + `"networkID":"CHANGED"},` + `"availabilityZone":"CHANGED",` + `"flavor":{"flavor_id":"CHANGED"},` + `"image":{"image_id":"CHANGED"},` + @@ -249,8 +250,8 @@ var _ = Describe("Check loadbalancer reconcile", func() { if err != nil { return err } - if lb.Spec.Infrastructure.NetworkID == *testInfraDefaults.NetworkID && - *lb.Spec.Infrastructure.FloatingNetID == *testInfraDefaults.FloatingNetworkID && + if lb.Spec.Infrastructure.DefaultNetwork.NetworkID == *testInfraDefaults.NetworkID && + *lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID == *testInfraDefaults.FloatingNetworkID && *lb.Spec.Infrastructure.Flavor.FlavorID == *testInfraDefaults.FlavorRef.FlavorID && *lb.Spec.Infrastructure.Image.ImageID == *testInfraDefaults.ImageRef.ImageID { return nil @@ -1892,5 +1893,220 @@ var _ = Describe("Check loadbalancer reconcile", func() { return fmt.Errorf("wrong ServerGroupPolicy is not nil %v", lb.Spec.Options.ServerGroupPolicy) }, time.Second*5, time.Millisecond*500).Should(Succeed()) }) + + It("should update the floatingNetwork field", func() { + By("creating a service without floatingNetwork") + service := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-test28", + Namespace: "default", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "port1", + Protocol: v1.ProtocolTCP, + Port: 12345, + TargetPort: intstr.IntOrString{IntVal: 12345}, + NodePort: 31028, + }, + }, + Type: "LoadBalancer", + }} + Expect(k8sClient.Create(ctx, &service)).Should(Succeed()) + + By("checking that the floatingNetwork ID are set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test28", Namespace: "default"}, &lb) + if err != nil { + return err + } + if *lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID == *testInfraDefaults.FloatingNetworkID { + return nil + } + return fmt.Errorf("floatingNetwork ID is not correct %v", lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to overwrite floatingNetwork ID") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{ + yawolv1beta1.ServiceFloatingNetworkID: "newFloatingID", + } + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("check if lb gets new floatingNetwork ID") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test28", Namespace: "default"}, &lb) + if err != nil { + return err + } + if *lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID == "newFloatingID" { + return nil + } + return fmt.Errorf("floatingNetwork ID is not correct %v", lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to disable overwrite floatingNetwork ID") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{} + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("checking that the defaultNetwork ID are set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test28", Namespace: "default"}, &lb) + if err != nil { + return err + } + if *lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID == *testInfraDefaults.FloatingNetworkID { + return nil + } + return fmt.Errorf("floatingNetwork ID is not correct %v", lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + }) + + It("should update the defaultNetwork field", func() { + By("creating a service without overwritten defaultNetwork") + service := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-test29", + Namespace: "default", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "port1", + Protocol: v1.ProtocolTCP, + Port: 12345, + TargetPort: intstr.IntOrString{IntVal: 12345}, + NodePort: 31029, + }, + }, + Type: "LoadBalancer", + }} + Expect(k8sClient.Create(ctx, &service)).Should(Succeed()) + + By("checking that the defaultNetwork ID are set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test29", Namespace: "default"}, &lb) + if err != nil { + return err + } + if lb.Spec.Infrastructure.DefaultNetwork.NetworkID == *testInfraDefaults.NetworkID { + return nil + } + return fmt.Errorf("defaultNetwork IDs are not correct %v", lb.Spec.Infrastructure.DefaultNetwork.NetworkID) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to overwrite default network IDs") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{ + yawolv1beta1.ServiceDefaultNetworkID: "newNetworkID", + } + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("check if lb gets new defaultNetwork IDs") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test29", Namespace: "default"}, &lb) + if err != nil { + return err + } + if lb.Spec.Infrastructure.DefaultNetwork.NetworkID == "newNetworkID" && + len(lb.Spec.Infrastructure.AdditionalNetworks) == 1 && + lb.Spec.Infrastructure.AdditionalNetworks[0].NetworkID == *testInfraDefaults.NetworkID { + return nil + } + return fmt.Errorf("defaultNetwork ID is not correct %v or not infraDefault is not present in additionalNetworks %v", + lb.Spec.Infrastructure.DefaultNetwork.NetworkID, lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to disable overwrite default network ID") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{} + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("checking that the defaultNetwork ID are set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test29", Namespace: "default"}, &lb) + if err != nil { + return err + } + if lb.Spec.Infrastructure.DefaultNetwork.NetworkID == *testInfraDefaults.NetworkID && + len(lb.Spec.Infrastructure.AdditionalNetworks) == 0 { + return nil + } + return fmt.Errorf("defaultNetwork ID are not correct %v or still present in additionalNetworks%v", + lb.Spec.Infrastructure.DefaultNetwork.NetworkID, lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + }) + + It("should update the additionalNetwork field", func() { + By("creating a service without additionalNetwork") + service := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-test30", + Namespace: "default", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "port1", + Protocol: v1.ProtocolTCP, + Port: 12345, + TargetPort: intstr.IntOrString{IntVal: 12345}, + NodePort: 31030, + }, + }, + Type: "LoadBalancer", + }} + Expect(k8sClient.Create(ctx, &service)).Should(Succeed()) + + By("checking that the additionalNetwork are not set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test30", Namespace: "default"}, &lb) + if err != nil { + return err + } + if len(lb.Spec.Infrastructure.AdditionalNetworks) == 0 { + return nil + } + return fmt.Errorf("additionalNetwork are already set %v", lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to add additionalNetwork") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{ + yawolv1beta1.ServiceAdditionalNetworks: "additionalNetworkID1,additionalNetworkID1,additionalNetworkID2", + } + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("check if lb gets additionalNetworks") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test30", Namespace: "default"}, &lb) + if err != nil { + return err + } + if len(lb.Spec.Infrastructure.AdditionalNetworks) == 2 { + return nil + } + return fmt.Errorf("additionalNetworks are not correct %v", lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to disable additionalNetworks") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{} + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("checking that no additionalNetworks are set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test30", Namespace: "default"}, &lb) + if err != nil { + return err + } + if len(lb.Spec.Infrastructure.AdditionalNetworks) == 0 { + return nil + } + return fmt.Errorf("additionalNetwork are still set %v", lb.Spec.Infrastructure.AdditionalNetworks) + }, 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 59163507..13f14c3e 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go @@ -503,10 +503,19 @@ func (r *Reconciler) reconcilePort( } // Create Port - //nolint: dupl // we can't extract this code because of generics if lb.Status.PortID == nil { r.Log.Info("Create Port", "lb", lb.Name) - port, err = openstackhelper.CreatePort(ctx, portClient, *lb.Status.PortName, lb.Spec.Infrastructure.NetworkID) + + // TODO cleanup after removing deprecated fields + var networkID string + if lb.Spec.Infrastructure.NetworkID != "" { //nolint: staticcheck // needed to be backwards compatible + networkID = lb.Spec.Infrastructure.NetworkID //nolint: staticcheck // needed to be backwards compatible + } + if lb.Spec.Infrastructure.DefaultNetwork.NetworkID != "" { + networkID = lb.Spec.Infrastructure.DefaultNetwork.NetworkID + } + + port, err = openstackhelper.CreatePort(ctx, portClient, *lb.Status.PortName, networkID) if err != nil { r.Log.Info("unexpected error occurred claiming a port", "lb", req.NamespacedName) return false, kubernetes.SendErrorAsEvent(r.RecorderLB, err, lb) @@ -740,7 +749,6 @@ func (r *Reconciler) reconcileServerGroup( } // Create server group - //nolint: dupl // we can't extract this code because of generics if lb.Status.ServerGroupID == nil { r.Log.Info("Create ServerGroup", "lb", lb.Name) serverGroup, err = openstackhelper.CreateServerGroup( diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go index ae6f1fe0..fe112c70 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go @@ -680,8 +680,10 @@ func getMockLB(lbNN types.NamespacedName) *LB { Ports: nil, ExistingFloatingIP: nil, Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: pointer.String("floatingnet-id"), - NetworkID: "network-id", + DefaultNetwork: yawolv1beta1.LoadBalancerDefaultNetwork{ + FloatingNetID: pointer.String("floatingnet-id"), + NetworkID: "network-id", + }, Flavor: &yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("flavor-id"), }, diff --git a/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go index bed6f3a2..feea999b 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go @@ -67,8 +67,10 @@ var _ = Describe("LB Status update", func() { Endpoints: nil, Ports: nil, Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: pointer.String("floatingnetid"), - NetworkID: "networkid", + DefaultNetwork: yawolv1beta1.LoadBalancerDefaultNetwork{ + FloatingNetID: pointer.String("floatingnetid"), + NetworkID: "networkid", + }, Flavor: &yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("mycool-openstack-flavor-id"), }, diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index d55b3dc5..17f19de9 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go @@ -95,9 +95,13 @@ func (r *LoadBalancerMachineReconciler) Reconcile(ctx context.Context, req ctrl. if err := r.deleteServer(ctx, osClient, loadBalancerMachine); err != nil { return ctrl.Result{}, err } - if err := r.deletePort(ctx, osClient, req, loadBalancerMachine); err != nil { + requeue, err := r.deletePort(ctx, osClient, loadBalancerMachine) + if err != nil { return ctrl.Result{}, err } + if requeue { + return ctrl.Result{RequeueAfter: DefaultRequeueTime}, nil + } // delete k8s resources if err := r.deleteSA(ctx, loadBalancerMachine); err != nil { @@ -346,7 +350,34 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( ) error { var err error - if lbm.Spec.Infrastructure.NetworkID == "" { + // TODO cleanup after removing deprecated fields + if lbm.Status.PortID != nil && lbm.Status.DefaultPortID == nil { //nolint: staticcheck // needed to be backwards compatible + if err := helper.PatchLBMStatus( + ctx, + r.Client.Status(), + lbm, + yawolv1beta1.LoadBalancerMachineStatus{DefaultPortID: lbm.Status.PortID}, //nolint: staticcheck // needed to be backwards compatible + ); err != nil { + return err + } + if lbm.Status.PortID != nil && lbm.Status.DefaultPortID != nil && //nolint: staticcheck // needed to be backwards compatible + *lbm.Status.PortID == *lbm.Status.DefaultPortID { //nolint: staticcheck // needed to be backwards compatible + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "portID"); err != nil { + return err + } + } + } + + // TODO cleanup after removing deprecated fields + var networkID string + if lbm.Spec.Infrastructure.NetworkID != "" { //nolint: staticcheck // needed to be backwards compatible + networkID = lbm.Spec.Infrastructure.NetworkID //nolint: staticcheck // needed to be backwards compatible + } + if lbm.Spec.Infrastructure.DefaultNetwork.NetworkID != "" { + networkID = lbm.Spec.Infrastructure.DefaultNetwork.NetworkID + } + + if networkID == "" { return helper.ErrNoNetworkID } @@ -356,20 +387,35 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( return err } - portName := req.NamespacedName.String() + if lbm.Status.DefaultPortName == nil { + portName := req.NamespacedName.String() + if err := helper.PatchLBMStatus( + ctx, + r.Client.Status(), + lbm, + yawolv1beta1.LoadBalancerMachineStatus{DefaultPortName: &portName}, + ); err != nil { + return err + } + } + + if lbm.Status.DefaultPortName == nil { + return helper.ErrPortNameEmpty + } + var port *ports.Port // find or create port - if lbm.Status.PortID == nil { + if lbm.Status.DefaultPortID == nil { // try to find port by name - port, err = openstackhelper.GetPortByName(ctx, portClient, portName) + port, err = openstackhelper.GetPortByName(ctx, portClient, *lbm.Status.DefaultPortName) if err != nil { return err } // create port if port == nil { - port, err = openstackhelper.CreatePort(ctx, portClient, portName, lbm.Spec.Infrastructure.NetworkID) + port, err = openstackhelper.CreatePort(ctx, portClient, *lbm.Status.DefaultPortName, networkID) if err != nil { r.Log.Info("unexpected error occurred claiming a port", "lbm", lbm.Name) return kubernetes.SendErrorAsEvent(r.RecorderLB, err, lbm) @@ -398,20 +444,20 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( ctx, r.Client.Status(), lbm, - yawolv1beta1.LoadBalancerMachineStatus{PortID: &port.ID}, + yawolv1beta1.LoadBalancerMachineStatus{DefaultPortID: &port.ID}, ); err != nil { return err } } // Check if port still exists properly - if lbm.Status.PortID != nil { - if port, err = openstackhelper.GetPortByID(ctx, portClient, *lbm.Status.PortID); err != nil { + if lbm.Status.DefaultPortID != nil { + if port, err = openstackhelper.GetPortByID(ctx, portClient, *lbm.Status.DefaultPortID); err != nil { switch err.(type) { case gophercloud.ErrDefault404: - r.Log.Info("lbm port not found in openstack", "portID", *lbm.Status.PortID) + r.Log.Info("lbm port not found in openstack", "defaultPortID", *lbm.Status.DefaultPortID) // remove port from LB status so a new one gets created next reconcile - if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "portID"); err != nil { + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "defaultPortID"); err != nil { return err } return kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) @@ -443,12 +489,12 @@ func (r *LoadBalancerMachineReconciler) reconcilePortAddressPair( return "", err } - if lbm.Status.PortID == nil { + if lbm.Status.DefaultPortID == nil { r.Log.Info(helper.ErrLBMPortNotSet.Error(), "lbm", lbm.Name) return "", helper.ErrLBMPortNotSet } - portLBM, err := openstackhelper.GetPortByID(ctx, portClient, *lbm.Status.PortID) + portLBM, err := openstackhelper.GetPortByID(ctx, portClient, *lbm.Status.DefaultPortID) if err != nil { return "", kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) } @@ -543,7 +589,7 @@ func (r *LoadBalancerMachineReconciler) reconcileServer( return r.Client.Delete(ctx, loadBalancerMachine) } - if loadBalancerMachine.Status.PortID == nil { + if loadBalancerMachine.Status.DefaultPortID == nil { return helper.ErrPortIDEmpty } @@ -604,9 +650,27 @@ func (r *LoadBalancerMachineReconciler) createServer( return nil, err } - if loadBalancerMachine.Spec.Infrastructure.NetworkID == "" { + // TODO cleanup after removing deprecated fields + var networkID string + + if loadBalancerMachine.Spec.Infrastructure.NetworkID != "" { //nolint: staticcheck // needed to be backwards compatible + networkID = loadBalancerMachine.Spec.Infrastructure.NetworkID //nolint: staticcheck // needed to be backwards compatible + } + if loadBalancerMachine.Spec.Infrastructure.DefaultNetwork.NetworkID != "" { + networkID = loadBalancerMachine.Spec.Infrastructure.DefaultNetwork.NetworkID + } + + if networkID == "" { return nil, helper.ErrNoNetworkID } + var serverNetworks []servers.Network + serverNetworks = append(serverNetworks, servers.Network{UUID: networkID, Port: *loadBalancerMachine.Status.DefaultPortID}) + + for i := range loadBalancerMachine.Spec.Infrastructure.AdditionalNetworks { + // we don't test for overlapping networks because this can be a valid use case, + // and it is hard to validate over multiple openstack objects. + serverNetworks = append(serverNetworks, servers.Network{UUID: loadBalancerMachine.Spec.Infrastructure.AdditionalNetworks[i].NetworkID}) + } var createOpts servers.CreateOptsBuilder createOpts = &servers.CreateOpts{ @@ -616,7 +680,7 @@ func (r *LoadBalancerMachineReconciler) createServer( SecurityGroups: nil, UserData: []byte(userdata), AvailabilityZone: loadBalancerMachine.Spec.Infrastructure.AvailabilityZone, - Networks: []servers.Network{{UUID: loadBalancerMachine.Spec.Infrastructure.NetworkID, Port: *loadBalancerMachine.Status.PortID}}, + Networks: serverNetworks, Metadata: nil, } @@ -726,55 +790,75 @@ func (r *LoadBalancerMachineReconciler) deleteServer( func (r *LoadBalancerMachineReconciler) deletePort( ctx context.Context, osClient os.Client, - req ctrl.Request, lbm *yawolv1beta1.LoadBalancerMachine, -) error { +) (bool, error) { var err error + var requeue bool var portClient os.PortClient portClient, err = osClient.PortClient(ctx) if err != nil { - return err + return false, err } - if lbm.Status.PortID != nil { - if err = openstackhelper.DeletePort(ctx, portClient, *lbm.Status.PortID); err != nil { + if lbm.Status.DefaultPortID != nil { + if err = openstackhelper.DeletePort(ctx, portClient, *lbm.Status.DefaultPortID); err != nil { switch err.(type) { case gophercloud.ErrDefault404: - r.Log.Info("error deleting port, already deleted", "lbm", lbm.Name) + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "defaultPortID"); err != nil { + return false, err + } default: r.Log.Info("an unexpected error occurred deleting port", "lbm", lbm.Name) - return kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) + return false, kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) } } + requeue = true } - // delete orphan ports - portName := req.NamespacedName.String() - var portList []ports.Port - portList, err = portClient.List(ctx, ports.ListOpts{Name: portName}) - if err != nil { - return err - } - - for i := range portList { - port := &portList[i] - // double check - if port.ID == "" || port.Name != portName { - continue - } - - if err = openstackhelper.DeletePort(ctx, portClient, port.ID); err != nil { + // TODO cleanup after removing deprecated fields + if lbm.Status.PortID != nil { //nolint: staticcheck // needed to be backwards compatible + //nolint: staticcheck // needed to be backwards compatible + if err = openstackhelper.DeletePort(ctx, portClient, *lbm.Status.PortID); err != nil { switch err.(type) { case gophercloud.ErrDefault404: - r.Log.Info("error deleting port, already deleted", "lbm", lbm.Name) + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "portID"); err != nil { + return false, err + } default: r.Log.Info("an unexpected error occurred deleting port", "lbm", lbm.Name) - return kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) + return false, kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) } } + requeue = true } - return nil + if requeue { + return true, nil + } + + if lbm.Status.DefaultPortName != nil { + port, err := openstackhelper.GetPortByName(ctx, portClient, *lbm.Status.DefaultPortName) + if err != nil { + return false, err + } + if port != nil && port.ID != "" { + if err = openstackhelper.DeletePort(ctx, portClient, port.ID); err != nil { + switch err.(type) { + case gophercloud.ErrDefault404: + default: + r.Log.Info("an unexpected error occurred deleting port", "lbm", lbm.Name) + return false, kubernetes.SendErrorAsEvent(r.Recorder, err, lbm) + } + } + requeue = true + } else { + if err := helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "defaultPortName"); err != nil { + return false, err + } + } + } + + return requeue, nil } func (r *LoadBalancerMachineReconciler) deleteSA( diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go index 16f53fec..3f38ce92 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go @@ -223,6 +223,35 @@ var _ = Describe("load balancer machine", func() { }) }) // openstack not working + Context("additionalNetworks features", func() { + BeforeEach(func() { + lbm.Spec.Infrastructure.AdditionalNetworks = []yawolv1beta1.LoadBalancerAdditionalNetwork{ + {NetworkID: "networkID-1"}, + {NetworkID: "networkID-2"}, + } + }) + + 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(len(actual.Spec.Infrastructure.AdditionalNetworks)).To(Equal(2)) + g.Expect(actual.Status.ServerID).ToNot(BeNil()) + + server, err := client.ServerClientObj.Get(ctx, *actual.Status.ServerID) + g.Expect(err).To(Succeed()) + g.Expect(len(server.Addresses)).To(Equal(3)) + + g.Expect(server.Addresses["networkID-1"].(servers.Address).Address).ToNot(Equal("")) + g.Expect(server.Addresses["networkID-2"].(servers.Address).Address).ToNot(Equal("")) + g.Expect(server.Addresses[lbm.Spec.Infrastructure.DefaultNetwork.NetworkID].(servers.Address).Address).ToNot(Equal("")) + }, timeout, interval).Should(Succeed()) + }) + }) // additionalNetworks features + Context("HA features", func() { It("should create openstack resources", func() { lbmNN := runtimeClient.ObjectKeyFromObject(lbm) @@ -232,8 +261,8 @@ var _ = Describe("load balancer machine", func() { var actual yawolv1beta1.LoadBalancerMachine g.Expect(k8sClient.Get(ctx, lbmNN, &actual)).To(Succeed()) - g.Expect(actual.Status.PortID).ToNot(BeNil()) - port, err := client.PortClientObj.Get(ctx, *actual.Status.PortID) + g.Expect(actual.Status.DefaultPortName).ToNot(BeNil()) + port, err := client.PortClientObj.Get(ctx, *actual.Status.DefaultPortID) g.Expect(err).To(Succeed()) g.Expect(len(port.AllowedAddressPairs)).To(Equal(1)) @@ -272,7 +301,7 @@ var _ = Describe("load balancer machine", func() { Eventually(func(g Gomega) { var actual yawolv1beta1.LoadBalancerMachine g.Expect(k8sClient.Get(ctx, lbmNN, &actual)).To(Succeed()) - g.Expect(actual.Status.PortID).To(Not(BeNil())) + g.Expect(actual.Status.DefaultPortID).To(Not(BeNil())) }, timeout, interval).Should(Succeed()) By("checking if one of the ports got used") @@ -380,8 +409,10 @@ func getMockLB() *LB { Endpoints: nil, Ports: nil, Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: pointer.String("floatingnet-id"), - NetworkID: "network-id", + DefaultNetwork: yawolv1beta1.LoadBalancerDefaultNetwork{ + FloatingNetID: pointer.String("floatingnetid"), + NetworkID: "networkid", + }, Flavor: &yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("flavor-id"), }, @@ -411,8 +442,10 @@ func getMockLBM(lb *LB) *LBM { }, PortID: "0", Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: pointer.String("floatingnet-id"), - NetworkID: "network-id", + DefaultNetwork: yawolv1beta1.LoadBalancerDefaultNetwork{ + FloatingNetID: pointer.String("floatingnetid"), + NetworkID: "networkid", + }, Flavor: &yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("flavor-id"), }, diff --git a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go index 6e39db6d..28c26936 100644 --- a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go +++ b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go @@ -59,8 +59,10 @@ var _ = Describe("LoadBalancerSet controller", func() { Labels: map[string]string{LoadBalancerLabelKey: LoadBalancerLabelValue}, Spec: yawolv1beta1.LoadBalancerMachineSpec{ Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ - FloatingNetID: pointer.String(FloatingNetID), - NetworkID: NetworkID, + DefaultNetwork: yawolv1beta1.LoadBalancerDefaultNetwork{ + FloatingNetID: pointer.String(FloatingNetID), + NetworkID: NetworkID, + }, Flavor: &yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String(FlavorID), }, diff --git a/go.mod b/go.mod index 3b94a226..f0216eca 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/envoyproxy/go-control-plane v0.10.3 github.com/go-logr/logr v1.2.3 github.com/golang/protobuf v1.5.2 - github.com/gophercloud/gophercloud v1.0.0 + github.com/gophercloud/gophercloud v1.1.1 github.com/gophercloud/utils v0.0.0-20220927104426-4113af8d2663 github.com/onsi/ginkgo/v2 v2.6.1 github.com/onsi/gomega v1.24.2 diff --git a/go.sum b/go.sum index 4681bf9a..72110635 100644 --- a/go.sum +++ b/go.sum @@ -266,8 +266,8 @@ github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+ github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/googleapis/gax-go/v2 v2.1.0/go.mod h1:Q3nei7sK6ybPYH7twZdmQpAd1MKb7pfu6SK+H1/DsU0= github.com/gophercloud/gophercloud v0.20.0/go.mod h1:wRtmUelyIIv3CSSDI47aUwbs075O6i+LY+pXsKCBsb4= -github.com/gophercloud/gophercloud v1.0.0 h1:9nTGx0jizmHxDobe4mck89FyQHVyA3CaXLIUSGJjP9k= -github.com/gophercloud/gophercloud v1.0.0/go.mod h1:Q8fZtyi5zZxPS/j9aj3sSxtvj41AdQMDwyo1myduD5c= +github.com/gophercloud/gophercloud v1.1.1 h1:MuGyqbSxiuVBqkPZ3+Nhbytk1xZxhmfCB2Rg1cJWFWM= +github.com/gophercloud/gophercloud v1.1.1/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM= github.com/gophercloud/utils v0.0.0-20220927104426-4113af8d2663 h1:cHUPA6mgL0RGwopSxYRil6v8mK4ab6yefs1PJRXPXUE= github.com/gophercloud/utils v0.0.0-20220927104426-4113af8d2663/go.mod h1:qOGlfG6OIJ193/c3Xt/XjOfHataNZdQcVgiu93LxBUM= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= @@ -467,9 +467,9 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.0.0-20211202192323-5770296d904e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= +golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= diff --git a/internal/helper/errors.go b/internal/helper/errors.go index 3f10a072..4b2b9b2a 100644 --- a/internal/helper/errors.go +++ b/internal/helper/errors.go @@ -20,6 +20,7 @@ var ( ErrLBPortNotSet = errors.New("load balancer port not set") ErrFIPIDEmpty = errors.New("fip was successfully created but fip id is empty") ErrPortIDEmpty = errors.New("port was successfully created but port id is empty") + ErrPortNameEmpty = errors.New("port name is empty") ErrServerGroupIDEmpty = errors.New("server group was successfully created but server group id is empty") ErrSecGroupIDEmpty = errors.New("secGroup was successfully created but secGroup id is empty") ErrSecGroupNil = errors.New("SecGroup is nil, cannot create rules") diff --git a/internal/helper/loadbalancermachine.go b/internal/helper/loadbalancermachine.go index 7c506408..7f550496 100644 --- a/internal/helper/loadbalancermachine.go +++ b/internal/helper/loadbalancermachine.go @@ -126,8 +126,8 @@ func parseLoadBalancerMachineOpenstackInfoMetrics( "serverID": "nil", } - if loadBalancerMachine.Status.PortID != nil { - labels["portID"] = *loadBalancerMachine.Status.PortID + if loadBalancerMachine.Status.DefaultPortID != nil { + labels["portID"] = *loadBalancerMachine.Status.DefaultPortID } if loadBalancerMachine.Status.ServerID != nil { diff --git a/internal/helper/openstack/floatingip.go b/internal/helper/openstack/floatingip.go index 1d341186..b5b73cbc 100644 --- a/internal/helper/openstack/floatingip.go +++ b/internal/helper/openstack/floatingip.go @@ -16,9 +16,18 @@ func CreateFIP( fipClient openstack.FipClient, lb *yawolv1beta1.LoadBalancer, ) (*floatingips.FloatingIP, error) { + // TODO cleanup after removing deprecated fields + var floatingNetID string + if lb.Spec.Infrastructure.FloatingNetID != nil { //nolint: staticcheck // needed to be backwards compatible + floatingNetID = *lb.Spec.Infrastructure.FloatingNetID //nolint: staticcheck // needed to be backwards compatible + } + if lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID != nil { + floatingNetID = *lb.Spec.Infrastructure.DefaultNetwork.FloatingNetID + } + fip, err := fipClient.Create(ctx, floatingips.CreateOpts{ Description: *lb.Status.FloatingName, - FloatingNetworkID: *lb.Spec.Infrastructure.FloatingNetID, + FloatingNetworkID: floatingNetID, }) if err != nil { return nil, err diff --git a/internal/openstack/testing/fake.go b/internal/openstack/testing/fake.go index 19187108..f0476f3c 100644 --- a/internal/openstack/testing/fake.go +++ b/internal/openstack/testing/fake.go @@ -265,6 +265,10 @@ func GetFakeClient() *MockClient { }, DeleteFunc: func(ctx context.Context, id string) error { prts := client.StoredValues["ports"] + _, found := prts.(map[string]*ports.Port)[id] + if !found { + return gophercloud.ErrDefault404{} + } delete(prts.(map[string]*ports.Port), id) return nil }, @@ -306,11 +310,19 @@ func GetFakeClient() *MockClient { CreateFunc: func(ctx context.Context, optsBuilder servers.CreateOptsBuilder) (*servers.Server, error) { opts := optsBuilder.(*servers.CreateOpts) + addresses := make(map[string]interface{}) + if nets, ok := opts.Networks.([]servers.Network); ok { + for i := range nets { + addresses[nets[i].UUID] = servers.Address{Address: "10.0.0.1"} + } + } + server := &servers.Server{ - ID: getID(&client), - Name: opts.Name, - Status: "ACTIVE", - Created: time.Now(), + ID: getID(&client), + Name: opts.Name, + Status: "ACTIVE", + Created: time.Now(), + Addresses: addresses, } srvs := client.StoredValues["servers"] From bb640720679a43a138c908f07aa11ec818eb9a9d Mon Sep 17 00:00:00 2001 From: Maxmilian Geberl Date: Mon, 19 Dec 2022 10:14:19 +0100 Subject: [PATCH 02/11] add portIP to lb status --- api/v1beta1/loadbalancer_types.go | 3 +++ .../loadbalancer/loadbalancer_controller.go | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index ede2868a..4a87776a 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/api/v1beta1/loadbalancer_types.go @@ -295,6 +295,9 @@ type LoadBalancerStatus struct { // PortName is the current openstack name from the virtual Port. // +optional PortName *string `json:"portName,omitempty"` + // PortIP is the IP from the openstack virtual Port. + // +optional + PortIP *string `json:"portIP,omitempty"` // ServerGroupID is the current sever group ID // +optional ServerGroupID *string `json:"serverGroupID,omitempty"` diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go index 13f14c3e..e2241ed4 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go @@ -582,6 +582,17 @@ func (r *Reconciler) reconcilePort( return true, nil } + // Patch PortIP to status + if len(port.FixedIPs) >= 1 && + (lb.Status.PortIP == nil || *lb.Status.PortIP != port.FixedIPs[0].IPAddress) { + if err := helper.PatchLBStatus(ctx, r.Status(), lb, yawolv1beta1.LoadBalancerStatus{ + PortIP: &port.FixedIPs[0].IPAddress, + }); err != nil { + return false, err + } + requeue = true + } + // If internal LB, use first port ip as external ip if lb.Spec.Options.InternalLB && len(port.FixedIPs) >= 1 && (lb.Status.ExternalIP == nil || *lb.Status.ExternalIP != port.FixedIPs[0].IPAddress) { @@ -1183,6 +1194,7 @@ func (r *Reconciler) deletePorts( case gophercloud.ErrDefault404, gophercloud.ErrResourceNotFound: r.Log.Info("port has already been deleted", "lb", lb.Namespace+"/"+lb.Name, "portID", *lb.Status.PortID) // requeue true to clean orphan ports in the next run + _ = helper.RemoveFromLBStatus(ctx, r.Status(), lb, "portIP") return true, helper.RemoveFromLBStatus(ctx, r.Status(), lb, "portID") default: r.Log.Info("an unexpected error occurred retrieving port", "lb", lb.Namespace+"/"+lb.Name, "portID", *lb.Status.PortID) From ce45249bde5e09ab25ad70fe420ae29a3ddfcc18 Mon Sep 17 00:00:00 2001 From: einfachnuralex Date: Mon, 19 Dec 2022 10:19:29 +0100 Subject: [PATCH 03/11] add projectID to infrastructure to overwrite the openstack projectID from the secretRef --- api/v1beta1/loadbalancer_types.go | 7 ++ api/v1beta1/zz_generated.deepcopy.go | 10 +++ ...ol.stackit.cloud_loadbalancermachines.yaml | 5 ++ .../yawol.stackit.cloud_loadbalancers.yaml | 8 +++ .../yawol.stackit.cloud_loadbalancersets.yaml | 6 ++ .../targetcontroller/service_controller.go | 16 ++++- .../loadbalancer/loadbalancer_controller.go | 15 +++-- .../loadbalancer_controller_test.go | 2 +- .../loadbalancerset_status_controller_test.go | 2 +- .../loadbalancermachine_controller.go | 16 +++-- .../loadbalancermachine_controller_test.go | 2 +- internal/helper/errors.go | 1 + internal/helper/openstack/client.go | 16 +++-- internal/helper/openstack/port.go | 5 +- internal/openstack/client.go | 64 +++++++++++-------- internal/openstack/interface.go | 2 +- internal/openstack/testing/client.go | 2 +- 17 files changed, 127 insertions(+), 52 deletions(-) diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index 4a87776a..019ccc96 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/api/v1beta1/loadbalancer_types.go @@ -15,6 +15,9 @@ 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" + // ServiceDefaultProjectID overwrites the projectID which is set by the secret. + // If not set the settings from the secret binding will be used. + ServiceDefaultProjectID = "yawol.stackit.cloud/projectID" // ServiceFloatingNetworkID overwrites the openstack floating network for the loadbalancer ServiceFloatingNetworkID = "yawol.stackit.cloud/floatingNetworkID" // ServiceAvailabilityZone set availability zone for specific service @@ -194,6 +197,10 @@ type LoadBalancerInfrastructure struct { AvailabilityZone string `json:"availabilityZone"` // AuthSecretRef defines a secretRef for the openstack secret. AuthSecretRef corev1.SecretReference `json:"authSecretRef"` + // ProjectID defines an openstack project ID which will be used instead of the project from the secret ref. + // If not set the project from the secret ref will be used. + // +optional + ProjectID *string `json:"projectID"` } // LoadBalancerAdditionalNetwork defines additional networks for the LoadBalancer diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index c7b6a724..7b278077 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -133,6 +133,11 @@ func (in *LoadBalancerInfrastructure) DeepCopyInto(out *LoadBalancerInfrastructu (*in).DeepCopyInto(*out) } out.AuthSecretRef = in.AuthSecretRef + if in.ProjectID != nil { + in, out := &in.ProjectID, &out.ProjectID + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerInfrastructure. @@ -620,6 +625,11 @@ func (in *LoadBalancerStatus) DeepCopyInto(out *LoadBalancerStatus) { *out = new(string) **out = **in } + if in.PortIP != nil { + in, out := &in.PortIP, &out.PortIP + *out = new(string) + **out = **in + } if in.ServerGroupID != nil { in, out := &in.ServerGroupID, &out.ServerGroupID *out = new(string) diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml index a449021a..e6ce1396 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml @@ -184,6 +184,11 @@ spec: description: 'Deprecated: use defaultNetwork instead NetworkID defines a openstack ID for the network.' type: string + projectID: + description: ProjectID defines an openstack project ID which will + be used instead of the project from the secret ref. If not set + the project from the secret ref will be used. + type: string required: - authSecretRef 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 8df78225..3ac06307 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml @@ -215,6 +215,11 @@ spec: description: 'Deprecated: use defaultNetwork instead NetworkID defines a openstack ID for the network.' type: string + projectID: + description: ProjectID defines an openstack project ID which will + be used instead of the project from the secret ref. If not set + the project from the secret ref will be used. + type: string required: - authSecretRef type: object @@ -414,6 +419,9 @@ spec: portID: description: PortID is the current openstack ID from the virtual Port. type: string + portIP: + description: PortIP is the IP from the openstack virtual Port. + type: string portName: description: PortName is the current openstack name from the virtual Port. diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml index 43d2a307..3aa25da3 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml @@ -250,6 +250,12 @@ spec: description: 'Deprecated: use defaultNetwork instead NetworkID defines a openstack ID for the network.' type: string + projectID: + description: ProjectID defines an openstack project ID + which will be used instead of the project from the secret + ref. If not set the project from the secret ref will + be used. + type: string required: - authSecretRef type: object diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index 5346734e..be7d8462 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go @@ -206,6 +206,7 @@ func (r *ServiceReconciler) createLoadBalancer( Flavor: infraConfig.FlavorRef, Image: infraConfig.ImageRef, AvailabilityZone: *infraConfig.AvailabilityZone, + ProjectID: getProjectID(svc), AuthSecretRef: coreV1.SecretReference{ Name: *infraConfig.AuthSecretName, Namespace: *infraConfig.Namespace, @@ -231,11 +232,17 @@ func (r *ServiceReconciler) reconcileInfrastructure( Flavor: infraConfig.FlavorRef, Image: infraConfig.ImageRef, AvailabilityZone: *infraConfig.AvailabilityZone, + ProjectID: getProjectID(svc), AuthSecretRef: coreV1.SecretReference{ Name: *infraConfig.AuthSecretName, Namespace: *infraConfig.Namespace, }, } + + if !reflect.DeepEqual(newInfra.ProjectID, lb.Spec.Infrastructure.ProjectID) { + return helper.ErrProjectIsImmutable + } + if !reflect.DeepEqual(newInfra, lb.Spec.Infrastructure) { newInfraJSON, err := json.Marshal(newInfra) if err != nil { @@ -648,6 +655,13 @@ func getDefaultNetwork( floatingNetID := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID] defaultNetwork.FloatingNetID = &floatingNetID } - return defaultNetwork } + +func getProjectID(svc *coreV1.Service) *string { + if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultProjectID]; ok { + projectID := svc.Annotations[yawolv1beta1.ServiceDefaultProjectID] + return &projectID + } + return nil +} diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go index e2241ed4..14ccafcd 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go @@ -45,7 +45,7 @@ type Reconciler struct { skipReconciles bool skipAllButNN *types.NamespacedName Metrics *helpermetrics.LoadBalancerMetricList - getOsClientForIni func(iniData []byte) (openstack.Client, error) + getOsClientForIni openstack.GetOSClientFunc WorkerCount int OpenstackTimeout time.Duration } @@ -73,7 +73,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } // get OpenStack Client for LoadBalancer - osClient, err := openstackhelper.GetOpenStackClientForAuthRef(ctx, r.Client, lb.Spec.Infrastructure.AuthSecretRef, r.getOsClientForIni) + osClient, err := openstackhelper.GetOpenStackClientForInfrastructure(ctx, r.Client, lb.Spec.Infrastructure, r.getOsClientForIni) if err != nil { return ctrl.Result{}, err } @@ -114,9 +114,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { if r.getOsClientForIni == nil { - r.getOsClientForIni = func(iniData []byte) (openstack.Client, error) { + r.getOsClientForIni = func(iniData []byte, overwrite openstack.OSClientOverwrite) (openstack.Client, error) { osClient := openstack.OSClient{} - err := osClient.Configure(iniData, r.OpenstackTimeout, r.Metrics.OpenstackMetrics) + err := osClient.Configure(iniData, overwrite, r.OpenstackTimeout, r.Metrics.OpenstackMetrics) if err != nil { return nil, err } @@ -471,6 +471,7 @@ func (r *Reconciler) reconcilePort( var err error portClient, err = osClient.PortClient(ctx) + if err != nil { return false, err } @@ -515,7 +516,11 @@ func (r *Reconciler) reconcilePort( networkID = lb.Spec.Infrastructure.DefaultNetwork.NetworkID } - port, err = openstackhelper.CreatePort(ctx, portClient, *lb.Status.PortName, networkID) + port, err = openstackhelper.CreatePort( + ctx, + portClient, + *lb.Status.PortName, + networkID) if err != nil { r.Log.Info("unexpected error occurred claiming a port", "lb", req.NamespacedName) return false, kubernetes.SendErrorAsEvent(r.RecorderLB, err, lb) diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go index fe112c70..d19642fe 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go @@ -54,7 +54,7 @@ var _ = Describe("loadbalancer controller", func() { lb = getMockLB(lbNN) client = testing.GetFakeClient() - loadBalancerReconciler.getOsClientForIni = func(iniData []byte) (openstack.Client, error) { + loadBalancerReconciler.getOsClientForIni = func(_ []byte, _ openstack.OSClientOverwrite) (openstack.Client, error) { return client, nil } }) diff --git a/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go index feea999b..954db776 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go @@ -144,7 +144,7 @@ var _ = Describe("LB Status update", func() { By("Setup - Mocks") - loadBalancerReconciler.getOsClientForIni = func(iniData []byte) (openstack.Client, error) { + loadBalancerReconciler.getOsClientForIni = func(_ []byte, _ openstack.OSClientOverwrite) (openstack.Client, error) { return testing.GetFakeClient(), nil } diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index 17f19de9..61da5c53 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go @@ -56,7 +56,7 @@ type LoadBalancerMachineReconciler struct { //nolint:revive // naming from kubeb RecorderLB record.EventRecorder APIEndpoint string Metrics *helpermetrics.LoadBalancerMachineMetricList - getOsClientForIni func(iniData []byte) (os.Client, error) + getOsClientForIni os.GetOSClientFunc WorkerCount int OpenstackTimeout time.Duration } @@ -78,10 +78,10 @@ func (r *LoadBalancerMachineReconciler) Reconcile(ctx context.Context, req ctrl. var err error var osClient os.Client - osClient, err = openstackhelper.GetOpenStackClientForAuthRef( + osClient, err = openstackhelper.GetOpenStackClientForInfrastructure( ctx, r.Client, - loadBalancerMachine.Spec.Infrastructure.AuthSecretRef, + loadBalancerMachine.Spec.Infrastructure, r.getOsClientForIni, ) if err != nil { @@ -199,9 +199,9 @@ func (r *LoadBalancerMachineReconciler) Reconcile(ctx context.Context, req ctrl. // SetupWithManager is used by kubebuilder to init the controller loop func (r *LoadBalancerMachineReconciler) SetupWithManager(mgr ctrl.Manager) error { if r.getOsClientForIni == nil { - r.getOsClientForIni = func(iniData []byte) (os.Client, error) { + r.getOsClientForIni = func(iniData []byte, overwrite os.OSClientOverwrite) (os.Client, error) { osClient := os.OSClient{} - err := osClient.Configure(iniData, r.OpenstackTimeout, r.Metrics.OpenstackMetrics) + err := osClient.Configure(iniData, overwrite, r.OpenstackTimeout, r.Metrics.OpenstackMetrics) if err != nil { return nil, err } @@ -415,7 +415,11 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( // create port if port == nil { - port, err = openstackhelper.CreatePort(ctx, portClient, *lbm.Status.DefaultPortName, networkID) + port, err = openstackhelper.CreatePort( + ctx, + portClient, + *lbm.Status.DefaultPortName, + networkID) 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 3f38ce92..96f210cc 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go @@ -72,7 +72,7 @@ var _ = Describe("load balancer machine", func() { }) Expect(err).ToNot(HaveOccurred()) - loadBalancerMachineReconciler.getOsClientForIni = func(iniData []byte) (openstack.Client, error) { + loadBalancerMachineReconciler.getOsClientForIni = func(_ []byte, _ openstack.OSClientOverwrite) (openstack.Client, error) { return client, nil } }) diff --git a/internal/helper/errors.go b/internal/helper/errors.go index 4b2b9b2a..dfef32a4 100644 --- a/internal/helper/errors.go +++ b/internal/helper/errors.go @@ -65,4 +65,5 @@ var ( ErrCouldNotParseSourceRange = errors.New("could not parse LoadBalancerSourceRange") ErrListingChildLBMs = errors.New("unable to list child loadbalancerMachines") ErrUnsupportedProtocol = errors.New("unsupported protocol used (TCP and UDP is supported)") + ErrProjectIsImmutable = errors.New("project id is immutable, cant be changed after inital creation") ) diff --git a/internal/helper/openstack/client.go b/internal/helper/openstack/client.go index 63d5b3d0..bd3a1278 100644 --- a/internal/helper/openstack/client.go +++ b/internal/helper/openstack/client.go @@ -3,6 +3,7 @@ package openstack import ( "context" "fmt" + yawolv1beta1 "github.com/stackitcloud/yawol/api/v1beta1" "github.com/stackitcloud/yawol/internal/openstack" @@ -10,17 +11,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func GetOpenStackClientForAuthRef( +func GetOpenStackClientForInfrastructure( ctx context.Context, c client.Client, - authRef coreV1.SecretReference, - getOsClientForIni func(iniData []byte) (openstack.Client, error), + infra yawolv1beta1.LoadBalancerInfrastructure, + getOsClientForIni openstack.GetOSClientFunc, ) (openstack.Client, error) { // get openstack infrastructure secret var infraSecret coreV1.Secret err := c.Get(ctx, client.ObjectKey{ - Name: authRef.Name, - Namespace: authRef.Namespace, + Name: infra.AuthSecretRef.Name, + Namespace: infra.AuthSecretRef.Namespace, }, &infraSecret) if err != nil { return nil, err @@ -33,7 +34,10 @@ func GetOpenStackClientForAuthRef( // create openstack client from secret var osClient openstack.Client - if osClient, err = getOsClientForIni(infraSecret.Data["cloudprovider.conf"]); err != nil { + if osClient, err = getOsClientForIni( + infraSecret.Data["cloudprovider.conf"], + openstack.OSClientOverwrite{ProjectID: infra.ProjectID}, + ); err != nil { return nil, err } return osClient, nil diff --git a/internal/helper/openstack/port.go b/internal/helper/openstack/port.go index 6e8a9548..3081ae93 100644 --- a/internal/helper/openstack/port.go +++ b/internal/helper/openstack/port.go @@ -39,10 +39,11 @@ func CreatePort( portName string, networkID string, ) (*ports.Port, error) { - port, err := portClient.Create(ctx, ports.CreateOpts{ + opts := ports.CreateOpts{ Name: portName, NetworkID: networkID, - }) + } + port, err := portClient.Create(ctx, opts) if err != nil { return nil, err } diff --git a/internal/openstack/client.go b/internal/openstack/client.go index 96dee62d..7959d395 100644 --- a/internal/openstack/client.go +++ b/internal/openstack/client.go @@ -13,6 +13,12 @@ import ( "gopkg.in/ini.v1" ) +type GetOSClientFunc func(iniData []byte, overwrite OSClientOverwrite) (Client, error) + +type OSClientOverwrite struct { + ProjectID *string +} + // OSClient is an implementation of Client. It must be configured by calling Configure(). // When requesting any specific client the required resources will be created on first call. Mind, that you should // not call Configure() again, because the created resources will not be invalidated. @@ -20,6 +26,7 @@ type OSClient struct { networkV2 *gophercloud.ServiceClient computeV2 *gophercloud.ServiceClient ini []byte + overwrite OSClientOverwrite timeout time.Duration promCounter *prometheus.CounterVec } @@ -36,10 +43,11 @@ type OSClient struct { // username="itmyuser" // password="suupersecret" // region="eu01" -func (r *OSClient) Configure(iniBytes []byte, timeout time.Duration, promCounter *prometheus.CounterVec) error { +func (r *OSClient) Configure(iniBytes []byte, overwrite OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error { r.ini = iniBytes r.timeout = timeout r.promCounter = promCounter + r.overwrite = overwrite return nil } @@ -48,7 +56,7 @@ func (r *OSClient) Configure(iniBytes []byte, timeout time.Duration, promCounter func (r *OSClient) FipClient(ctx context.Context) (FipClient, error) { if r.networkV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createNetworkV2FromIni(ctx, r.ini, r.timeout) + sc, err := createNetworkV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -64,7 +72,7 @@ func (r *OSClient) FipClient(ctx context.Context) (FipClient, error) { func (r *OSClient) PortClient(ctx context.Context) (PortClient, error) { if r.networkV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createNetworkV2FromIni(ctx, r.ini, r.timeout) + sc, err := createNetworkV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -80,7 +88,7 @@ func (r *OSClient) PortClient(ctx context.Context) (PortClient, error) { func (r *OSClient) GroupClient(ctx context.Context) (GroupClient, error) { if r.networkV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createNetworkV2FromIni(ctx, r.ini, r.timeout) + sc, err := createNetworkV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -96,7 +104,7 @@ func (r *OSClient) GroupClient(ctx context.Context) (GroupClient, error) { func (r *OSClient) RuleClient(ctx context.Context) (RuleClient, error) { if r.networkV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createNetworkV2FromIni(ctx, r.ini, r.timeout) + sc, err := createNetworkV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -112,7 +120,7 @@ func (r *OSClient) RuleClient(ctx context.Context) (RuleClient, error) { func (r *OSClient) ServerClient(ctx context.Context) (ServerClient, error) { if r.computeV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createComputeV2FromIni(ctx, r.ini, r.timeout) + sc, err := createComputeV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -128,7 +136,7 @@ func (r *OSClient) ServerClient(ctx context.Context) (ServerClient, error) { func (r *OSClient) ServerGroupClient(ctx context.Context) (ServerGroupClient, error) { if r.computeV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createComputeV2FromIni(ctx, r.ini, r.timeout) + sc, err := createComputeV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -144,7 +152,7 @@ func (r *OSClient) ServerGroupClient(ctx context.Context) (ServerGroupClient, er func (r *OSClient) KeyPairClient(ctx context.Context) (KeyPairClient, error) { if r.computeV2 == nil { var sc *gophercloud.ServiceClient - sc, err := createComputeV2FromIni(ctx, r.ini, r.timeout) + sc, err := createComputeV2FromIni(ctx, r.ini, r.overwrite, r.timeout) if err != nil { return nil, err } @@ -155,8 +163,8 @@ func (r *OSClient) KeyPairClient(ctx context.Context) (KeyPairClient, error) { return client.Configure(r.computeV2, r.timeout, r.promCounter), nil } -func createNetworkV2FromIni(ctx context.Context, iniData []byte, timeout time.Duration) (*gophercloud.ServiceClient, error) { - provider, opts, err := getProvider(ctx, iniData, timeout) +func createNetworkV2FromIni(ctx context.Context, iniData []byte, overwrite OSClientOverwrite, timeout time.Duration) (*gophercloud.ServiceClient, error) { + provider, opts, err := getProvider(ctx, iniData, overwrite, timeout) if err != nil { return nil, err } @@ -169,8 +177,8 @@ func createNetworkV2FromIni(ctx context.Context, iniData []byte, timeout time.Du return client, nil } -func createComputeV2FromIni(ctx context.Context, iniData []byte, timeout time.Duration) (*gophercloud.ServiceClient, error) { - provider, opts, err := getProvider(ctx, iniData, timeout) +func createComputeV2FromIni(ctx context.Context, iniData []byte, overwrite OSClientOverwrite, timeout time.Duration) (*gophercloud.ServiceClient, error) { + provider, opts, err := getProvider(ctx, iniData, overwrite, timeout) if err != nil { return nil, err } @@ -186,6 +194,7 @@ func createComputeV2FromIni(ctx context.Context, iniData []byte, timeout time.Du func getProvider( ctx context.Context, iniData []byte, + overwrite OSClientOverwrite, timeout time.Duration, ) (*gophercloud.ProviderClient, *gophercloud.EndpointOpts, error) { cfg, err := ini.Load(iniData) @@ -193,23 +202,24 @@ func getProvider( return nil, nil, err } - var authURL, username, password, domainName, projectName, region string - authURL = strings.TrimSpace(cfg.Section("Global").Key("auth-url").String()) - username = strings.TrimSpace(cfg.Section("Global").Key("username").String()) - password = strings.TrimSpace(cfg.Section("Global").Key("password").String()) - domainName = strings.TrimSpace(cfg.Section("Global").Key("domain-name").String()) - projectName = strings.TrimSpace(cfg.Section("Global").Key("tenant-name").String()) - region = strings.TrimSpace(cfg.Section("Global").Key("region").String()) + var authInfo clientconfig.AuthInfo - clientOpts := new(clientconfig.ClientOpts) - authInfo := &clientconfig.AuthInfo{ - AuthURL: authURL, - Username: username, - Password: password, - DomainName: domainName, - ProjectName: projectName, + authURL := strings.TrimSpace(cfg.Section("Global").Key("auth-url").String()) + authInfo.AuthURL = authURL + authInfo.Username = strings.TrimSpace(cfg.Section("Global").Key("username").String()) + authInfo.Password = strings.TrimSpace(cfg.Section("Global").Key("password").String()) + authInfo.DomainName = strings.TrimSpace(cfg.Section("Global").Key("domain-name").String()) + authInfo.ProjectName = strings.TrimSpace(cfg.Section("Global").Key("tenant-name").String()) + + region := strings.TrimSpace(cfg.Section("Global").Key("region").String()) + + if overwrite.ProjectID != nil { + authInfo.ProjectName = "" + authInfo.ProjectID = *overwrite.ProjectID } - clientOpts.AuthInfo = authInfo + + clientOpts := new(clientconfig.ClientOpts) + clientOpts.AuthInfo = &authInfo ao, err := clientconfig.AuthOptions(clientOpts) if err != nil { diff --git a/internal/openstack/interface.go b/internal/openstack/interface.go index 8fe04f97..1d355cc8 100644 --- a/internal/openstack/interface.go +++ b/internal/openstack/interface.go @@ -44,7 +44,7 @@ import ( // Client provides a interface to configure and use different OpenStack clients. type Client interface { // Takes the content of an ini-file, to configure the openstack client - Configure(ini []byte, timeout time.Duration, promCounter *prometheus.CounterVec) error + Configure(ini []byte, overwrite OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error // Returns the FipClient created from the configured ini FipClient(ctx context.Context) (FipClient, error) // Returns the PortClient created from the configured ini diff --git a/internal/openstack/testing/client.go b/internal/openstack/testing/client.go index be2835c7..b93cc3cb 100644 --- a/internal/openstack/testing/client.go +++ b/internal/openstack/testing/client.go @@ -39,7 +39,7 @@ type MockClient struct { ServerGroupClientObj openstack.ServerGroupClient } -func (r *MockClient) Configure(ini []byte, timeout time.Duration, promCounter *prometheus.CounterVec) error { +func (r *MockClient) Configure(ini []byte, overwrite openstack.OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error { r.StoredValues = make(map[string]interface{}) return nil } From 7c7e67a2eed860564effefbb43389d56e4595549 Mon Sep 17 00:00:00 2001 From: Maxmilian Geberl Date: Mon, 19 Dec 2022 15:31:05 +0100 Subject: [PATCH 04/11] add flag to skip yawol cloud controller default IP as additional network if default is overwritten --- api/v1beta1/loadbalancer_types.go | 3 +++ .../targetcontroller/service_controller.go | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index 019ccc96..87fb5116 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/api/v1beta1/loadbalancer_types.go @@ -15,6 +15,9 @@ 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" + // 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" // ServiceDefaultProjectID overwrites the projectID which is set by the secret. // If not set the settings from the secret binding will be used. ServiceDefaultProjectID = "yawol.stackit.cloud/projectID" diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index be7d8462..99c574d2 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go @@ -626,7 +626,9 @@ func getAdditionalNetworks( if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { if svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID] != *infraConfig.NetworkID { - networkIDs[*infraConfig.NetworkID] = struct{}{} + if skip, _ := strconv.ParseBool(svc.Annotations[yawolv1beta1.ServiceSkipCloudControllerDefaultNetworkID]); !skip { + networkIDs[*infraConfig.NetworkID] = struct{}{} + } } } From 77bc05ec8811f5b54006a6bc0cf44d94fb937117 Mon Sep 17 00:00:00 2001 From: Maxmilian Geberl Date: Tue, 20 Dec 2022 10:01:22 +0100 Subject: [PATCH 05/11] change to dhcpcd in yawollet image for ipv6 --- image/install-alpine.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/image/install-alpine.yaml b/image/install-alpine.yaml index 61008ce0..70b26230 100644 --- a/image/install-alpine.yaml +++ b/image/install-alpine.yaml @@ -7,6 +7,22 @@ - name: update alpine command: "apk upgrade --update-cache --available" + # use dhcpcd instead of udhcpd (for ipv6) + - name: install dhcpcd + command: "apk add dhcpcd" + + - name: + ansible.builtin.lineinfile: + path: /etc/dhcpcd.conf + search_string: 'slaac private' + line: '#slaac private' + + - name: + ansible.builtin.lineinfile: + path: /etc/dhcpcd.conf + search_string: 'slaac hwaddr' + line: 'slaac hwaddr' + # keepalived - name: install keepalived command: "apk add keepalived" From 1b662478043c372241425f95a4174d2b63a6205a Mon Sep 17 00:00:00 2001 From: Maxmilian Geberl Date: Tue, 20 Dec 2022 16:35:15 +0100 Subject: [PATCH 06/11] change DnsLookupFamily in cluster to auto instead of ipv4 only --- internal/helper/yawollet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/helper/yawollet.go b/internal/helper/yawollet.go index 8ea1d8bb..3616c14c 100644 --- a/internal/helper/yawollet.go +++ b/internal/helper/yawollet.go @@ -253,7 +253,7 @@ func createEnvoyCluster(lb *yawolv1beta1.LoadBalancer) []envoytypes.Resource { ClusterName: fmt.Sprintf("%v-%v", port.Protocol, port.Port), Endpoints: endpoints, }, - DnsLookupFamily: envoycluster.Cluster_V4_ONLY, + DnsLookupFamily: envoycluster.Cluster_AUTO, HealthChecks: healthChecks, TransportSocket: transportSocket, PerConnectionBufferLimitBytes: &wrappers.UInt32Value{Value: 32768}, // 32Kib From b1c541d4d431446b5423504bc36aec392e4fa666 Mon Sep 17 00:00:00 2001 From: Maxmilian Geberl Date: Wed, 21 Dec 2022 16:10:01 +0100 Subject: [PATCH 07/11] add defaultPortIP to lbm status --- api/v1beta1/loadbalancermachine_types.go | 5 ++++- api/v1beta1/zz_generated.deepcopy.go | 5 +++++ .../yawol.stackit.cloud_loadbalancermachines.yaml | 6 +++++- .../loadbalancermachine_controller.go | 11 +++++++++++ .../loadbalancermachine_controller_test.go | 5 +++++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/api/v1beta1/loadbalancermachine_types.go b/api/v1beta1/loadbalancermachine_types.go index 710b8237..3690913f 100644 --- a/api/v1beta1/loadbalancermachine_types.go +++ b/api/v1beta1/loadbalancermachine_types.go @@ -75,9 +75,12 @@ type LoadBalancerMachineStatus struct { // DefaultPortID contains the default openstack port ID for a LoadBalancerMachine. // +optional DefaultPortID *string `json:"defaultPortID,omitempty"` - // DefaultPortID contains the default openstack port ID for a LoadBalancerMachine. + // DefaultPortName contains the default openstack port Name for a LoadBalancerMachine. // +optional DefaultPortName *string `json:"defaultPortName,omitempty"` + // DefaultPortIP contains the default openstack port IP for a LoadBalancerMachine. + // +optional + DefaultPortIP *string `json:"defaultPortIP,omitempty"` // ServiceAccountName contains the namespacedName from the ServiceAccount for a LoadBalancerMachine. // +optional ServiceAccountName *string `json:"serviceAccountName,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 7b278077..117ae066 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -342,6 +342,11 @@ func (in *LoadBalancerMachineStatus) DeepCopyInto(out *LoadBalancerMachineStatus *out = new(string) **out = **in } + if in.DefaultPortIP != nil { + in, out := &in.DefaultPortIP, &out.DefaultPortIP + *out = new(string) + **out = **in + } if in.ServiceAccountName != nil { in, out := &in.ServiceAccountName, &out.ServiceAccountName *out = new(string) diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml index e6ce1396..b9d93197 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml @@ -266,8 +266,12 @@ spec: description: DefaultPortID contains the default openstack port ID for a LoadBalancerMachine. type: string + defaultPortIP: + description: DefaultPortIP contains the default openstack port IP + for a LoadBalancerMachine. + type: string defaultPortName: - description: DefaultPortID contains the default openstack port ID + description: DefaultPortName contains the default openstack port Name for a LoadBalancerMachine. type: string lastOpenstackReconcile: diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index 61da5c53..46592bf7 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go @@ -476,6 +476,16 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( return helper.ErrFailedToCreatePortForLBM } + // Patch defaultPortIP to status + if len(port.FixedIPs) >= 1 && + (lbm.Status.DefaultPortIP == nil || *lbm.Status.DefaultPortIP != port.FixedIPs[0].IPAddress) { + if err := helper.PatchLBMStatus(ctx, r.Status(), lbm, yawolv1beta1.LoadBalancerMachineStatus{ + DefaultPortIP: &port.FixedIPs[0].IPAddress, + }); err != nil { + return err + } + } + return nil } @@ -808,6 +818,7 @@ func (r *LoadBalancerMachineReconciler) deletePort( if err = openstackhelper.DeletePort(ctx, portClient, *lbm.Status.DefaultPortID); err != nil { switch err.(type) { case gophercloud.ErrDefault404: + _ = helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "defaultPortIP") if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "defaultPortID"); err != nil { return false, err } diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go index 96f210cc..029205db 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go @@ -262,9 +262,14 @@ var _ = Describe("load balancer machine", func() { g.Expect(k8sClient.Get(ctx, lbmNN, &actual)).To(Succeed()) g.Expect(actual.Status.DefaultPortName).ToNot(BeNil()) + g.Expect(actual.Status.DefaultPortIP).ToNot(BeNil()) + g.Expect(actual.Status.DefaultPortID).ToNot(BeNil()) port, err := client.PortClientObj.Get(ctx, *actual.Status.DefaultPortID) g.Expect(err).To(Succeed()) g.Expect(len(port.AllowedAddressPairs)).To(Equal(1)) + g.Expect(len(port.AllowedAddressPairs)).To(Equal(1)) + g.Expect(len(port.FixedIPs)).To(Equal(1)) + g.Expect(port.FixedIPs[0].IPAddress).To(Equal(*actual.Status.DefaultPortIP)) var actualLB yawolv1beta1.LoadBalancer g.Expect(k8sClient.Get(ctx, lbNN, &actualLB)).To(Succeed()) From b0599896915cae1b9070590d9cf86365c765d6cf Mon Sep 17 00:00:00 2001 From: Maxmilian Geberl Date: Wed, 21 Dec 2022 16:21:45 +0100 Subject: [PATCH 08/11] add tests, docs and fix lint --- README.md | 7 + .../targetcontroller/service_controller.go | 2 +- .../service_controller_test.go | 154 ++++++++++++++++++ .../loadbalancer/loadbalancer_controller.go | 5 +- .../loadbalancer_controller_test.go | 12 +- .../loadbalancermachine_controller.go | 2 +- internal/helper/errors.go | 2 +- internal/helper/openstack/client.go | 1 + internal/openstack/client.go | 21 ++- internal/openstack/testing/client.go | 2 +- 10 files changed, 197 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 80d0342e..c64947d3 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,13 @@ metadata: # 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. yawol.stackit.cloud/defaultNetworkID: "OS-networkID" + # If set to true it do not add the default network ID from + # the yawol-cloud-controller to the additionalNetworks. + yawol.stackit.cloud/skipCloudControllerDefaultNetworkID: "false" + # Overwrites the projectID which is set by the secret. + # If not set the settings from the secret binding will be used. + # This field is immutable and can not be changed after the service is created. + yawol.stackit.cloud/projectID: "OS-ProjectID" # Overwrites the openstack floating network for the loadbalancer. yawol.stackit.cloud/floatingNetworkID: "OS-floatingNetID" # Override the default OpenStack availability zone. diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index 99c574d2..5f122345 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go @@ -240,7 +240,7 @@ func (r *ServiceReconciler) reconcileInfrastructure( } if !reflect.DeepEqual(newInfra.ProjectID, lb.Spec.Infrastructure.ProjectID) { - return helper.ErrProjectIsImmutable + return kubernetes.SendErrorAsEvent(r.Recorder, helper.ErrProjectIsImmutable, svc) } if !reflect.DeepEqual(newInfra, lb.Spec.Infrastructure) { diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go index b4e31758..894c13f1 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go @@ -2108,5 +2108,159 @@ var _ = Describe("Check loadbalancer reconcile", func() { return fmt.Errorf("additionalNetwork are still set %v", lb.Spec.Infrastructure.AdditionalNetworks) }, time.Second*5, time.Millisecond*500).Should(Succeed()) }) + + It("should update the skipCloudControllerDefaultNetworkID field", func() { + By("creating a service without skipCloudControllerDefaultNetworkID") + service := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-test31", + Namespace: "default", + Annotations: map[string]string{ + yawolv1beta1.ServiceDefaultNetworkID: "default-networkID", + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "port1", + Protocol: v1.ProtocolTCP, + Port: 12345, + TargetPort: intstr.IntOrString{IntVal: 12345}, + NodePort: 31031, + }, + }, + Type: "LoadBalancer", + }} + Expect(k8sClient.Create(ctx, &service)).Should(Succeed()) + + By("checking that defaultNetworkID is in additional networks") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test31", Namespace: "default"}, &lb) + if err != nil { + return err + } + if len(lb.Spec.Infrastructure.AdditionalNetworks) == 1 && + lb.Spec.Infrastructure.AdditionalNetworks[0].NetworkID == *testInfraDefaults.NetworkID { + return nil + } + return fmt.Errorf("additionalNetwork is not set with defaultNetwork %v", lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to add skipCloudControllerDefaultNetworkID") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{ + yawolv1beta1.ServiceDefaultNetworkID: "default-networkID", + yawolv1beta1.ServiceSkipCloudControllerDefaultNetworkID: "true", + } + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("check if default network is not present in additionalNetworks") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test31", Namespace: "default"}, &lb) + if err != nil { + return err + } + if len(lb.Spec.Infrastructure.AdditionalNetworks) == 0 { + return nil + } + return fmt.Errorf("additionalNetworks are not correct %v", lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc to disable skipCloudControllerDefaultNetworkID") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{ + yawolv1beta1.ServiceDefaultNetworkID: "default-networkID", + } + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("checking that defaultNetworkID is in additional networks") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test31", Namespace: "default"}, &lb) + if err != nil { + return err + } + if len(lb.Spec.Infrastructure.AdditionalNetworks) == 1 && + lb.Spec.Infrastructure.AdditionalNetworks[0].NetworkID == *testInfraDefaults.NetworkID { + return nil + } + return fmt.Errorf("additionalNetwork is not set with defaultNetwork %v", lb.Spec.Infrastructure.AdditionalNetworks) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + }) + + It("should be possible to add a project - but it is immutable", func() { + By("creating a service with different project") + service := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-test32", + Namespace: "default", + Annotations: map[string]string{ + yawolv1beta1.ServiceDefaultProjectID: "otherProject", + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "port1", + Protocol: v1.ProtocolTCP, + Port: 12345, + TargetPort: intstr.IntOrString{IntVal: 12345}, + NodePort: 31032, + }, + }, + Type: "LoadBalancer", + }} + Expect(k8sClient.Create(ctx, &service)).Should(Succeed()) + + By("checking that projectID is set") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test32", Namespace: "default"}, &lb) + if err != nil { + return err + } + if lb.Spec.Infrastructure.ProjectID != nil && + *lb.Spec.Infrastructure.ProjectID == "otherProject" { + return nil + } + return fmt.Errorf("projectID is not set in infrastructure %v", lb.Spec.Infrastructure.ProjectID) + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("update svc with other project - should not work") + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed()) + service.ObjectMeta.Annotations = map[string]string{ + yawolv1beta1.ServiceDefaultProjectID: "otherProject2", + } + Expect(k8sClient.Update(ctx, &service)).Should(Succeed()) + + By("check for error event") + Eventually(func() error { + eventList := v1.EventList{} + err := k8sClient.List(ctx, &eventList) + if err != nil { + return err + } + for _, event := range eventList.Items { + if event.InvolvedObject.Name == "service-test32" && + event.InvolvedObject.Kind == "Service" && + event.Type == v1.EventTypeWarning && + strings.Contains(event.Message, helper.ErrProjectIsImmutable.Error()) { + return nil + } + } + return helper.ErrNoEventFound + }, time.Second*5, time.Millisecond*500).Should(Succeed()) + + By("checking that projectID is still the ID from the creation") + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test32", Namespace: "default"}, &lb) + if err != nil { + return err + } + if lb.Spec.Infrastructure.ProjectID != nil && + *lb.Spec.Infrastructure.ProjectID == "otherProject" { + return nil + } + return fmt.Errorf("projectID is not correctly set in infrastructure %v", lb.Spec.Infrastructure.ProjectID) + }, 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 14ccafcd..d217615c 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go @@ -459,7 +459,7 @@ func (r *Reconciler) assignOrCreateFIP( return nil } -func (r *Reconciler) reconcilePort( +func (r *Reconciler) reconcilePort( //nolint: gocyclo // TODO reduce complexity in future ctx context.Context, req ctrl.Request, lb *yawolv1beta1.LoadBalancer, @@ -1191,7 +1191,6 @@ func (r *Reconciler) deletePorts( var requeue bool - //nolint: dupl // we can't extract this code because of generics if lb.Status.PortID != nil { port, err := openstackhelper.GetPortByID(ctx, portClient, *lb.Status.PortID) if err != nil { @@ -1217,6 +1216,7 @@ func (r *Reconciler) deletePorts( } // clean up orphan ports + //nolint: dupl // we can't extract this code because of generics if lb.Status.PortName != nil { portName := *lb.Status.PortName var portList []ports.Port @@ -1306,6 +1306,7 @@ func (r *Reconciler) deleteSecGroups( } // clean up orphan secgroups + //nolint: dupl // we can't extract this code because of generics if lb.Status.SecurityGroupName != nil { secGroupName := *lb.Status.SecurityGroupName var secGroupList []groups.SecGroup diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go index d19642fe..bf70c985 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go @@ -103,12 +103,14 @@ var _ = Describe("loadbalancer controller", func() { g.Expect(act.Status.PortID).ToNot(BeNil()) g.Expect(act.Status.PortName).ToNot(BeNil()) + g.Expect(act.Status.PortIP).ToNot(BeNil()) + g.Expect(*act.Status.PortIP).ToNot(Equal("")) + g.Expect(act.Status.FloatingID).To(BeNil()) g.Expect(act.Status.FloatingName).To(BeNil()) g.Expect(act.Status.ExternalIP).ToNot(BeNil()) g.Expect(*act.Status.ExternalIP).ToNot(Equal("")) - return nil }) }) @@ -118,11 +120,17 @@ var _ = Describe("loadbalancer controller", func() { It("should swap to an internal lb", func() { By("checking that the lb gets created with an public ip") hopefully(lbNN, func(g Gomega, act LB) error { + g.Expect(act.Status.PortID).ToNot(BeNil()) + g.Expect(act.Status.PortName).ToNot(BeNil()) + + g.Expect(act.Status.PortIP).ToNot(BeNil()) + g.Expect(*act.Status.PortIP).ToNot(Equal("")) + g.Expect(act.Status.FloatingID).ToNot(BeNil()) g.Expect(act.Status.FloatingName).ToNot(BeNil()) g.Expect(act.Status.ExternalIP).ToNot(BeNil()) - + g.Expect(*act.Status.ExternalIP).ToNot(Equal("")) return nil }) diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index 46592bf7..2babd4cc 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go @@ -341,7 +341,7 @@ func (r *LoadBalancerMachineReconciler) reconcileRoleBinding( return nil } -func (r *LoadBalancerMachineReconciler) reconcilePort( +func (r *LoadBalancerMachineReconciler) reconcilePort( //nolint: gocyclo // TODO reduce complexity in future ctx context.Context, osClient os.Client, req ctrl.Request, diff --git a/internal/helper/errors.go b/internal/helper/errors.go index dfef32a4..b98f5fb5 100644 --- a/internal/helper/errors.go +++ b/internal/helper/errors.go @@ -65,5 +65,5 @@ var ( ErrCouldNotParseSourceRange = errors.New("could not parse LoadBalancerSourceRange") ErrListingChildLBMs = errors.New("unable to list child loadbalancerMachines") ErrUnsupportedProtocol = errors.New("unsupported protocol used (TCP and UDP is supported)") - ErrProjectIsImmutable = errors.New("project id is immutable, cant be changed after inital creation") + ErrProjectIsImmutable = errors.New("project id is immutable, cant be changed after initial creation") ) diff --git a/internal/helper/openstack/client.go b/internal/helper/openstack/client.go index bd3a1278..11f9700d 100644 --- a/internal/helper/openstack/client.go +++ b/internal/helper/openstack/client.go @@ -3,6 +3,7 @@ package openstack import ( "context" "fmt" + yawolv1beta1 "github.com/stackitcloud/yawol/api/v1beta1" "github.com/stackitcloud/yawol/internal/openstack" diff --git a/internal/openstack/client.go b/internal/openstack/client.go index 7959d395..abbff7da 100644 --- a/internal/openstack/client.go +++ b/internal/openstack/client.go @@ -43,7 +43,12 @@ type OSClient struct { // username="itmyuser" // password="suupersecret" // region="eu01" -func (r *OSClient) Configure(iniBytes []byte, overwrite OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error { +func (r *OSClient) Configure( + iniBytes []byte, + overwrite OSClientOverwrite, + timeout time.Duration, + promCounter *prometheus.CounterVec, +) error { r.ini = iniBytes r.timeout = timeout r.promCounter = promCounter @@ -163,7 +168,12 @@ func (r *OSClient) KeyPairClient(ctx context.Context) (KeyPairClient, error) { return client.Configure(r.computeV2, r.timeout, r.promCounter), nil } -func createNetworkV2FromIni(ctx context.Context, iniData []byte, overwrite OSClientOverwrite, timeout time.Duration) (*gophercloud.ServiceClient, error) { +func createNetworkV2FromIni( + ctx context.Context, + iniData []byte, + overwrite OSClientOverwrite, + timeout time.Duration, +) (*gophercloud.ServiceClient, error) { provider, opts, err := getProvider(ctx, iniData, overwrite, timeout) if err != nil { return nil, err @@ -177,7 +187,12 @@ func createNetworkV2FromIni(ctx context.Context, iniData []byte, overwrite OSCli return client, nil } -func createComputeV2FromIni(ctx context.Context, iniData []byte, overwrite OSClientOverwrite, timeout time.Duration) (*gophercloud.ServiceClient, error) { +func createComputeV2FromIni( + ctx context.Context, + iniData []byte, + overwrite OSClientOverwrite, + timeout time.Duration, +) (*gophercloud.ServiceClient, error) { provider, opts, err := getProvider(ctx, iniData, overwrite, timeout) if err != nil { return nil, err diff --git a/internal/openstack/testing/client.go b/internal/openstack/testing/client.go index b93cc3cb..83d487dd 100644 --- a/internal/openstack/testing/client.go +++ b/internal/openstack/testing/client.go @@ -39,7 +39,7 @@ type MockClient struct { ServerGroupClientObj openstack.ServerGroupClient } -func (r *MockClient) Configure(ini []byte, overwrite openstack.OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error { +func (r *MockClient) Configure(_ []byte, _ openstack.OSClientOverwrite, _ time.Duration, _ *prometheus.CounterVec) error { r.StoredValues = make(map[string]interface{}) return nil } From 82dee3207e9b411f9f72484c62990097f1ada035 Mon Sep 17 00:00:00 2001 From: Maximilian Geberl Date: Mon, 2 Jan 2023 16:40:04 +0100 Subject: [PATCH 09/11] make flavor and image in infrastructure required --- api/v1beta1/loadbalancer_types.go | 14 ++-- api/v1beta1/zz_generated.deepcopy.go | 12 +--- ...ol.stackit.cloud_loadbalancermachines.yaml | 60 +++++++---------- .../yawol.stackit.cloud_loadbalancers.yaml | 66 +++++++----------- .../yawol.stackit.cloud_loadbalancersets.yaml | 67 ++++++++----------- .../targetcontroller/service_controller.go | 8 +-- .../loadbalancer_controller_test.go | 6 +- .../loadbalancerset_status_controller_test.go | 4 +- .../loadbalancermachine_controller.go | 4 +- .../loadbalancermachine_controller_test.go | 8 +-- .../loadbalancerset_controller_test.go | 4 +- 11 files changed, 104 insertions(+), 149 deletions(-) diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index 87fb5116..d5724611 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/api/v1beta1/loadbalancer_types.go @@ -189,12 +189,10 @@ type LoadBalancerInfrastructure struct { // AdditionalNetworks defines additional networks that will be added to the LoadBalancerMachines. // +optional AdditionalNetworks []LoadBalancerAdditionalNetwork `json:"additionalNetworks"` - // Flavor defines openstack flavor for the LoadBalancer. Uses a default if not defined. - // +optional - Flavor *OpenstackFlavorRef `json:"flavor"` - // Image defines openstack image for the LoadBalancer. Uses a default if not defined. - // +optional - Image *OpenstackImageRef `json:"image"` + // Flavor defines openstack flavor for the LoadBalancer. + Flavor OpenstackFlavorRef `json:"flavor"` + // Image defines openstack image for the LoadBalancer. + Image OpenstackImageRef `json:"image"` // AvailabilityZone defines the openstack availability zone for the LoadBalancer. // +optional AvailabilityZone string `json:"availabilityZone"` @@ -226,10 +224,12 @@ type OpenstackImageRef struct { // ImageID is the image ID used for requesting virtual machines. // +optional ImageID *string `json:"imageID,omitempty"` + // NOT IMPLEMENTED ONLY ImageID is supported. // ImageName is the name of the image used for requesting virtual machines. // ImageName is only used if ImageID is not defined. // +optional ImageName *string `json:"imageName,omitempty"` + // NOT IMPLEMENTED ONLY ImageID is supported. // ImageSearch is a search string to find the image used for requesting virtual machines. // Search will be performed in metadata of the images. // ImageSearch is only used if ImageName and ImageID are not defined. @@ -253,10 +253,12 @@ type OpenstackFlavorRef struct { // FlavorID is the flavor ID used for requesting virtual machines. // +optional FlavorID *string `json:"flavorID,omitempty"` + // NOT IMPLEMENTED ONLY FlavorID is supported. // FlavorName is the name of the flavor used for requesting virtual machines. // FlavorName is only used if FlavorID is not defined. // +optional FlavorName *string `json:"flavorName,omitempty"` + // NOT IMPLEMENTED ONLY FlavorID is supported. // FlavorSearch is a search string to find the flavor used for requesting virtual machines. // Search will be performed in metadata of the flavors. // FlavorSearch is only used if FlavorName and FlavorID are not defined. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 117ae066..007839cd 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -122,16 +122,8 @@ func (in *LoadBalancerInfrastructure) DeepCopyInto(out *LoadBalancerInfrastructu *out = make([]LoadBalancerAdditionalNetwork, len(*in)) copy(*out, *in) } - if in.Flavor != nil { - in, out := &in.Flavor, &out.Flavor - *out = new(OpenstackFlavorRef) - (*in).DeepCopyInto(*out) - } - if in.Image != nil { - in, out := &in.Image, &out.Image - *out = new(OpenstackImageRef) - (*in).DeepCopyInto(*out) - } + in.Flavor.DeepCopyInto(&out.Flavor) + in.Image.DeepCopyInto(&out.Image) out.AuthSecretRef = in.AuthSecretRef if in.ProjectID != nil { in, out := &in.ProjectID, &out.ProjectID diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml index b9d93197..6b1c51d7 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml @@ -106,38 +106,31 @@ spec: type: object flavor: description: Flavor defines openstack flavor for the LoadBalancer. - Uses a default if not defined. properties: flavor_id: - description: 'Deprecated: used for migration FlavorID is the - flavor ID used for requesting virtual machines.' + description: 'Deprecated: use flavorID instead.' type: string flavor_name: - description: 'Deprecated: used for migration FlavorName is - the name of the flavor used for requesting virtual machines. - FlavorName is only used if FlavorID is not defined.' + description: 'Deprecated: use flavorName instead.' type: string flavor_search: - description: 'Deprecated: used for migration FlavorSearch - is a search string to find the flavor used for requesting - virtual machines. Search will be performed in metadata of - the flavors. FlavorSearch is only used if FlavorName and - FlavorID are not defined.' + description: 'Deprecated: use flavorSearch instead.' type: string flavorID: description: FlavorID is the flavor ID used for requesting virtual machines. type: string flavorName: - description: FlavorName is the name of the flavor used for - requesting virtual machines. FlavorName is only used if - FlavorID is not defined. + description: NOT IMPLEMENTED ONLY FlavorID is supported. FlavorName + is the name of the flavor used for requesting virtual machines. + FlavorName is only used if FlavorID is not defined. type: string flavorSearch: - description: FlavorSearch is a search string to find the flavor - used for requesting virtual machines. Search will be performed - in metadata of the flavors. FlavorSearch is only used if - FlavorName and FlavorID are not defined. + description: NOT IMPLEMENTED ONLY FlavorID is supported. FlavorSearch + is a search string to find the flavor used for requesting + virtual machines. Search will be performed in metadata of + the flavors. FlavorSearch is only used if FlavorName and + FlavorID are not defined. type: string type: object floatingNetID: @@ -146,38 +139,31 @@ spec: type: string image: description: Image defines openstack image for the LoadBalancer. - Uses a default if not defined. properties: image_id: - description: 'Deprecated: used for migration ImageID is the - image ID used for requesting virtual machines.' + description: 'Deprecated: use imageID instead.' type: string image_name: - description: 'Deprecated: used for migration ImageName is - the name of the image used for requesting virtual machines. - ImageName is only used if ImageID is not defined.' + description: 'Deprecated: use imageName instead.' type: string image_search: - description: 'Deprecated: used for migration ImageSearch is - a search string to find the image used for requesting virtual - machines. Search will be performed in metadata of the images. - ImageSearch is only used if ImageName and ImageID are not - defined.' + description: 'Deprecated: use imageSearch instead.' type: string imageID: description: ImageID is the image ID used for requesting virtual machines. type: string imageName: - description: ImageName is the name of the image used for requesting - virtual machines. ImageName is only used if ImageID is not - defined. + description: NOT IMPLEMENTED ONLY ImageID is supported. ImageName + is the name of the image used for requesting virtual machines. + ImageName is only used if ImageID is not defined. type: string imageSearch: - description: ImageSearch is a search string to find the image - used for requesting virtual machines. Search will be performed - in metadata of the images. ImageSearch is only used if ImageName - and ImageID are not defined. + description: NOT IMPLEMENTED ONLY ImageID is supported. ImageSearch + is a search string to find the image used for requesting + virtual machines. Search will be performed in metadata of + the images. ImageSearch is only used if ImageName and ImageID + are not defined. type: string type: object networkID: @@ -191,6 +177,8 @@ spec: type: string required: - authSecretRef + - flavor + - image type: object loadBalancerRef: description: LoadBalancerRef defines a reference to the LoadBalancer diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml index 3ac06307..49d59d8b 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml @@ -137,38 +137,31 @@ spec: type: object flavor: description: Flavor defines openstack flavor for the LoadBalancer. - Uses a default if not defined. properties: flavor_id: - description: 'Deprecated: used for migration FlavorID is the - flavor ID used for requesting virtual machines.' + description: 'Deprecated: use flavorID instead.' type: string flavor_name: - description: 'Deprecated: used for migration FlavorName is - the name of the flavor used for requesting virtual machines. - FlavorName is only used if FlavorID is not defined.' + description: 'Deprecated: use flavorName instead.' type: string flavor_search: - description: 'Deprecated: used for migration FlavorSearch - is a search string to find the flavor used for requesting - virtual machines. Search will be performed in metadata of - the flavors. FlavorSearch is only used if FlavorName and - FlavorID are not defined.' + description: 'Deprecated: use flavorSearch instead.' type: string flavorID: description: FlavorID is the flavor ID used for requesting virtual machines. type: string flavorName: - description: FlavorName is the name of the flavor used for - requesting virtual machines. FlavorName is only used if - FlavorID is not defined. + description: NOT IMPLEMENTED ONLY FlavorID is supported. FlavorName + is the name of the flavor used for requesting virtual machines. + FlavorName is only used if FlavorID is not defined. type: string flavorSearch: - description: FlavorSearch is a search string to find the flavor - used for requesting virtual machines. Search will be performed - in metadata of the flavors. FlavorSearch is only used if - FlavorName and FlavorID are not defined. + description: NOT IMPLEMENTED ONLY FlavorID is supported. FlavorSearch + is a search string to find the flavor used for requesting + virtual machines. Search will be performed in metadata of + the flavors. FlavorSearch is only used if FlavorName and + FlavorID are not defined. type: string type: object floatingNetID: @@ -177,38 +170,31 @@ spec: type: string image: description: Image defines openstack image for the LoadBalancer. - Uses a default if not defined. properties: image_id: - description: 'Deprecated: used for migration ImageID is the - image ID used for requesting virtual machines.' + description: 'Deprecated: use imageID instead.' type: string image_name: - description: 'Deprecated: used for migration ImageName is - the name of the image used for requesting virtual machines. - ImageName is only used if ImageID is not defined.' + description: 'Deprecated: use imageName instead.' type: string image_search: - description: 'Deprecated: used for migration ImageSearch is - a search string to find the image used for requesting virtual - machines. Search will be performed in metadata of the images. - ImageSearch is only used if ImageName and ImageID are not - defined.' + description: 'Deprecated: use imageSearch instead.' type: string imageID: description: ImageID is the image ID used for requesting virtual machines. type: string imageName: - description: ImageName is the name of the image used for requesting - virtual machines. ImageName is only used if ImageID is not - defined. + description: NOT IMPLEMENTED ONLY ImageID is supported. ImageName + is the name of the image used for requesting virtual machines. + ImageName is only used if ImageID is not defined. type: string imageSearch: - description: ImageSearch is a search string to find the image - used for requesting virtual machines. Search will be performed - in metadata of the images. ImageSearch is only used if ImageName - and ImageID are not defined. + description: NOT IMPLEMENTED ONLY ImageID is supported. ImageSearch + is a search string to find the image used for requesting + virtual machines. Search will be performed in metadata of + the images. ImageSearch is only used if ImageName and ImageID + are not defined. type: string type: object networkID: @@ -222,6 +208,8 @@ spec: type: string required: - authSecretRef + - flavor + - image type: object options: description: Options for additional LoadBalancer settings @@ -434,12 +422,10 @@ spec: deployment type: integer security_group_id: - description: 'Deprecated: used for migration SecurityGroupID is the - current security group ID mapped to the port' + description: 'Deprecated: use securityGroupID instead.' type: string security_group_name: - description: 'Deprecated: used for migration SecurityGroupName is - the current security group name mapped to the port' + description: 'Deprecated: use securityGroupName instead.' type: string securityGroupID: description: SecurityGroupID is the current security group ID mapped diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml index 3aa25da3..452e99ce 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml @@ -168,40 +168,33 @@ spec: type: object flavor: description: Flavor defines openstack flavor for the LoadBalancer. - Uses a default if not defined. properties: flavor_id: - description: 'Deprecated: used for migration FlavorID - is the flavor ID used for requesting virtual machines.' + description: 'Deprecated: use flavorID instead.' type: string flavor_name: - description: 'Deprecated: used for migration FlavorName - is the name of the flavor used for requesting virtual - machines. FlavorName is only used if FlavorID is - not defined.' + description: 'Deprecated: use flavorName instead.' type: string flavor_search: - description: 'Deprecated: used for migration FlavorSearch - is a search string to find the flavor used for requesting - virtual machines. Search will be performed in metadata - of the flavors. FlavorSearch is only used if FlavorName - and FlavorID are not defined.' + description: 'Deprecated: use flavorSearch instead.' type: string flavorID: description: FlavorID is the flavor ID used for requesting virtual machines. type: string flavorName: - description: FlavorName is the name of the flavor - used for requesting virtual machines. FlavorName - is only used if FlavorID is not defined. + description: NOT IMPLEMENTED ONLY FlavorID is supported. + FlavorName is the name of the flavor used for requesting + virtual machines. FlavorName is only used if FlavorID + is not defined. type: string flavorSearch: - description: FlavorSearch is a search string to find - the flavor used for requesting virtual machines. - Search will be performed in metadata of the flavors. - FlavorSearch is only used if FlavorName and FlavorID - are not defined. + description: NOT IMPLEMENTED ONLY FlavorID is supported. + FlavorSearch is a search string to find the flavor + used for requesting virtual machines. Search will + be performed in metadata of the flavors. FlavorSearch + is only used if FlavorName and FlavorID are not + defined. type: string type: object floatingNetID: @@ -210,40 +203,32 @@ spec: type: string image: description: Image defines openstack image for the LoadBalancer. - Uses a default if not defined. properties: image_id: - description: 'Deprecated: used for migration ImageID - is the image ID used for requesting virtual machines.' + description: 'Deprecated: use imageID instead.' type: string image_name: - description: 'Deprecated: used for migration ImageName - is the name of the image used for requesting virtual - machines. ImageName is only used if ImageID is not - defined.' + description: 'Deprecated: use imageName instead.' type: string image_search: - description: 'Deprecated: used for migration ImageSearch - is a search string to find the image used for requesting - virtual machines. Search will be performed in metadata - of the images. ImageSearch is only used if ImageName - and ImageID are not defined.' + description: 'Deprecated: use imageSearch instead.' type: string imageID: description: ImageID is the image ID used for requesting virtual machines. type: string imageName: - description: ImageName is the name of the image used - for requesting virtual machines. ImageName is only - used if ImageID is not defined. + description: NOT IMPLEMENTED ONLY ImageID is supported. + ImageName is the name of the image used for requesting + virtual machines. ImageName is only used if ImageID + is not defined. type: string imageSearch: - description: ImageSearch is a search string to find - the image used for requesting virtual machines. - Search will be performed in metadata of the images. - ImageSearch is only used if ImageName and ImageID - are not defined. + description: NOT IMPLEMENTED ONLY ImageID is supported. + ImageSearch is a search string to find the image + used for requesting virtual machines. Search will + be performed in metadata of the images. ImageSearch + is only used if ImageName and ImageID are not defined. type: string type: object networkID: @@ -258,6 +243,8 @@ spec: type: string required: - authSecretRef + - flavor + - image type: object loadBalancerRef: description: LoadBalancerRef defines a reference to the LoadBalancer diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index 5f122345..fddef3b2 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go @@ -203,8 +203,8 @@ func (r *ServiceReconciler) createLoadBalancer( Infrastructure: yawolv1beta1.LoadBalancerInfrastructure{ DefaultNetwork: getDefaultNetwork(svc, infraConfig), AdditionalNetworks: getAdditionalNetworks(svc, infraConfig), - Flavor: infraConfig.FlavorRef, - Image: infraConfig.ImageRef, + Flavor: *infraConfig.FlavorRef, + Image: *infraConfig.ImageRef, AvailabilityZone: *infraConfig.AvailabilityZone, ProjectID: getProjectID(svc), AuthSecretRef: coreV1.SecretReference{ @@ -229,8 +229,8 @@ func (r *ServiceReconciler) reconcileInfrastructure( newInfra := yawolv1beta1.LoadBalancerInfrastructure{ DefaultNetwork: getDefaultNetwork(svc, infraConfig), AdditionalNetworks: getAdditionalNetworks(svc, infraConfig), - Flavor: infraConfig.FlavorRef, - Image: infraConfig.ImageRef, + Flavor: *infraConfig.FlavorRef, + Image: *infraConfig.ImageRef, AvailabilityZone: *infraConfig.AvailabilityZone, ProjectID: getProjectID(svc), AuthSecretRef: coreV1.SecretReference{ diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go index bf70c985..049fe241 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go @@ -219,7 +219,7 @@ var _ = Describe("loadbalancer controller", func() { By("changing flavorid") updateLB(lbNN, func(act *LB) { - act.Spec.Infrastructure.Flavor = &yawolv1beta1.OpenstackFlavorRef{ + act.Spec.Infrastructure.Flavor = yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("somenewid"), } }) @@ -692,10 +692,10 @@ func getMockLB(lbNN types.NamespacedName) *LB { FloatingNetID: pointer.String("floatingnet-id"), NetworkID: "network-id", }, - Flavor: &yawolv1beta1.OpenstackFlavorRef{ + Flavor: yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("flavor-id"), }, - Image: &yawolv1beta1.OpenstackImageRef{ + Image: yawolv1beta1.OpenstackImageRef{ ImageID: pointer.String("image-id"), }, AuthSecretRef: v1.SecretReference{ diff --git a/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go b/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go index 954db776..0ee21bf0 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller_test.go @@ -71,10 +71,10 @@ var _ = Describe("LB Status update", func() { FloatingNetID: pointer.String("floatingnetid"), NetworkID: "networkid", }, - Flavor: &yawolv1beta1.OpenstackFlavorRef{ + Flavor: yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("mycool-openstack-flavor-id"), }, - Image: &yawolv1beta1.OpenstackImageRef{ + Image: yawolv1beta1.OpenstackImageRef{ ImageID: pointer.String("mycool-openstack-image-id"), }, AuthSecretRef: v1.SecretReference{ diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index 2babd4cc..3d3ba08b 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go @@ -653,13 +653,13 @@ func (r *LoadBalancerMachineReconciler) createServer( var err error var imageID string - imageID, err = helper.GetImageID(*loadBalancerMachine.Spec.Infrastructure.Image) + imageID, err = helper.GetImageID(loadBalancerMachine.Spec.Infrastructure.Image) if err != nil { return nil, err } var flavorID string - flavorID, err = helper.GetFlavorID(*loadBalancerMachine.Spec.Infrastructure.Flavor) + flavorID, err = helper.GetFlavorID(loadBalancerMachine.Spec.Infrastructure.Flavor) if err != nil { return nil, err } diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go index 029205db..efc054be 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller_test.go @@ -418,10 +418,10 @@ func getMockLB() *LB { FloatingNetID: pointer.String("floatingnetid"), NetworkID: "networkid", }, - Flavor: &yawolv1beta1.OpenstackFlavorRef{ + Flavor: yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("flavor-id"), }, - Image: &yawolv1beta1.OpenstackImageRef{ + Image: yawolv1beta1.OpenstackImageRef{ ImageID: pointer.String("image-id"), }, AuthSecretRef: v1.SecretReference{ @@ -451,10 +451,10 @@ func getMockLBM(lb *LB) *LBM { FloatingNetID: pointer.String("floatingnetid"), NetworkID: "networkid", }, - Flavor: &yawolv1beta1.OpenstackFlavorRef{ + Flavor: yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String("flavor-id"), }, - Image: &yawolv1beta1.OpenstackImageRef{ + Image: yawolv1beta1.OpenstackImageRef{ ImageID: pointer.String("image-id"), }, AuthSecretRef: v1.SecretReference{ diff --git a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go index 28c26936..6e5473c1 100644 --- a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go +++ b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go @@ -63,10 +63,10 @@ var _ = Describe("LoadBalancerSet controller", func() { FloatingNetID: pointer.String(FloatingNetID), NetworkID: NetworkID, }, - Flavor: &yawolv1beta1.OpenstackFlavorRef{ + Flavor: yawolv1beta1.OpenstackFlavorRef{ FlavorID: pointer.String(FlavorID), }, - Image: &yawolv1beta1.OpenstackImageRef{ + Image: yawolv1beta1.OpenstackImageRef{ ImageID: pointer.String(ImageID), }, AuthSecretRef: v1.SecretReference{ From 813c6bbcadc46dd19d8ab70ae3ad0fa3a851d31f Mon Sep 17 00:00:00 2001 From: Maximilian Geberl Date: Mon, 2 Jan 2023 17:27:25 +0100 Subject: [PATCH 10/11] add suggestions from code review --- .../targetcontroller/service_controller.go | 20 ++++++------- .../loadbalancer/loadbalancer_controller.go | 3 +- .../loadbalancermachine_controller.go | 28 ++++++++++--------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index fddef3b2..3564eded 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go @@ -618,14 +618,14 @@ func getAdditionalNetworks( // use struct to make sure there are no duplicates networkIDs := make(map[string]struct{}) - if _, ok := svc.Annotations[yawolv1beta1.ServiceAdditionalNetworks]; ok { - for _, networkID := range strings.Split(svc.Annotations[yawolv1beta1.ServiceAdditionalNetworks], ",") { + if networks, ok := svc.Annotations[yawolv1beta1.ServiceAdditionalNetworks]; ok { + for _, networkID := range strings.Split(networks, ",") { networkIDs[networkID] = struct{}{} } } - if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { - if svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID] != *infraConfig.NetworkID { + if networkID, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { + if networkID != *infraConfig.NetworkID { if skip, _ := strconv.ParseBool(svc.Annotations[yawolv1beta1.ServiceSkipCloudControllerDefaultNetworkID]); !skip { networkIDs[*infraConfig.NetworkID] = struct{}{} } @@ -649,20 +649,18 @@ func getDefaultNetwork( NetworkID: *infraConfig.NetworkID, } - if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { - defaultNetwork.NetworkID = svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID] + if networkID, ok := svc.Annotations[yawolv1beta1.ServiceDefaultNetworkID]; ok { + defaultNetwork.NetworkID = networkID } - if _, ok := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID]; ok { - floatingNetID := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID] - defaultNetwork.FloatingNetID = &floatingNetID + if floatingID, ok := svc.Annotations[yawolv1beta1.ServiceFloatingNetworkID]; ok { + defaultNetwork.FloatingNetID = &floatingID } return defaultNetwork } func getProjectID(svc *coreV1.Service) *string { - if _, ok := svc.Annotations[yawolv1beta1.ServiceDefaultProjectID]; ok { - projectID := svc.Annotations[yawolv1beta1.ServiceDefaultProjectID] + if projectID, ok := svc.Annotations[yawolv1beta1.ServiceDefaultProjectID]; ok { return &projectID } return nil diff --git a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go index d217615c..a77138b3 100644 --- a/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go +++ b/controllers/yawol-controller/loadbalancer/loadbalancer_controller.go @@ -520,7 +520,8 @@ func (r *Reconciler) reconcilePort( //nolint: gocyclo // TODO reduce complexity ctx, portClient, *lb.Status.PortName, - networkID) + networkID, + ) if err != nil { r.Log.Info("unexpected error occurred claiming a port", "lb", req.NamespacedName) return false, kubernetes.SendErrorAsEvent(r.RecorderLB, err, lb) diff --git a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go index 3d3ba08b..f607a30a 100644 --- a/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go +++ b/controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go @@ -187,7 +187,7 @@ func (r *LoadBalancerMachineReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - if err := helper.PatchLBMStatus(ctx, r.Status(), loadBalancerMachine, yawolv1beta1.LoadBalancerMachineStatus{ + if err := helper.PatchLBMStatus(ctx, r.Client.Status(), loadBalancerMachine, yawolv1beta1.LoadBalancerMachineStatus{ LastOpenstackReconcile: &metav1.Time{Time: time.Now()}, }); err != nil { return ctrl.Result{}, err @@ -360,11 +360,13 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( //nolint: gocyclo // TODO ); err != nil { return err } - if lbm.Status.PortID != nil && lbm.Status.DefaultPortID != nil && //nolint: staticcheck // needed to be backwards compatible - *lbm.Status.PortID == *lbm.Status.DefaultPortID { //nolint: staticcheck // needed to be backwards compatible - if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "portID"); err != nil { - return err - } + } + + // TODO cleanup after removing deprecated fields + if lbm.Status.PortID != nil && lbm.Status.DefaultPortID != nil && //nolint: staticcheck // needed to be backwards compatible + *lbm.Status.PortID == *lbm.Status.DefaultPortID { //nolint: staticcheck // needed to be backwards compatible + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "portID"); err != nil { + return err } } @@ -479,7 +481,7 @@ func (r *LoadBalancerMachineReconciler) reconcilePort( //nolint: gocyclo // TODO // Patch defaultPortIP to status if len(port.FixedIPs) >= 1 && (lbm.Status.DefaultPortIP == nil || *lbm.Status.DefaultPortIP != port.FixedIPs[0].IPAddress) { - if err := helper.PatchLBMStatus(ctx, r.Status(), lbm, yawolv1beta1.LoadBalancerMachineStatus{ + if err := helper.PatchLBMStatus(ctx, r.Client.Status(), lbm, yawolv1beta1.LoadBalancerMachineStatus{ DefaultPortIP: &port.FixedIPs[0].IPAddress, }); err != nil { return err @@ -794,7 +796,7 @@ func (r *LoadBalancerMachineReconciler) deleteServer( } } - if err := helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "serverID"); err != nil { + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "serverID"); err != nil { return err } @@ -818,7 +820,7 @@ func (r *LoadBalancerMachineReconciler) deletePort( if err = openstackhelper.DeletePort(ctx, portClient, *lbm.Status.DefaultPortID); err != nil { switch err.(type) { case gophercloud.ErrDefault404: - _ = helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "defaultPortIP") + _ = helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "defaultPortIP") if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "defaultPortID"); err != nil { return false, err } @@ -867,7 +869,7 @@ func (r *LoadBalancerMachineReconciler) deletePort( } requeue = true } else { - if err := helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "defaultPortName"); err != nil { + if err := helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "defaultPortName"); err != nil { return false, err } } @@ -890,7 +892,7 @@ func (r *LoadBalancerMachineReconciler) deleteSA( return err } - return helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "serviceAccountName") + return helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "serviceAccountName") } func (r *LoadBalancerMachineReconciler) deleteRoleBinding( @@ -907,7 +909,7 @@ func (r *LoadBalancerMachineReconciler) deleteRoleBinding( return err } - return helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "roleBindingName") + return helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "roleBindingName") } func (r *LoadBalancerMachineReconciler) deleteRole( @@ -924,7 +926,7 @@ func (r *LoadBalancerMachineReconciler) deleteRole( return err } - return helper.RemoveFromLBMStatus(ctx, r.Status(), lbm, "roleName") + return helper.RemoveFromLBMStatus(ctx, r.Client.Status(), lbm, "roleName") } func (r *LoadBalancerMachineReconciler) waitForServerStatus( From e3df5cbc7ced26e14d30cfbfdf0c6292f6a97a48 Mon Sep 17 00:00:00 2001 From: Maximilian Geberl Date: Thu, 5 Jan 2023 15:55:54 +0100 Subject: [PATCH 11/11] mark lbm.spec.serverGroupID as optional, to be sure this works while update --- api/v1beta1/loadbalancermachine_types.go | 1 + .../crds/yawol.stackit.cloud_loadbalancermachines.yaml | 1 - .../crds/yawol.stackit.cloud_loadbalancersets.yaml | 1 - go.mod | 2 +- go.sum | 4 ++-- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/api/v1beta1/loadbalancermachine_types.go b/api/v1beta1/loadbalancermachine_types.go index 3690913f..18eccda8 100644 --- a/api/v1beta1/loadbalancermachine_types.go +++ b/api/v1beta1/loadbalancermachine_types.go @@ -38,6 +38,7 @@ type LoadBalancerMachineSpec struct { // PortID defines the openstack ID of the port attached to the FloatingIP. PortID string `json:"portID"` // ServerGroupID defines the openstack ID of the openstack server group. + // +optional ServerGroupID string `json:"serverGroupID"` // LoadBalancerRef defines a reference to the LoadBalancer Object. LoadBalancerRef LoadBalancerRef `json:"loadBalancerRef"` diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml index 6b1c51d7..55becdb6 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancermachines.yaml @@ -208,7 +208,6 @@ spec: - infrastructure - loadBalancerRef - portID - - serverGroupID type: object status: description: LoadBalancerMachineStatus defines the observed state of LoadBalancerMachine. diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml index 452e99ce..e470ae02 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml +++ b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml @@ -274,7 +274,6 @@ spec: - infrastructure - loadBalancerRef - portID - - serverGroupID type: object required: - labels diff --git a/go.mod b/go.mod index f0216eca..8acc2645 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/onsi/ginkgo/v2 v2.6.1 github.com/onsi/gomega v1.24.2 github.com/prometheus/client_golang v1.14.0 - github.com/shirou/gopsutil/v3 v3.22.11 + github.com/shirou/gopsutil/v3 v3.22.12 google.golang.org/grpc v1.51.0 google.golang.org/protobuf v1.28.1 gopkg.in/ini.v1 v1.67.0 diff --git a/go.sum b/go.sum index 72110635..a2f562b6 100644 --- a/go.sum +++ b/go.sum @@ -398,8 +398,8 @@ github.com/prometheus/procfs v0.8.0/go.mod h1:z7EfXMXOkbkqb9IINtpCn86r/to3BnA0ua github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/shirou/gopsutil/v3 v3.22.11 h1:kxsPKS+Eeo+VnEQ2XCaGJepeP6KY53QoRTETx3+1ndM= -github.com/shirou/gopsutil/v3 v3.22.11/go.mod h1:xl0EeL4vXJ+hQMAGN8B9VFpxukEMA0XdevQOe5MZ1oY= +github.com/shirou/gopsutil/v3 v3.22.12 h1:oG0ns6poeUSxf78JtOsfygNWuEHYYz8hnnNg7P04TJs= +github.com/shirou/gopsutil/v3 v3.22.12/go.mod h1:Xd7P1kwZcp5VW52+9XsirIKd/BROzbb2wdX3Kqlz9uI= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=