properly handle multiple router external ips (including ipv6) (#958)

hack/api-reference/api.md

@@ -1373,7 +1373,7 @@ string
 </td>
 <td>
 <p>Worker is a CIDRs of a worker subnet (private) to create (used for the VMs).
-Deprecated - use <code>workers</code> instead.</p>
+Deprecated: use <code>workers</code> instead.</p>
 </td>
 </tr>
 <tr>
@@ -1574,7 +1574,19 @@ string
 </em>
 </td>
 <td>
-<p>IP is the router ip.</p>
+<p>IP is the router ip.
+Deprecated: use ExternalFixedIPs instead.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>externalFixedIP</code></br>
+<em>
+[]string
+</em>
+</td>
+<td>
+<p>ExternalFixedIPs is the list of the router&rsquo;s assigned external fixed IPs.</p>
 </td>
 </tr>
 </tbody>

pkg/apis/openstack/types_infrastructure.go

@@ -27,7 +27,7 @@ type Networks struct {
 	// Router indicates whether to use an existing router or create a new one.
 	Router *Router
 	// Worker is a CIDRs of a worker subnet (private) to create (used for the VMs).
-	// Deprecated - use `workers` instead.
+	// Deprecated: use `workers` instead.
 	Worker string
 	// Workers is a CIDRs of a worker subnet (private) to create (used for the VMs).
 	Workers string
@@ -89,7 +89,10 @@ type RouterStatus struct {
 	// ID is the Router id.
 	ID string
 	// IP is the router ip.
+	// Deprecated: use ExternalFixedIPs instead.
 	IP string
+	// ExternalFixedIPs is the list of the router's assigned external fixed IPs.
+	ExternalFixedIPs []string
 }
 
 // FloatingPoolStatus contains information about the floating pool.

pkg/apis/openstack/v1alpha1/types_infrastructure.go

@@ -30,7 +30,7 @@ type Networks struct {
 	// +optional
 	Router *Router `json:"router,omitempty"`
 	// Worker is a CIDRs of a worker subnet (private) to create (used for the VMs).
-	// Deprecated - use `workers` instead.
+	// Deprecated: use `workers` instead.
 	Worker string `json:"worker"`
 	// Workers is a CIDRs of a worker subnet (private) to create (used for the VMs).
 	Workers string `json:"workers"`
@@ -95,7 +95,10 @@ type RouterStatus struct {
 	// ID is the Router id.
 	ID string `json:"id"`
 	// IP is the router ip.
+	// Deprecated: use ExternalFixedIPs instead.
 	IP string `json:"ip"`
+	// ExternalFixedIPs is the list of the router's assigned external fixed IPs.
+	ExternalFixedIPs []string `json:"externalFixedIP"`
 }
 
 // FloatingPoolStatus contains information about the floating pool.

pkg/controller/infrastructure/add.go

@@ -40,7 +40,7 @@ type AddOptions struct {
 // The opts.Reconciler is being set with a newly instantiated actuator.
 func AddToManagerWithOptions(ctx context.Context, mgr manager.Manager, options AddOptions) error {
 	return infrastructure.Add(ctx, mgr, infrastructure.AddArgs{
-		Actuator:          NewActuator(mgr, options.DisableProjectedTokenMount),
+		Actuator:          NewActuator(mgr, true),
 		ConfigValidator:   NewConfigValidator(mgr, openstackclient.FactoryFactoryFunc(openstackclient.NewOpenstackClientFromCredentials), log.Log),
 		ControllerOptions: options.Controller,
 		Predicates:        infrastructure.DefaultPredicates(ctx, mgr, options.IgnoreOperationAnnotation),

pkg/controller/infrastructure/infraflow/access/networking_access.go

@@ -29,7 +29,7 @@ type NetworkingAccess interface {
 	CreateRouter(desired *Router) (*Router, error)
 	GetRouterByID(id string) (*Router, error)
 	GetRouterByName(name string) ([]*Router, error)
-	UpdateRouter(desired, current *Router) (modified bool, err error)
+	UpdateRouter(desired, current *Router) (modified bool, router *Router, err error)
 	LookupFloatingPoolSubnetIDs(networkID, floatingPoolSubnetNameRegex string) ([]string, error)
 	AddRouterInterfaceAndWait(ctx context.Context, routerID, subnetID string) error
 	GetRouterInterfacePortID(routerID, subnetID string) (portID *string, err error)
@@ -172,7 +172,8 @@ func (a *networkingAccess) toRouter(raw *routers.Router) *Router {
 }
 
 // UpdateRouter updates the router if important fields have changed
-func (a *networkingAccess) UpdateRouter(desired, current *Router) (modified bool, err error) {
+func (a *networkingAccess) UpdateRouter(desired, current *Router) (modified bool, router *Router, err error) {
+	router = current
 	updateOpts := routers.UpdateOpts{}
 	if desired.Name != current.Name {
 		modified = true
@@ -188,9 +189,13 @@ func (a *networkingAccess) UpdateRouter(desired, current *Router) (modified bool
 		}
 	}
 	if modified {
-		_, err = a.networking.UpdateRouter(current.ID, updateOpts)
+		updated, err := a.networking.UpdateRouter(current.ID, updateOpts)
+		if err != nil {
+			return false, nil, err
+		}
+		router = a.toRouter(updated)
 	}
-	return
+	return modified, router, err
 }
 
 // AddRouterInterfaceAndWait adds router interface and waits up to

pkg/controller/infrastructure/infraflow/context.go

@@ -39,6 +39,8 @@ const (
 	IdentifierSecGroup = "SecurityGroup"
 	// IdentifierShareNetwork is the key for the share network id
 	IdentifierShareNetwork = "ShareNetwork"
+	// IdentifierEgressCIDRs is the key for the slice containing egress CIDRs strings.
+	IdentifierEgressCIDRs = "EgressCIDRs"
 
 	// NameFloatingNetwork is the key for the floating network name
 	NameFloatingNetwork = "FloatingNetworkName"
@@ -171,7 +173,11 @@ func (fctx *FlowContext) computeInfrastructureStatus() *openstackv1alpha1.Infras
 	status.Networks.Name = ptr.Deref(fctx.state.Get(NameNetwork), "")
 
 	status.Networks.Router.ID = ptr.Deref(fctx.state.Get(IdentifierRouter), "")
-	status.Networks.Router.IP = ptr.Deref(fctx.state.Get(RouterIP), "")
+	status.Networks.Router.ExternalFixedIPs = fctx.state.GetObject(IdentifierEgressCIDRs).([]string)
+	// backwards compatibility change for the deprecated field
+	if len(status.Networks.Router.ExternalFixedIPs) > 0 {
+		status.Networks.Router.IP = status.Networks.Router.ExternalFixedIPs[0]
+	}
 
 	status.Node.KeyName = ptr.Deref(fctx.state.Get(NameKeyPair), "")
 

pkg/controller/infrastructure/infraflow/reconcile.go

@@ -41,25 +41,31 @@ func (fctx *FlowContext) Reconcile(ctx context.Context) error {
 		return errors.Join(flow.Causes(err), fctx.persistState(ctx))
 	}
 
-	status := fctx.computeInfrastructureStatus()
 	state := fctx.computeInfrastructureState()
+	status := fctx.computeInfrastructureStatus()
 	return infrainternal.PatchProviderStatusAndState(ctx, fctx.client, fctx.infra, status, state)
 }
 
 func (fctx *FlowContext) buildReconcileGraph() *flow.Graph {
 	g := flow.NewGraph("Openstack infrastructure reconciliation")
 
+	prehook := fctx.AddTask(g, "pre-reconcile hook", func(_ context.Context) error {
+		// delete unnecessary state object. RouterIP was replaced by IdentifierEgressCIDRs to handle cases where the router had multiple externalFixedIPs attached to it.
+		fctx.state.Delete(RouterIP)
+		return nil
+	})
+
 	ensureExternalNetwork := fctx.AddTask(g, "ensure external network",
 		fctx.ensureExternalNetwork,
-		shared.Timeout(defaultTimeout))
+		shared.Timeout(defaultTimeout), shared.Dependencies(prehook))
 
 	ensureRouter := fctx.AddTask(g, "ensure router",
 		fctx.ensureRouter,
 		shared.Timeout(defaultTimeout), shared.Dependencies(ensureExternalNetwork))
 
 	ensureNetwork := fctx.AddTask(g, "ensure network",
 		fctx.ensureNetwork,
-		shared.Timeout(defaultTimeout))
+		shared.Timeout(defaultTimeout), shared.Dependencies(prehook))
 
 	ensureSubnet := fctx.AddTask(g, "ensure subnet",
 		fctx.ensureSubnet,
@@ -118,7 +124,6 @@ func (fctx *FlowContext) ensureConfiguredRouter(_ context.Context) error {
 	router, err := fctx.access.GetRouterByID(fctx.config.Networks.Router.ID)
 	if err != nil {
 		fctx.state.Set(IdentifierRouter, "")
-		fctx.state.Set(RouterIP, "")
 		return err
 	}
 	if router == nil {
@@ -130,8 +135,8 @@ func (fctx *FlowContext) ensureConfiguredRouter(_ context.Context) error {
 	if len(router.ExternalFixedIPs) < 1 {
 		return fmt.Errorf("expected at least one external fixed ip")
 	}
-	fctx.state.Set(RouterIP, router.ExternalFixedIPs[0].IPAddress)
-	return nil
+
+	return fctx.ensureEgressCIDRs(router)
 }
 
 func (fctx *FlowContext) ensureNewRouter(ctx context.Context, externalNetworkID string) error {
@@ -150,10 +155,11 @@ func (fctx *FlowContext) ensureNewRouter(ctx context.Context, externalNetworkID
 		if len(current.ExternalFixedIPs) < 1 {
 			return fmt.Errorf("expected at least one external fixed ip")
 		}
+		if _, current, err = fctx.access.UpdateRouter(desired, current); err != nil {
+			return err
+		}
 		fctx.state.Set(IdentifierRouter, current.ID)
-		fctx.state.Set(RouterIP, current.ExternalFixedIPs[0].IPAddress)
-		_, err := fctx.access.UpdateRouter(desired, current)
-		return err
+		return fctx.ensureEgressCIDRs(current)
 	}
 
 	floatingPoolSubnetName := fctx.findFloatingPoolSubnetName()
@@ -171,10 +177,9 @@ func (fctx *FlowContext) ensureNewRouter(ctx context.Context, externalNetworkID
 	if err != nil {
 		return err
 	}
-	fctx.state.Set(IdentifierRouter, created.ID)
-	fctx.state.Set(RouterIP, created.ExternalFixedIPs[0].IPAddress)
 
-	return nil
+	fctx.state.Set(IdentifierRouter, created.ID)
+	return fctx.ensureEgressCIDRs(created)
 }
 
 func (fctx *FlowContext) findExistingRouter() (*access.Router, error) {
@@ -513,3 +518,12 @@ func (fctx *FlowContext) ensureShareNetwork(ctx context.Context) error {
 	fctx.state.Set(NameShareNetwork, created.Name)
 	return nil
 }
+
+func (fctx *FlowContext) ensureEgressCIDRs(router *access.Router) error {
+	var result []string
+	for _, efip := range router.ExternalFixedIPs {
+		result = append(result, efip.IPAddress)
+	}
+	fctx.state.SetObject(IdentifierEgressCIDRs, result)
+	return nil
+}

pkg/internal/infrastructure/infrastucture.go

@@ -24,6 +24,7 @@ import (
 	"github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
 	openstackv1alpha1 "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/v1alpha1"
 	openstackclient "github.com/gardener/gardener-extension-provider-openstack/pkg/openstack/client"
+	"github.com/gardener/gardener-extension-provider-openstack/pkg/utils"
 )
 
 const (
@@ -158,9 +159,7 @@ func PatchProviderStatusAndState(
 	patch := client.MergeFrom(infra.DeepCopy())
 	if status != nil {
 		infra.Status.ProviderStatus = &runtime.RawExtension{Object: status}
-		if status.Networks.Router.IP != "" {
-			infra.Status.EgressCIDRs = []string{fmt.Sprintf("%s/32", status.Networks.Router.IP)}
-		}
+		infra.Status.EgressCIDRs = utils.ComputeEgressCIDRs(status.Networks.Router.ExternalFixedIPs)
 	}
 
 	if state != nil {

pkg/internal/infrastructure/templates/main.tpl.tf

@@ -168,8 +168,8 @@ output "{{ .outputKeys.routerID }}" {
   value = {{ .router.id }}
 }
 
-output "{{ .outputKeys.routerIP }}" {
-  value = {{ template "router-ip" $ }}
+output "{{ .outputKeys.routerIPs }}" {
+  value = {{ template "router-ips" $ }}
 }
 
 output "{{ .outputKeys.networkID }}" {
@@ -227,10 +227,10 @@ openstack_networking_network_v2.cluster.name
 data.openstack_networking_network_v2.cluster.name
 {{ end -}}
 {{- end -}}
-{{- define "router-ip" -}}
+{{- define "router-ips" -}}
 {{ if .create.router -}}
-openstack_networking_router_v2.router.external_fixed_ip[0].ip_address
+join(",", openstack_networking_router_v2.router.external_fixed_ip[*].ip_address)
 {{ else -}}
-data.openstack_networking_router_v2.router.external_fixed_ip[0].ip_address
+join(",", data.openstack_networking_router_v2.router.external_fixed_ip[*].ip_address)
 {{ end -}}
 {{- end -}}