fix: reserve ping probed ip with expiration (#18854)

Co-authored-by: Qiu Jian <[email protected]>

pkg/cloudmon/misc/pinger.go

@@ -16,6 +16,7 @@ package misc
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 	"time"
 
@@ -146,6 +147,7 @@ func pingProbeNetwork(s *mcclient.ClientSession, net sNetwork) ([]influxdb.SMetr
 		return nil, errors.Wrap(err, "getNetworkAddrMap")
 	}
 
+	reserveIps := make([]string, 0)
 	now := time.Now().UTC()
 	for addr := addrStart; addr <= addrEnd; addr = addr.StepUp() {
 		addrStr := addr.String()
@@ -154,15 +156,16 @@ func pingProbeNetwork(s *mcclient.ClientSession, net sNetwork) ([]influxdb.SMetr
 		if allocated {
 			if netAddr.OwnerType == api.RESERVEDIP_RESOURCE_TYPES {
 				loss := pingResult.Loss()
-				status := api.RESERVEDIP_STATUS_OFFLINE
 				if loss < 100 {
-					status = api.RESERVEDIP_STATUS_ONLINE
-				}
-				params := jsonutils.NewDict()
-				params.Add(jsonutils.NewString(status), "status")
-				_, err := compute.ReservedIPs.Update(s, netAddr.OwnerId, params)
-				if err != nil {
-					log.Errorf("update reserved ip %s status fail: %s", addrStr, err)
+					log.Debugf("Reserved address %s continues responding ping, extend reserving the address", addrStr)
+					reserveIps = append(reserveIps, addrStr)
+				} else {
+					params := jsonutils.NewDict()
+					params.Add(jsonutils.NewString(api.RESERVEDIP_STATUS_OFFLINE), "status")
+					_, err := compute.ReservedIPs.Update(s, netAddr.OwnerId, params)
+					if err != nil {
+						log.Errorf("update reserved ip %s status fail: %s", addrStr, err)
+					}
 				}
 			} else {
 				// send metrics
@@ -214,17 +217,23 @@ func pingProbeNetwork(s *mcclient.ClientSession, net sNetwork) ([]influxdb.SMetr
 			if loss < 100 {
 				// reserve ip
 				log.Debugf("Free address %s is responding ping, reserve the address", addrStr)
-				params := jsonutils.NewDict()
-				params.Add(jsonutils.NewStringArray([]string{addrStr}), "ips")
-				params.Add(jsonutils.NewString("ping detect online free IP"), "notes")
-				params.Add(jsonutils.NewString(api.RESERVEDIP_STATUS_ONLINE), "status")
-				_, err = compute.Networks.PerformAction(s, net.Id, "reserve-ip", params)
-				if err != nil {
-					log.Errorf("failed to reserve ip %s: %s", addrStr, err)
-				}
+				reserveIps = append(reserveIps, addrStr)
 			}
 		}
 		log.Debugf("%s %s allocated %v", addrStr, netAddr, allocated)
 	}
+	if len(reserveIps) > 0 {
+		params := jsonutils.NewDict()
+		params.Add(jsonutils.NewStringArray(reserveIps), "ips")
+		params.Add(jsonutils.NewString("ping detected online free IP"), "notes")
+		params.Add(jsonutils.NewString(api.RESERVEDIP_STATUS_ONLINE), "status")
+		if options.Options.PingReserveIPTimeoutHours > 0 {
+			params.Add(jsonutils.NewString(fmt.Sprintf("%dH", options.Options.PingReserveIPTimeoutHours)), "duration")
+		}
+		_, err = compute.Networks.PerformAction(s, net.Id, "reserve-ip", params)
+		if err != nil {
+			log.Errorf("failed to reserve ip %#v: %s", reserveIps, err)
+		}
+	}
 	return metrics, nil
 }

pkg/cloudmon/options/options.go

@@ -45,6 +45,8 @@ type PingProbeOptions struct {
 
 	DisablePingProbe      bool  `help:"enable ping probe"`
 	PingProbIntervalHours int64 `help:"PingProb Interval unit:hour" default:"6"`
+
+	PingReserveIPTimeoutHours int `help:"expire hours to reserve the probed IP, default 0, never expire" default:"0"`
 }
 
 var (

pkg/cloudmon/service/service.go

@@ -55,7 +55,7 @@ func StartService() {
 		cron.AddJobAtIntervals("UpdateResources", time.Duration(opts.ResourcesSyncInterval)*time.Minute, res.UpdateSync)
 		cron.AddJobAtIntervalsWithStarTime("CollectResources", time.Duration(opts.CollectMetricInterval)*time.Minute, res.CollectMetrics)
 
-		cron.AddJobAtIntervals("PingProb", time.Duration(opts.PingProbIntervalHours)*time.Hour, misc.PingProbe)
+		cron.AddJobAtIntervalsWithStartRun("PingProb", time.Duration(opts.PingProbIntervalHours)*time.Hour, misc.PingProbe, true)
 
 		cron.AddJobEveryFewDays("UsageMetricCollect", 1, 23, 10, 10, misc.UsegReport, false)
 		cron.AddJobEveryFewDays("AlertHistoryMetricCollect", 1, 23, 59, 59, misc.AlertHistoryReport, false)

pkg/compute/models/networks.go

@@ -1225,37 +1225,49 @@ func (self *SNetwork) PerformReserveIp(ctx context.Context, userCred mcclient.To
 		duration = bc.Duration()
 	}
 
+	errs := make([]error, 0)
 	for _, ip := range input.Ips {
 		err := self.reserveIpWithDurationAndStatus(ctx, userCred, ip, input.Notes, duration, input.Status)
 		if err != nil {
-			return nil, err
+			errs = append(errs, errors.Wrap(err, "reserveIpWithDurationAndStatus"))
 		}
 	}
+	if len(errs) > 0 {
+		return nil, errors.NewAggregate(errs)
+	}
 	return nil, nil
 }
 
-func (self *SNetwork) reserveIpWithDuration(ctx context.Context, userCred mcclient.TokenCredential, ipstr string, notes string, duration time.Duration) error {
-	return self.reserveIpWithDurationAndStatus(ctx, userCred, ipstr, notes, duration, "")
+func (net *SNetwork) reserveIpWithDuration(ctx context.Context, userCred mcclient.TokenCredential, ipstr string, notes string, duration time.Duration) error {
+	return net.reserveIpWithDurationAndStatus(ctx, userCred, ipstr, notes, duration, "")
 }
 
-func (self *SNetwork) reserveIpWithDurationAndStatus(ctx context.Context, userCred mcclient.TokenCredential, ipstr string, notes string, duration time.Duration, status string) error {
+func (net *SNetwork) reserveIpWithDurationAndStatus(ctx context.Context, userCred mcclient.TokenCredential, ipstr string, notes string, duration time.Duration, status string) error {
 	ipAddr, err := netutils.NewIPV4Addr(ipstr)
 	if err != nil {
 		return httperrors.NewInputParameterError("not a valid ip address %s: %s", ipstr, err)
 	}
-	if !self.IsAddressInRange(ipAddr) {
+	if !net.IsAddressInRange(ipAddr) {
 		return httperrors.NewInputParameterError("Address %s not in network", ipstr)
 	}
-	used, err := self.isAddressUsed(ipstr)
+	used, err := net.isAddressUsed(ipstr)
 	if err != nil {
 		return httperrors.NewInternalServerError("isAddressUsed fail %s", err)
 	}
 	if used {
+		rip := ReservedipManager.getReservedIP(net, ipstr)
+		if rip != nil {
+			err := rip.extendWithDuration(notes, duration, status)
+			if err != nil {
+				return errors.Wrap(err, "extendWithDuration")
+			}
+			return nil
+		}
 		return httperrors.NewConflictError("Address %s has been used", ipstr)
 	}
-	err = ReservedipManager.ReserveIPWithDurationAndStatus(userCred, self, ipstr, notes, duration, status)
+	err = ReservedipManager.ReserveIPWithDurationAndStatus(userCred, net, ipstr, notes, duration, status)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "ReservedipManager.ReserveIPWithDurationAndStatus")
 	}
 	return nil
 }

pkg/compute/models/reservedips.go

@@ -95,32 +95,48 @@ func (manager *SReservedipManager) ReserveIPWithDurationAndStatus(userCred mccli
 	}
 	rip := manager.getReservedIP(network, ip)
 	if rip == nil {
+		// not found
 		rip := SReservedip{IpAddr: ip, Notes: notes, ExpiredAt: expiredAt, Status: status}
 		rip.NetworkId = network.Id
 		err := manager.TableSpec().Insert(context.TODO(), &rip)
 		if err != nil {
 			log.Errorf("ReserveIP fail: %s", err)
 			return errors.Wrap(err, "Insert")
 		}
-	} else if rip.IsExpired() {
-		_, err := db.Update(rip, func() error {
-			rip.Notes = notes
-			rip.ExpiredAt = expiredAt
-			if len(status) > 0 {
-				rip.Status = status
-			}
-			return nil
-		})
+	} else {
+		// extend the expiration
+		err := rip.extend(notes, expiredAt, status)
 		if err != nil {
-			return errors.Wrap(err, "Update")
+			return errors.Wrap(err, "extend")
 		}
-	} else {
-		return errors.Wrapf(httperrors.ErrConflict, "Address %s has been reserved", ip)
 	}
 	db.OpsLog.LogEvent(network, db.ACT_RESERVE_IP, ip, userCred)
 	return nil
 }
 
+func (rip *SReservedip) extendWithDuration(notes string, duration time.Duration, status string) error {
+	expiredAt := time.Time{}
+	if duration > 0 {
+		expiredAt = time.Now().UTC().Add(duration)
+	}
+	return rip.extend(notes, expiredAt, status)
+}
+
+func (rip *SReservedip) extend(notes string, expiredAt time.Time, status string) error {
+	_, err := db.Update(rip, func() error {
+		rip.Notes = notes
+		rip.ExpiredAt = expiredAt
+		if len(status) > 0 {
+			rip.Status = status
+		}
+		return nil
+	})
+	if err != nil {
+		return errors.Wrap(err, "Update")
+	}
+	return nil
+}
+
 func (manager *SReservedipManager) getReservedIP(network *SNetwork, ip string) *SReservedip {
 	rip := SReservedip{}
 	rip.SetModelManager(manager, &rip)