Skip to content

Commit

Permalink
Fix validations in ValidateClustersetIP
Browse files Browse the repository at this point in the history
...which give false negative when there is a conflict with
another cluster.

1. Don't check if allocation size and ClustersetIPCIDR are
   both configured. This check is not required coz user
   can't set size and is auto-allocated. This check causes
   a false negative when we have to retry CIDR allocation due
   to conflict with another cluster.

2. On conflict, reset netconfig.ClustersetIPCIDR to user configured
   CIDR before retrying. This avoids failing validation with CIDR
   that already conflicted.

Signed-off-by: Vishal Thapar <[email protected]>
  • Loading branch information
vthapar authored and tpantelis committed Sep 23, 2024
1 parent 8cd5208 commit 268a996
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions pkg/discovery/clustersetip/clustersetip.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,6 @@ func ValidateClustersetIPConfiguration(clustersetIPInfo *Info, netconfig Config,
clustersetIPInfo.AllocationSize = clusterSize
}

if clustersetIPCIDR != "" && clustersetIPClusterSize != 0 {
status.Failure("Only one of cluster size and clustersetip CIDR can be specified")

return "", errors.New("only one of cluster size and clustersetip CIDR can be specified")
}

if clustersetIPCIDR != "" {
err := cidr.IsValid(clustersetIPCIDR)
if err != nil {
Expand Down Expand Up @@ -178,6 +172,7 @@ func AllocateCIDRFromConfigMap(ctx context.Context, brokerAdminClient controller
netconfig.AllocationSize = DefaultAllocationSize
}

userClustersetIPCIDR := netconfig.ClustersetIPCIDR
retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
status.Start("Retrieving ClustersetIP information from the Broker")
defer status.End()
Expand Down Expand Up @@ -209,6 +204,8 @@ func AllocateCIDRFromConfigMap(ctx context.Context, brokerAdminClient controller
err = updateConfigMap(ctx, brokerAdminClient, clustersetIPConfigMap, newClusterInfo)
if apierrors.IsConflict(err) {
status.Warning("Conflict occurred updating the ClustersetIP ConfigMap - retrying")
// Conflict with allocation, retry with user given CIDR to try reallocation
netconfig.ClustersetIPCIDR = userClustersetIPCIDR
} else {
return status.Error(err, "error updating the ClustersetIP ConfigMap")
}
Expand Down

0 comments on commit 268a996

Please sign in to comment.