Skip to content

Commit

Permalink
Backoffs WithContext
Browse files Browse the repository at this point in the history
  • Loading branch information
@bilbobrovall
bilbobrovall committed Jan 15, 2024
1 parent 5a5a51d commit 97a0899
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 40 deletions.
8 changes: 4 additions & 4 deletions api/v1alpha1/openstackfloatingippool_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
// it is the newest API version we should no longer be making breaking
// changes to. If we bump this we need to look carefully for resulting
// CRD changes in v1alpha1 to ensure they are compatible.
"sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7"
infrav1alpha7 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7"
)

const (
Expand Down Expand Up @@ -56,11 +56,11 @@ type OpenStackFloatingIPPoolSpec struct {

// IdentityRef is a reference to a identity to be used when reconciling this pool.
// +optional
IdentityRef *v1alpha7.OpenStackIdentityReference `json:"identityRef,omitempty"`
IdentityRef *infrav1alpha7.OpenStackIdentityReference `json:"identityRef,omitempty"`

// FloatingIPNetwork is the external network to use for floating ips, if there's only one external network it will be used by default
// +optional
FloatingIPNetwork v1alpha7.NetworkFilter `json:"floatingIPNetwork"`
FloatingIPNetwork infrav1alpha7.NetworkFilter `json:"floatingIPNetwork"`

// The name of the cloud to use from the clouds secret
// +optional
Expand Down Expand Up @@ -88,7 +88,7 @@ type OpenStackFloatingIPPoolStatus struct {

// floatingIPNetwork contains information about the network used for floating ips
// +optional
FloatingIPNetwork *v1alpha7.NetworkStatus `json:"floatingIPNetwork,omitempty"`
FloatingIPNetwork *infrav1alpha7.NetworkStatus `json:"floatingIPNetwork,omitempty"`

Conditions clusterv1.Conditions `json:"conditions,omitempty"`
}
Expand Down
74 changes: 38 additions & 36 deletions controllers/openstackfloatingippool_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
"sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7"
infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
infrav1alpha7 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7"
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
Expand Down Expand Up @@ -76,7 +76,7 @@ type OpenStackFloatingIPPoolReconciler struct {

func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
log := ctrl.LoggerFrom(ctx)
pool := &v1alpha1.OpenStackFloatingIPPool{}
pool := &infrav1alpha1.OpenStackFloatingIPPool{}
if err := r.Client.Get(ctx, req.NamespacedName, pool); err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
}
Expand All @@ -93,7 +93,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c

if pool.ObjectMeta.DeletionTimestamp.IsZero() {
// Add finalizer if it does not exist
if controllerutil.AddFinalizer(pool, v1alpha1.OpenStackFloatingIPPoolFinalizer) {
if controllerutil.AddFinalizer(pool, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) {
return ctrl.Result{}, r.Client.Update(ctx, pool)
}
} else {
Expand All @@ -119,7 +119,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
}

claims := &ipamv1.IPAddressClaimList{}
if err := r.Client.List(context.Background(), claims, client.InNamespace(req.Namespace), client.MatchingFields{v1alpha1.OpenStackFloatingIPPoolNameIndex: pool.Name}); err != nil {
if err := r.Client.List(context.Background(), claims, client.InNamespace(req.Namespace), client.MatchingFields{infrav1alpha1.OpenStackFloatingIPPoolNameIndex: pool.Name}); err != nil {
return ctrl.Result{}, err
}

Expand All @@ -137,7 +137,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
return ctrl.Result{}, err
}
if apierrors.IsNotFound(err) {
ip, err := r.getIP(scope, pool)
ip, err := r.getIP(ctx, scope, pool)
if err != nil {
return ctrl.Result{}, err
}
Expand All @@ -147,7 +147,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
Name: claim.Name,
Namespace: claim.Namespace,
Finalizers: []string{
v1alpha1.DeleteFloatingIPFinalizer,
infrav1alpha1.DeleteFloatingIPFinalizer,
},
OwnerReferences: []metav1.OwnerReference{
{
Expand All @@ -163,7 +163,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
Name: claim.Name,
},
PoolRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String(v1alpha1.GroupVersion.Group),
APIGroup: pointer.String(infrav1alpha1.GroupVersion.Group),
Kind: pool.Kind,
Name: pool.Name,
},
Expand All @@ -173,7 +173,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
}

// Retry creating the IPAddress object
err = wait.ExponentialBackoff(backoff, func() (bool, error) {
err = wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (bool, error) {
if err := r.Client.Create(ctx, ipAddress); err != nil {
return false, err
}
Expand All @@ -196,10 +196,10 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
return ctrl.Result{}, r.Client.Status().Update(ctx, pool)
}

func (r *OpenStackFloatingIPPoolReconciler) reconcileDelete(ctx context.Context, scope scope.Scope, pool *v1alpha1.OpenStackFloatingIPPool) error {
func (r *OpenStackFloatingIPPoolReconciler) reconcileDelete(ctx context.Context, scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) error {
log := ctrl.LoggerFrom(ctx)
ipAddresses := &ipamv1.IPAddressList{}
if err := r.Client.List(ctx, ipAddresses, client.InNamespace(pool.Namespace), client.MatchingFields{v1alpha1.OpenStackFloatingIPPoolNameIndex: pool.Name}); err != nil {
if err := r.Client.List(ctx, ipAddresses, client.InNamespace(pool.Namespace), client.MatchingFields{infrav1alpha1.OpenStackFloatingIPPoolNameIndex: pool.Name}); err != nil {
return err
}

Expand All @@ -223,7 +223,7 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileDelete(ctx context.Context,
pool.Status.AvailableIPs = diff(pool.Status.AvailableIPs, []string{ip})
}

if controllerutil.RemoveFinalizer(pool, v1alpha1.OpenStackFloatingIPPoolFinalizer) {
if controllerutil.RemoveFinalizer(pool, infrav1alpha1.OpenStackFloatingIPPoolFinalizer) {
log.Info("Removing finalizer from OpenStackFloatingIPPool")
return r.Client.Update(ctx, pool)
}
Expand Down Expand Up @@ -260,9 +260,9 @@ func diff(a []string, b []string) []string {
return result
}

func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Context, scope scope.Scope, pool *v1alpha1.OpenStackFloatingIPPool) error {
func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Context, scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) error {
ipAddresses := &ipamv1.IPAddressList{}
if err := r.Client.List(ctx, ipAddresses, client.InNamespace(pool.Namespace), client.MatchingFields{v1alpha1.OpenStackFloatingIPPoolNameIndex: pool.Name}); err != nil {
if err := r.Client.List(ctx, ipAddresses, client.InNamespace(pool.Namespace), client.MatchingFields{infrav1alpha1.OpenStackFloatingIPPoolNameIndex: pool.Name}); err != nil {
return err
}

Expand All @@ -282,16 +282,16 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con
continue
}

if controllerutil.ContainsFinalizer(ipAddress, v1alpha1.DeleteFloatingIPFinalizer) {
if pool.Spec.ReclaimPolicy == v1alpha1.ReclaimDelete && !contains(pool.Spec.PreAllocatedFloatingIPs, ipAddress.Spec.Address) {
if controllerutil.ContainsFinalizer(ipAddress, infrav1alpha1.DeleteFloatingIPFinalizer) {
if pool.Spec.ReclaimPolicy == infrav1alpha1.ReclaimDelete && !contains(pool.Spec.PreAllocatedFloatingIPs, ipAddress.Spec.Address) {
if err = networkingService.DeleteFloatingIP(pool, ipAddress.Spec.Address); err != nil {
return fmt.Errorf("delete floating IP %q: %w", ipAddress.Spec.Address, err)
}
} else {
pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, ipAddress.Spec.Address)
}
}
controllerutil.RemoveFinalizer(ipAddress, v1alpha1.DeleteFloatingIPFinalizer)
controllerutil.RemoveFinalizer(ipAddress, infrav1alpha1.DeleteFloatingIPFinalizer)
if err := r.Client.Update(ctx, ipAddress); err != nil {
return err
}
Expand All @@ -302,7 +302,7 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con
return nil
}

func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *v1alpha1.OpenStackFloatingIPPool) (string, error) {
func (r *OpenStackFloatingIPPoolReconciler) getIP(ctx context.Context, scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) (string, error) {
// There's a potential leak of IPs here, if the reconcile loop fails after we claim an IP but before we create the IPAddress object.
var ip string

Expand All @@ -314,17 +314,19 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *v1alp

// Get tagged floating IPs and add them to the available IPs if they are not present in either the available IPs or the claimed IPs
// This is done to prevent leaking floating IPs if to prevent leaking floating IPs if the floating IP was created but the IPAddress object was not
taggedIPs, err := networkingService.GetFloatingIPsByTag(pool.GetFloatingIPTag())
if err != nil {
scope.Logger().Error(err, "Failed to get floating IPs by tag", "pool", pool.Name)
return "", err
}
for _, taggedIP := range taggedIPs {
if contains(pool.Status.AvailableIPs, taggedIP.FloatingIP) || contains(pool.Status.ClaimedIPs, taggedIP.FloatingIP) {
continue
if len(pool.Status.AvailableIPs) == 0 {
taggedIPs, err := networkingService.GetFloatingIPsByTag(pool.GetFloatingIPTag())
if err != nil {
scope.Logger().Error(err, "Failed to get floating IPs by tag", "pool", pool.Name)
return "", err
}
for _, taggedIP := range taggedIPs {
if contains(pool.Status.AvailableIPs, taggedIP.FloatingIP) || contains(pool.Status.ClaimedIPs, taggedIP.FloatingIP) {
continue
}
scope.Logger().Info("Tagged floating IP found that was not known to the pool, adding it to the pool", "ip", taggedIP.FloatingIP)
pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, taggedIP.FloatingIP)
}
scope.Logger().Info("Tagged floating IP found that was not known to the pool, adding it to the pool", "ip", taggedIP.FloatingIP)
pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, taggedIP.FloatingIP)
}

if len(pool.Status.AvailableIPs) > 0 {
Expand All @@ -345,7 +347,7 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *v1alp
fp, err := networkingService.CreateFloatingIPForPool(pool)
if err != nil {
scope.Logger().Error(err, "Failed to create floating IP", "pool", pool.Name)
conditions.MarkFalse(pool, v1alpha1.OpenstackFloatingIPPoolReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, "Failed to create floating IP: %v", err)
conditions.MarkFalse(pool, infrav1alpha1.OpenstackFloatingIPPoolReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, "Failed to create floating IP: %v", err)
if ip != "" {
pool.Status.FailedIPs = append(pool.Status.FailedIPs, ip)
}
Expand All @@ -354,7 +356,7 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *v1alp
defer func() {
tag := pool.GetFloatingIPTag()

err := wait.ExponentialBackoff(backoff, func() (bool, error) {
err := wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (bool, error) {
if err := networkingService.TagFloatingIP(fp.FloatingIP, tag); err != nil {
scope.Logger().Error(err, "Failed to tag floating, retrying", "ip", fp.FloatingIP, "tag", tag)
return false, err
Expand All @@ -366,14 +368,14 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *v1alp
}
}()

conditions.MarkTrue(pool, v1alpha1.OpenstackFloatingIPPoolReadyCondition)
conditions.MarkTrue(pool, infrav1alpha1.OpenstackFloatingIPPoolReadyCondition)

ip = fp.FloatingIP
pool.Status.ClaimedIPs = append(pool.Status.ClaimedIPs, ip)
return ip, nil
}

func (r *OpenStackFloatingIPPoolReconciler) reconcileFloatingIPNetwork(scope scope.Scope, pool *v1alpha1.OpenStackFloatingIPPool) error {
func (r *OpenStackFloatingIPPoolReconciler) reconcileFloatingIPNetwork(scope scope.Scope, pool *infrav1alpha1.OpenStackFloatingIPPool) error {
// If the pool already has a network, we don't need to do anything
if pool.Status.FloatingIPNetwork != nil {
return nil
Expand All @@ -397,7 +399,7 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileFloatingIPNetwork(scope sco
return fmt.Errorf("found multiple networks, expects filter to match one (result: %v)", networkList)
}

pool.Status.FloatingIPNetwork = &v1alpha7.NetworkStatus{
pool.Status.FloatingIPNetwork = &infrav1alpha7.NetworkStatus{
ID: networkList[0].ID,
Name: networkList[0].Name,
Tags: networkList[0].Tags,
Expand Down Expand Up @@ -442,7 +444,7 @@ func (r *OpenStackFloatingIPPoolReconciler) ipAddressToPoolMapper(_ context.Cont
}

func (r *OpenStackFloatingIPPoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
if err := mgr.GetFieldIndexer().IndexField(ctx, &ipamv1.IPAddressClaim{}, v1alpha1.OpenStackFloatingIPPoolNameIndex, func(rawObj client.Object) []string {
if err := mgr.GetFieldIndexer().IndexField(ctx, &ipamv1.IPAddressClaim{}, infrav1alpha1.OpenStackFloatingIPPoolNameIndex, func(rawObj client.Object) []string {
claim := rawObj.(*ipamv1.IPAddressClaim)
if claim.Spec.PoolRef.Kind != openStackFloatingIPPool {
return nil
Expand All @@ -452,15 +454,15 @@ func (r *OpenStackFloatingIPPoolReconciler) SetupWithManager(ctx context.Context
return err
}

if err := mgr.GetFieldIndexer().IndexField(ctx, &ipamv1.IPAddress{}, v1alpha1.OpenStackFloatingIPPoolNameIndex, func(rawObj client.Object) []string {
if err := mgr.GetFieldIndexer().IndexField(ctx, &ipamv1.IPAddress{}, infrav1alpha1.OpenStackFloatingIPPoolNameIndex, func(rawObj client.Object) []string {
ip := rawObj.(*ipamv1.IPAddress)
return []string{ip.Spec.PoolRef.Name}
}); err != nil {
return err
}

return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.OpenStackFloatingIPPool{}).
For(&infrav1alpha1.OpenStackFloatingIPPool{}).
Watches(
&ipamv1.IPAddressClaim{},
handler.EnqueueRequestsFromMapFunc(r.ipAddressClaimToPoolMapper),
Expand Down

0 comments on commit 97a0899

Please sign in to comment.