get fresh upgrade statuses

appgate · Jun 27, 2024 · 9b7b0d7 · 9b7b0d7
1 parent 8ae881a
commit 9b7b0d7
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 42 deletions.
diff --git a/cmd/appliance/upgrade/complete.go b/cmd/appliance/upgrade/complete.go
@@ -198,7 +198,11 @@ func upgradeCompleteRun(cmd *cobra.Command, args []string, opts *upgradeComplete
 	if err != nil {
 		return err
 	}
-	plan, err := appliancepkg.NewUpgradePlan(rawAppliances, initialStats, controlHost, filter, orderBy, descending)
+	upgradeStatusMap, err := a.UpgradeStatusMap(ctx, rawAppliances)
+	if err != nil {
+		return err
+	}
+	plan, err := appliancepkg.NewUpgradePlan(rawAppliances, initialStats, upgradeStatusMap, controlHost, filter, orderBy, descending)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/appliance/upgrade/complete_test.go b/cmd/appliance/upgrade/complete_test.go
@@ -80,6 +80,22 @@ func TestUpgradeCompleteCommand(t *testing.T) {
 			name:       "test no appliances ready",
 			cli:        "upgrade complete --no-interactive",
 			appliances: []string{appliancepkg.TestApplianceUnpreparedPrimary, appliancepkg.TestApplianceControllerNotPrepared},
+			customStubs: []httpmock.Stub{
+				{
+					URL: "/admin/appliances/{appliance}/upgrade",
+					Responder: func(w http.ResponseWriter, r *http.Request) {
+						us := openapi.NewStatsAppliancesListAllOfUpgradeWithDefaults()
+						us.SetStatus(appliancepkg.UpgradeStatusIdle)
+						body, err := us.MarshalJSON()
+						if err != nil {
+							w.WriteHeader(http.StatusInternalServerError)
+							return
+						}
+						w.Header().Add("Content-Type", "application/json")
+						w.Write(body)
+					},
+				},
+			},
 			from:       "6.2.0",
 			to:         "6.2.1",
 			wantErr:    true,

diff --git a/cmd/root.go b/cmd/root.go
@@ -8,7 +8,6 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"runtime"
 	"strings"
 	"time"
 
@@ -234,7 +233,6 @@ func BindEnvs(iface interface{}, parts ...string) {
 }
 
 func Execute() cmdutil.ExitCode {
-	defer runtime.GC()
 	var currentProfile string
 	rFlag := pflag.NewFlagSet("", pflag.ContinueOnError)
 	rFlag.StringVarP(&currentProfile, "profile", "p", "", "")

diff --git a/pkg/appliance/mock.go b/pkg/appliance/mock.go
@@ -147,23 +147,16 @@ var (
 		w.WriteHeader(http.StatusOK)
 		fmt.Fprint(w, string(body))
 	}
-	mutatingResponder = func(callback func(count int) ([]byte, error)) http.HandlerFunc {
+	DefaultResponder = func(callback func(rw http.ResponseWriter, req *http.Request, count int)) http.HandlerFunc {
 		count := 0
 		return func(w http.ResponseWriter, r *http.Request) {
-			mutated, err := callback(count)
-			if err != nil {
-				panic(fmt.Sprintf("Internal testing error; request mutation failed %q", err))
-			}
-			count++
-			w.Header().Set("Content-Type", "application/json")
-			w.WriteHeader(http.StatusOK)
-			fmt.Fprint(w, string(mutated))
+			callback(w, r, count)
 		}
 	}
-	mutatingApplianceResponder = func(id string, callback func(id string, count int) ([]byte, error)) http.HandlerFunc {
+	mutatingResponder = func(callback func(count int) ([]byte, error)) http.HandlerFunc {
 		count := 0
 		return func(w http.ResponseWriter, r *http.Request) {
-			mutated, err := callback(id, count)
+			mutated, err := callback(count)
 			if err != nil {
 				panic(fmt.Sprintf("Internal testing error; request mutation failed %q", err))
 			}
@@ -217,6 +210,32 @@ func (cts *CollectiveTestStruct) GenerateStubs(appliances []openapi.Appliance, s
 	}
 	stubs = append(stubs, statsStub)
 
+	stubs = append(stubs, httpmock.Stub{
+		URL: "/admin/appliances/{appliance}/upgrade",
+		Responder: DefaultResponder(func(rw http.ResponseWriter, req *http.Request, count int) {
+			id := req.PathValue("appliance")
+			for _, s := range stats.GetData() {
+				if s.GetId() != id {
+					continue
+				}
+				us := s.GetUpgrade()
+				if count <= 0 {
+					us.SetStatus(UpgradeStatusReady)
+				} else if count == 1 {
+					us.SetStatus(UpgradeStatusIdle)
+					us.SetDetails("")
+				}
+				body, err := us.MarshalJSON()
+				if err != nil {
+					rw.WriteHeader(http.StatusInternalServerError)
+					return
+				}
+				rw.Header().Set("Content-Type", "application/json")
+				rw.Write(body)
+			}
+		}),
+	})
+
 	// global settings stub
 	globalSettingsStub := httpmock.Stub{
 		URL: "/admin/global-settings",
@@ -245,25 +264,6 @@ func (cts *CollectiveTestStruct) GenerateStubs(appliances []openapi.Appliance, s
 			URL:       fmt.Sprintf("/admin/appliances/%s/maintenance", a.GetId()),
 			Responder: changeRequestResponder,
 		})
-		stubs = append(stubs, httpmock.Stub{
-			URL: fmt.Sprintf("/admin/appliances/%s/upgrade", a.GetId()),
-			Responder: mutatingApplianceResponder(a.GetId(), func(id string, count int) ([]byte, error) {
-				for _, s := range stats.GetData() {
-					if s.GetId() != id {
-						continue
-					}
-					us := s.GetUpgrade()
-					if count <= 0 {
-						us.SetStatus(UpgradeStatusReady)
-					} else if count == 1 {
-						us.SetStatus(UpgradeStatusIdle)
-						us.SetDetails("")
-					}
-					return json.Marshal(us)
-				}
-				return nil, fmt.Errorf("failed to find stats for '%s'", id)
-			}),
-		})
 
 		backupID := uuid.NewString()
 		stubs = append(stubs, httpmock.Stub{

diff --git a/pkg/appliance/upgrade.go b/pkg/appliance/upgrade.go
@@ -49,7 +49,7 @@ type UpgradePlan struct {
 	adminHostname           string
 }
 
-func NewUpgradePlan(appliances []openapi.Appliance, stats *openapi.StatsAppliancesList, adminHostname string, filter map[string]map[string]string, orderBy []string, descending bool) (*UpgradePlan, error) {
+func NewUpgradePlan(appliances []openapi.Appliance, stats *openapi.StatsAppliancesList, upgradeStatusMap map[string]UpgradeStatusResult, adminHostname string, filter map[string]map[string]string, orderBy []string, descending bool) (*UpgradePlan, error) {
 	plan := UpgradePlan{
 		adminHostname: adminHostname,
 		stats:         stats,
@@ -123,15 +123,21 @@ func NewUpgradePlan(appliances []openapi.Appliance, stats *openapi.StatsApplianc
 		}
 
 		// Get upgrade status and target version
-		upgradeStatus := stats.GetUpgrade()
-		if status := upgradeStatus.GetStatus(); status != UpgradeStatusReady {
+		upgradeStatus, ok := upgradeStatusMap[a.GetId()]
+		if !ok {
+			plan.Skipping = append(plan.Skipping, SkipUpgrade{
+				Appliance: a,
+				Reason:    ErrNoApplianceStats,
+			})
+		}
+		if upgradeStatus.Status != UpgradeStatusReady {
 			plan.Skipping = append(plan.Skipping, SkipUpgrade{
 				Appliance: a,
 				Reason:    ErrSkipReasonNotPrepared,
 			})
 			continue
 		}
-		targetVersion, err := ParseVersionString(upgradeStatus.GetDetails())
+		targetVersion, err := ParseVersionString(upgradeStatus.Details)
 		if err != nil {
 			errs = multierror.Append(errs, err)
 			plan.Skipping = append(plan.Skipping, SkipUpgrade{

diff --git a/pkg/appliance/upgrade_test.go b/pkg/appliance/upgrade_test.go
@@ -163,7 +163,21 @@ func TestMakeUpgradePlan(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := NewUpgradePlan(tt.args.appliances, tt.args.stats, tt.args.ctrlHostname, tt.args.filter, tt.args.orderBy, tt.args.descending)
+			upgradeStatusMap := map[string]UpgradeStatusResult{}
+			for _, a := range tt.args.appliances {
+				for _, s := range tt.args.stats.GetData() {
+					if a.GetId() != s.GetId() {
+						continue
+					}
+					us := s.GetUpgrade()
+					upgradeStatusMap[a.GetId()] = UpgradeStatusResult{
+						Name:    a.GetName(),
+						Status:  us.GetStatus(),
+						Details: us.GetDetails(),
+					}
+				}
+			}
+			got, err := NewUpgradePlan(tt.args.appliances, tt.args.stats, upgradeStatusMap, tt.args.ctrlHostname, tt.args.filter, tt.args.orderBy, tt.args.descending)
 			if tt.wantErr {
 				assert.Error(t, err)
 			}
@@ -339,10 +353,27 @@ Appliances that will be skipped:
 		t.Run(tt.name, func(t *testing.T) {
 			coll := GenerateCollective(t, tt.in.hostname, tt.in.from, tt.in.to, tt.in.Appliances)
 			appliances := make([]openapi.Appliance, 0, len(tt.in.Appliances))
+			upgradeStatusMap := map[string]UpgradeStatusResult{}
 			for _, v := range tt.in.Appliances {
-				appliances = append(appliances, coll.Appliances[v])
+				appliance, ok := coll.Appliances[v]
+				if !ok {
+					t.Fatalf("internal testing error: appliance name does not match any appliance")
+					return
+				}
+				for _, stat := range coll.Stats.GetData() {
+					if stat.GetId() != appliance.GetId() {
+						continue
+					}
+					us := stat.GetUpgrade()
+					upgradeStatusMap[appliance.GetId()] = UpgradeStatusResult{
+						Name:    appliance.GetName(),
+						Status:  us.GetStatus(),
+						Details: us.GetDetails(),
+					}
+				}
+				appliances = append(appliances, appliance)
 			}
-			up, err := NewUpgradePlan(appliances, coll.Stats, tt.in.hostname, tt.in.filter, tt.in.orderBy, tt.in.descending)
+			up, err := NewUpgradePlan(appliances, coll.Stats, upgradeStatusMap, tt.in.hostname, tt.in.filter, tt.in.orderBy, tt.in.descending)
 			if err != nil {
 				t.Fatalf("internal test error: %v", err)
 			}
@@ -417,10 +448,23 @@ WARNING: Upgrade was completed, but not all appliances are running the same vers
 			hostname := "appgate.test"
 			coll := GenerateCollective(t, hostname, tt.from, tt.to, tt.appliances)
 			appliances := make([]openapi.Appliance, 0, len(tt.appliances))
+			upgradeStatusMap := map[string]UpgradeStatusResult{}
 			for _, v := range tt.appliances {
-				appliances = append(appliances, coll.Appliances[v])
+				a := coll.Appliances[v]
+				for _, s := range coll.Stats.GetData() {
+					if s.GetId() != a.GetId() {
+						continue
+					}
+					us := s.GetUpgrade()
+					upgradeStatusMap[a.GetId()] = UpgradeStatusResult{
+						Name:    a.GetName(),
+						Status:  us.GetStatus(),
+						Details: us.GetDetails(),
+					}
+				}
+				appliances = append(appliances, a)
 			}
-			up, err := NewUpgradePlan(appliances, coll.Stats, hostname, DefaultCommandFilter, nil, false)
+			up, err := NewUpgradePlan(appliances, coll.Stats, upgradeStatusMap, hostname, DefaultCommandFilter, nil, false)
 			if err != nil {
 				t.Fatalf("PrintPostCompleteSummary() - internal test error: %v", err)
 				return