do validation seperately

appgate · Jul 2, 2024 · 684448a · 684448a
1 parent b82dda5
commit 684448a
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 66 deletions.
diff --git a/cmd/appliance/upgrade/complete.go b/cmd/appliance/upgrade/complete.go
@@ -212,6 +212,9 @@ func upgradeCompleteRun(cmd *cobra.Command, args []string, opts *upgradeComplete
 	}
 	primaryController := plan.GetPrimaryController()
 	plan.AddOfflineAppliances(offline)
+	if err := plan.Validate(); err != nil {
+		return err
+	}
 	bOpts := appliancepkg.BackupOpts{
 		Config:        opts.Config,
 		Appliance:     opts.Appliance,

diff --git a/pkg/appliance/upgrade.go b/pkg/appliance/upgrade.go
@@ -49,12 +49,14 @@ type UpgradePlan struct {
 	stats                   *openapi.StatsAppliancesList
 	adminHostname           string
 	primary                 *openapi.Appliance
+	allAppliances           []openapi.Appliance
 }
 
 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,
+		allAppliances: appliances,
 	}
 
 	primary, err := FindPrimaryController(appliances, plan.adminHostname, false)
@@ -63,11 +65,6 @@ func NewUpgradePlan(appliances []openapi.Appliance, stats *openapi.StatsApplianc
 	}
 	plan.primary = primary
 
-	// we check if all controllers need upgrade very early
-	if _, err := CheckNeedsMultiControllerUpgrade(stats, appliances); err != nil {
-		return nil, err
-	}
-
 	finalApplianceList, filtered, err := FilterAppliances(appliances, filter, orderBy, descending)
 	if err != nil {
 		return nil, err
@@ -287,12 +284,21 @@ func (up *UpgradePlan) AddOfflineAppliances(appliances []openapi.Appliance) {
 			Reason:    ErrSkipReasonOffline,
 		})
 	}
+	up.allAppliances = append(up.allAppliances, appliances...)
 }
 
 func (up *UpgradePlan) GetPrimaryController() *openapi.Appliance {
 	return up.primary
 }
 
+func (up *UpgradePlan) Validate() error {
+	// we check if all controllers need upgrade very early
+	if _, err := CheckNeedsMultiControllerUpgrade(up.stats, up.allAppliances); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (up *UpgradePlan) HasDiffVersions(newStats []openapi.StatsAppliancesListAllOfData) (bool, map[string]string) {
 	res := map[string]string{}
 	versionList := []string{}

diff --git a/pkg/appliance/upgrade_test.go b/pkg/appliance/upgrade_test.go
@@ -63,8 +63,6 @@ func TestMakeUpgradePlan(t *testing.T) {
 					{coll.Appliances["gatewayA2"], coll.Appliances["gatewayB2"], coll.Appliances["gatewayC2"], coll.Appliances["logforwarderA2"]},
 					{coll.Appliances["connectorA1"], coll.Appliances["gatewayA3"], coll.Appliances["logserver"], coll.Appliances["portalA1"]},
 				},
-				adminHostname: hostname,
-				stats:         coll.Stats,
 			},
 		},
 		{
@@ -100,66 +98,8 @@ func TestMakeUpgradePlan(t *testing.T) {
 					{coll.Appliances["gatewayA2"], coll.Appliances["gatewayB2"], coll.Appliances["gatewayC2"], coll.Appliances["logforwarderA2"]},
 					{coll.Appliances["connectorA1"], coll.Appliances["gatewayA3"], coll.Appliances["logserver"], coll.Appliances["portalA1"]},
 				},
-				adminHostname: hostname,
-				stats:         coll.Stats,
 			},
 		},
-		{
-			name: "test multi controller upgrade error",
-			args: args{
-				appliances: []openapi.Appliance{
-					coll.Appliances["primary"],
-					coll.Appliances["controller3"],
-					coll.Appliances["gatewayA1"],
-					coll.Appliances["gatewayB2"],
-					coll.Appliances["gatewayA2"],
-					coll.Appliances["logserver"],
-					coll.Appliances["logforwarderA2"],
-					coll.Appliances["gatewayB1"],
-					coll.Appliances["connectorA1"],
-					coll.Appliances["gatewayC1"],
-					coll.Appliances["secondary"],
-					coll.Appliances["gatewayA3"],
-					coll.Appliances["gatewayC2"],
-					coll.Appliances["portalA1"],
-					coll.Appliances["logforwarderA1"],
-				},
-				stats:        coll.Stats,
-				ctrlHostname: hostname,
-				filter:       DefaultCommandFilter,
-				orderBy:      nil,
-				descending:   false,
-			},
-			wantErr: true,
-		},
-		{
-			name: "test offline controller",
-			args: args{
-				appliances: []openapi.Appliance{
-					coll.Appliances["primary"],
-					coll.Appliances["controller4"],
-					coll.Appliances["gatewayA1"],
-					coll.Appliances["gatewayB2"],
-					coll.Appliances["gatewayA2"],
-					coll.Appliances["logserver"],
-					coll.Appliances["logforwarderA2"],
-					coll.Appliances["gatewayB1"],
-					coll.Appliances["connectorA1"],
-					coll.Appliances["gatewayC1"],
-					coll.Appliances["secondary"],
-					coll.Appliances["gatewayA3"],
-					coll.Appliances["gatewayC2"],
-					coll.Appliances["portalA1"],
-					coll.Appliances["logforwarderA1"],
-				},
-				stats:        coll.Stats,
-				ctrlHostname: hostname,
-				filter:       DefaultCommandFilter,
-				orderBy:      nil,
-				descending:   false,
-			},
-			wantErr: true,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -181,7 +121,7 @@ func TestMakeUpgradePlan(t *testing.T) {
 			if tt.wantErr {
 				assert.Error(t, err)
 			}
-			assert.Equal(t, tt.want, got)
+			assert.EqualExportedValues(t, tt.want, got)
 		})
 	}
 }