Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Loadbalancer doesn't find and attach the ip's address to the backendAddressPools #4368

Open
jsturtevant opened this issue Oct 22, 2024 · 10 comments
Assignees
Labels
bug 🪲 Something isn't working waiting-on-user-response Waiting on more information from the original user before progressing.
Milestone

Comments

@jsturtevant
Copy link

jsturtevant commented Oct 22, 2024

Version of Azure Service Operator

Describe the bug
A backend for the loadbalancer object is created but the vnet and ip configs are not wired up properly:

apiVersion: network.azure.com/v1api20201101
kind: LoadBalancer
metadata:
  creationTimestamp: null
  name: capz-public-lb
  annotations:
    serviceoperator.azure.com/credential-from: capz-conf-a6xnyi-aso-secret
    serviceoperator.azure.com/operator-namespace: capz-system
spec:
  owner: 
    name: capz-conf-a6xnyi
  azureName: capz-conf-public-lb
  backendAddressPools:
  - loadBalancerBackendAddresses:
    - name: capz-conf-a6xnyi-control-plane-jhq22
      virtualNetwork: 
        reference:
          armId: /subscriptions/subid/resourceGroups/capz-conf-a6xnyi/providers/Microsoft.Network/virtualNetworks/capz-conf-a6xnyi-vnet
      subnet:
        reference:
          armId: /subscriptions/subid/resourceGroups/capz-conf-a6xnyi/providers/Microsoft.Network/virtualNetworks/capz-conf-a6xnyi-vnet/subnets/control-plane-subnet
      ipAddress: "10.255.0.4"
    name: capz-conf-public-lb-backendPool
.... more config

To Reproduce
Steps to reproduce the behavior:

Expected behavior
A clear and concise description of what you expected to happen.

A backend should adopt the IP address into its backend pool

Screenshots

you can see the backend got created but no ip addresses were adopted (notice the vnet info is missing but was provided):
Image
Image

Additional context
Add any other context about the problem here.
it should look like:

Image

@jsturtevant
Copy link
Author

a quick look at the code and I found that id just takes the name but doesn't do anything else:

var bcPoolsToAdd []vnetwork.BackendAddressPool
bcPoolsToAdd = append(
bcPoolsToAdd,
vnetwork.BackendAddressPool{
Name: &backendAddressPoolName,
},
)

@nojnhuh
Copy link
Member

nojnhuh commented Oct 23, 2024

Could the issue here be that ASO would need to provide a separate CRD for "Load Balancer Backend Address Pools"? https://learn.microsoft.com/en-us/rest/api/load-balancer/load-balancer-backend-address-pools?view=rest-load-balancer-2024-03-01

I see the REST API docs for the backendAddressPools field says "Collection of backend address pools used by a load balancer" which suggests to me that those are only references to pools that are expected to be created via some other means than directly alongside the load balancer.

@theunrepentantgeek
Copy link
Member

a quick look at the code and I found that id just takes the name but doesn't do anything else:
azure-service-operator/pkg/resourcemanager/loadbalancer/client.go

That's a link to the code for ASO v1, but from your YAML sample it looks like you're using ASO v2.

In ASO v2, the LoadBalancer_Spec has the property BackendAddressPools:

// BackendAddressPools: Collection of backend address pools used by a load balancer.
BackendAddressPools []BackendAddressPool_LoadBalancer_SubResourceEmbedded `json:"backendAddressPools,omitempty"`

where BackendAddressPool_LoadBalancer_SubResourceEmbedded has this definition:

// Pool of backend IP addresses.
type BackendAddressPool_LoadBalancer_SubResourceEmbedded struct {
	// LoadBalancerBackendAddresses: An array of backend addresses.
	LoadBalancerBackendAddresses []LoadBalancerBackendAddress `json:"loadBalancerBackendAddresses,omitempty"`

	// Name: The name of the resource that is unique within the set of backend address pools used by the load balancer. This
	// name can be used to access the resource.
	Name *string `json:"name,omitempty"`
}

This is a name plus a list of backend addresses, and it looks as though the API definition here defines these as embedded subresources (thus the _SubResourceEmbedded suffix).

A backend should adopt the IP address into its backend pool

I wonder if the Azure Portal is doing some extra work here to wire things up - it wouldn't be the first time we've seen a discrepancy between the Resource Provider and the portal. I'm not particularly familiar with how this stuff works, but will make sure we discuss this at our next issue triage meeting.

@jsturtevant
Copy link
Author

I wonder if the Azure Portal is doing some extra work here to wire things up - it wouldn't be the first time we've seen a discrepancy between the Resource Provider and the portal. I'm not particularly familiar with how this stuff works, but will make sure we discuss this at our next issue triage meeting.

Oh interesting! Thanks for checking it out

@matthchr matthchr self-assigned this Oct 28, 2024
@matthchr matthchr added this to the v2.11.0 milestone Oct 28, 2024
@matthchr
Copy link
Member

matthchr commented Oct 29, 2024

I looked at this some... and it's possible that those fields are documented as writable in the networking API but aren't actually writable?

The reason I am saying that is that I looked through ~10 sample ARM templates and every single one of them only passed name for the backend address pool (and then referred to it elsewhere): https://learn.microsoft.com/en-us/azure/templates/microsoft.network/loadbalancers?pivots=deployment-language-arm-template

This makes me think that the way it actually works is that you:

  1. Create the LB.
  2. Create the Backend Address Pool
  3. Join "things" to the backend address pool after the pool has been defined.

Step 3 is accomplished by putting VMs/NICs into the backend address pool on the VM configuration, not on the LB/Backend pool configuration.

Can you try just giving a name for the backend pool there, and then using a reference to it on your VM definition (assuming that's what you actually want to join to the backend pool to serve requests?)

edit: Apologies with the confusion on this... the Azure networking API is super confusing about what resources you can "make" embedded and what ones you can't (and to a certain extent what their lifecycle is).

@matthchr
Copy link
Member

Ok I checked and the fields shouldn't be readonly, there are cases where you can set them (but other cases where you should not, such as when using ipConfiguration-based registration via VMSS like I mentioned above was common.

@matthchr
Copy link
Member

Also another thing to look at, when you try this in the portal can you look at the raw JSON view for the "works" and "doesn't work" use-cases and share them with us?

Relying on the portal UI can sometimes be fraught as it does "fancy" stuff sometimes. The first thing we should confirm is, does the JSON view match the JSON we sent.

@jsturtevant
Copy link
Author

Also another thing to look at, when you try this in the portal can you look at the raw JSON view for the "works" and "doesn't work" use-cases and share them with us?

How do I get the raw json view? using browser developer tools?

@nojnhuh
Copy link
Member

nojnhuh commented Oct 29, 2024

az or this button in the portal
Image

@theunrepentantgeek theunrepentantgeek added the waiting-on-user-response Waiting on more information from the original user before progressing. label Nov 18, 2024
@theunrepentantgeek
Copy link
Member

Have you had a chance to find the raw JSON for sharing with us?

@theunrepentantgeek theunrepentantgeek modified the milestones: v2.11.0, v2.12.0 Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working waiting-on-user-response Waiting on more information from the original user before progressing.
Projects
Development

No branches or pull requests

4 participants