do not download binary before verifying the version (#20523)

fixes https://github.com/minio/mc/issues/4980

cmd/admin-handlers.go

@@ -93,11 +93,18 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		return
 	}
 
-	if globalInplaceUpdateDisabled || currentReleaseTime.IsZero() {
+	if globalInplaceUpdateDisabled {
 		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrMethodNotAllowed), r.URL)
 		return
 	}
 
+	if currentReleaseTime.IsZero() || currentReleaseTime.Equal(timeSentinel) {
+		apiErr := errorCodes.ToAPIErr(ErrMethodNotAllowed)
+		apiErr.Description = fmt.Sprintf("unable to perform in-place update, release time is unrecognized: %s", currentReleaseTime)
+		writeErrorResponseJSON(ctx, w, apiErr, r.URL)
+		return
+	}
+
 	vars := mux.Vars(r)
 	updateURL := vars["updateURL"]
 	dryRun := r.Form.Get("dry-run") == "true"
@@ -110,6 +117,11 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		}
 	}
 
+	local := globalLocalNodeName
+	if local == "" {
+		local = "127.0.0.1"
+	}
+
 	u, err := url.Parse(updateURL)
 	if err != nil {
 		writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
@@ -128,6 +140,39 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		return
 	}
 
+	updateStatus := madmin.ServerUpdateStatusV2{
+		DryRun:  dryRun,
+		Results: make([]madmin.ServerPeerUpdateStatus, 0, len(globalNotificationSys.allPeerClients)),
+	}
+	peerResults := make(map[string]madmin.ServerPeerUpdateStatus, len(globalNotificationSys.allPeerClients))
+	failedClients := make(map[int]bool, len(globalNotificationSys.allPeerClients))
+
+	if lrTime.Sub(currentReleaseTime) <= 0 {
+		updateStatus.Results = append(updateStatus.Results, madmin.ServerPeerUpdateStatus{
+			Host:           local,
+			Err:            fmt.Sprintf("server is running the latest version: %s", Version),
+			CurrentVersion: Version,
+		})
+
+		for _, client := range globalNotificationSys.peerClients {
+			updateStatus.Results = append(updateStatus.Results, madmin.ServerPeerUpdateStatus{
+				Host:           client.String(),
+				Err:            fmt.Sprintf("server is running the latest version: %s", Version),
+				CurrentVersion: Version,
+			})
+		}
+
+		// Marshal API response
+		jsonBytes, err := json.Marshal(updateStatus)
+		if err != nil {
+			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
+			return
+		}
+
+		writeSuccessResponseJSON(w, jsonBytes)
+		return
+	}
+
 	u.Path = path.Dir(u.Path) + SlashSeparator + releaseInfo
 	// Download Binary Once
 	binC, bin, err := downloadBinary(u, mode)
@@ -137,16 +182,6 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		return
 	}
 
-	updateStatus := madmin.ServerUpdateStatusV2{DryRun: dryRun}
-	peerResults := make(map[string]madmin.ServerPeerUpdateStatus)
-
-	local := globalLocalNodeName
-	if local == "" {
-		local = "127.0.0.1"
-	}
-
-	failedClients := make(map[int]struct{})
-
 	if globalIsDistErasure {
 		// Push binary to other servers
 		for idx, nerr := range globalNotificationSys.VerifyBinary(ctx, u, sha256Sum, releaseInfo, binC) {
@@ -156,7 +191,7 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 					Err:            nerr.Err.Error(),
 					CurrentVersion: Version,
 				}
-				failedClients[idx] = struct{}{}
+				failedClients[idx] = true
 			} else {
 				peerResults[nerr.Host.String()] = madmin.ServerPeerUpdateStatus{
 					Host:           nerr.Host.String(),
@@ -167,25 +202,17 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		}
 	}
 
-	if lrTime.Sub(currentReleaseTime) > 0 {
+	if err = verifyBinary(u, sha256Sum, releaseInfo, mode, bytes.NewReader(bin)); err != nil {
-		if err = verifyBinary(u, sha256Sum, releaseInfo, mode, bytes.NewReader(bin)); err != nil {
+		peerResults[local] = madmin.ServerPeerUpdateStatus{
-			peerResults[local] = madmin.ServerPeerUpdateStatus{
+			Host:           local,
-				Host:           local,
+			Err:            err.Error(),
-				Err:            err.Error(),
+			CurrentVersion: Version,
-				CurrentVersion: Version,
-			}
-		} else {
-			peerResults[local] = madmin.ServerPeerUpdateStatus{
-				Host:           local,
-				CurrentVersion: Version,
-				UpdatedVersion: lrTime.Format(MinioReleaseTagTimeLayout),
-			}
 		}
 	} else {
 		peerResults[local] = madmin.ServerPeerUpdateStatus{
 			Host:           local,
-			Err:            fmt.Sprintf("server is already running the latest version: %s", Version),
 			CurrentVersion: Version,
+			UpdatedVersion: lrTime.Format(MinioReleaseTagTimeLayout),
 		}
 	}
 
@@ -193,8 +220,7 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		if globalIsDistErasure {
 			ng := WithNPeers(len(globalNotificationSys.peerClients))
 			for idx, client := range globalNotificationSys.peerClients {
-				_, ok := failedClients[idx]
+				if failedClients[idx] {
-				if ok {
 					continue
 				}
 				client := client
@@ -240,14 +266,14 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 		startTime := time.Now().Add(restartUpdateDelay)
 		ng := WithNPeers(len(globalNotificationSys.peerClients))
 		for idx, client := range globalNotificationSys.peerClients {
-			_, ok := failedClients[idx]
+			if failedClients[idx] {
-			if ok {
 				continue
 			}
 			client := client
 			ng.Go(ctx, func() error {
 				prs, ok := peerResults[client.String()]
-				if ok && prs.CurrentVersion != prs.UpdatedVersion && prs.UpdatedVersion != "" {
+				// We restart only on success, not for any failures.
+				if ok && prs.Err == "" {
 					return client.SignalService(serviceRestart, "", dryRun, &startTime)
 				}
 				return nil
@@ -284,7 +310,9 @@ func (a adminAPIHandlers) ServerUpdateV2Handler(w http.ResponseWriter, r *http.R
 	writeSuccessResponseJSON(w, jsonBytes)
 
 	if !dryRun {
-		if lrTime.Sub(currentReleaseTime) > 0 {
+		prs, ok := peerResults[local]
+		// We restart only on success, not for any failures.
+		if ok && prs.Err == "" {
 			globalServiceSignalCh <- serviceRestart
 		}
 	}

cmd/peer-rest-server.go

@@ -636,7 +636,7 @@ func (s *peerRESTServer) VerifyBinaryHandler(w http.ResponseWriter, r *http.Requ
 	}
 
 	if lrTime.Sub(currentReleaseTime) <= 0 {
-		s.writeErrorResponse(w, fmt.Errorf("server is already running the latest version: %s", Version))
+		s.writeErrorResponse(w, fmt.Errorf("server is running the latest version: %s", Version))
 		return
 	}