Code cleanup

spilchen · Mar 21, 2022 · e4be224 · e4be224
1 parent 3571584
commit e4be224
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 13 deletions.
diff --git a/pkg/builder/labels_annotations.go b/pkg/builder/labels_annotations.go
@@ -145,7 +145,8 @@ func MakeBaseSvcSelectorLabels(vdb *vapi.VerticaDB) map[string]string {
 func MakeSvcSelectorLabelsForServiceNameRouting(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string]string {
 	m := MakeBaseSvcSelectorLabels(vdb)
 	m[SubclusterSvcNameLabel] = sc.GetServiceName()
-	// Only route to nodes that have verified they own at least one shard
+	// Only route to nodes that have verified they own at least one shard and
+	// aren't pending delete
 	m[ClientRoutingLabel] = ClientRoutingVal
 	return m
 }
@@ -154,7 +155,7 @@ func MakeSvcSelectorLabelsForServiceNameRouting(vdb *vapi.VerticaDB, sc *vapi.Su
 // we want a service object to pick the pods based on the subcluster name.
 func MakeSvcSelectorLabelsForSubclusterNameRouting(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string]string {
 	m := MakeBaseSvcSelectorLabels(vdb)
-	// Routing is done solely with the subcluster name.
+	// Routing is done using the subcluster name rather than the service name.
 	m[SubclusterNameLabel] = sc.Name
 	m[ClientRoutingLabel] = ClientRoutingVal
 

diff --git a/pkg/controllers/offlineupgrade_reconcile.go b/pkg/controllers/offlineupgrade_reconcile.go
@@ -61,7 +61,7 @@ func MakeOfflineUpgradeReconciler(vdbrecon *VerticaDBReconciler, log logr.Logger
 	vdb *vapi.VerticaDB, prunner cmds.PodRunner, pfacts *PodFacts) ReconcileActor {
 	return &OfflineUpgradeReconciler{VRec: vdbrecon, Log: log, Vdb: vdb, PRunner: prunner, PFacts: pfacts,
 		Finder:  iter.MakeSubclusterFinder(vdbrecon.Client, vdb),
-		Manager: *MakeUpgradeManager(vdbrecon, log, vdb, pfacts, vapi.OfflineUpgradeInProgress, offlineUpgradeAllowed),
+		Manager: *MakeUpgradeManager(vdbrecon, log, vdb, vapi.OfflineUpgradeInProgress, offlineUpgradeAllowed),
 	}
 }
 

diff --git a/pkg/controllers/onlineupgrade_reconciler.go b/pkg/controllers/onlineupgrade_reconciler.go
@@ -54,7 +54,7 @@ func MakeOnlineUpgradeReconciler(vdbrecon *VerticaDBReconciler, log logr.Logger,
 	vdb *vapi.VerticaDB, prunner cmds.PodRunner, pfacts *PodFacts) ReconcileActor {
 	return &OnlineUpgradeReconciler{VRec: vdbrecon, Log: log, Vdb: vdb, PRunner: prunner, PFacts: pfacts,
 		Finder:  iter.MakeSubclusterFinder(vdbrecon.Client, vdb),
-		Manager: *MakeUpgradeManager(vdbrecon, log, vdb, pfacts, vapi.OnlineUpgradeInProgress, onlineUpgradeAllowed),
+		Manager: *MakeUpgradeManager(vdbrecon, log, vdb, vapi.OnlineUpgradeInProgress, onlineUpgradeAllowed),
 	}
 }
 

diff --git a/pkg/controllers/upgrade.go b/pkg/controllers/upgrade.go
@@ -37,7 +37,6 @@ type UpgradeManager struct {
 	VRec              *VerticaDBReconciler
 	Vdb               *vapi.VerticaDB
 	Log               logr.Logger
-	PFacts            *PodFacts
 	Finder            iter.SubclusterFinder
 	ContinuingUpgrade bool // true if UpdateInProgress was already set upon entry
 	StatusCondition   vapi.VerticaDBConditionType
@@ -47,13 +46,12 @@ type UpgradeManager struct {
 
 // MakeUpgradeManager will construct a UpgradeManager object
 func MakeUpgradeManager(vdbrecon *VerticaDBReconciler, log logr.Logger, vdb *vapi.VerticaDB,
-	pfacts *PodFacts, statusCondition vapi.VerticaDBConditionType,
+	statusCondition vapi.VerticaDBConditionType,
 	isAllowedForUpgradePolicyFunc func(vdb *vapi.VerticaDB) bool) *UpgradeManager {
 	return &UpgradeManager{
 		VRec:                          vdbrecon,
 		Vdb:                           vdb,
 		Log:                           log,
-		PFacts:                        pfacts,
 		Finder:                        iter.MakeSubclusterFinder(vdbrecon.Client, vdb),
 		StatusCondition:               statusCondition,
 		IsAllowedForUpgradePolicyFunc: isAllowedForUpgradePolicyFunc,

diff --git a/pkg/controllers/upgrade_test.go b/pkg/controllers/upgrade_test.go
@@ -85,7 +85,7 @@ var _ = Describe("upgrade", func() {
 		test.CreatePods(ctx, k8sClient, vdb, test.AllPodsRunning)
 		defer test.DeletePods(ctx, k8sClient, vdb)
 
-		mgr := MakeUpgradeManager(vdbRec, logger, vdb, nil, vapi.OnlineUpgradeInProgress,
+		mgr := MakeUpgradeManager(vdbRec, logger, vdb, vapi.OnlineUpgradeInProgress,
 			func(vdb *vapi.VerticaDB) bool { return true })
 		Expect(mgr.IsUpgradeNeeded(ctx)).Should(Equal(false))
 	})
@@ -103,7 +103,7 @@ var _ = Describe("upgrade", func() {
 		test.CreateVDB(ctx, k8sClient, vdb)
 		defer test.DeleteVDB(ctx, k8sClient, vdb)
 
-		mgr := MakeUpgradeManager(vdbRec, logger, vdb, nil, vapi.OfflineUpgradeInProgress,
+		mgr := MakeUpgradeManager(vdbRec, logger, vdb, vapi.OfflineUpgradeInProgress,
 			func(vdb *vapi.VerticaDB) bool { return true })
 		Expect(mgr.IsUpgradeNeeded(ctx)).Should(Equal(true))
 		stsChange, res, err := mgr.updateImageInStatefulSets(ctx)
@@ -128,7 +128,7 @@ var _ = Describe("upgrade", func() {
 		defer test.DeletePods(ctx, k8sClient, vdb)
 		vdb.Spec.Image = NewImage // Change image to force pod deletion
 
-		mgr := MakeUpgradeManager(vdbRec, logger, vdb, nil, vapi.OfflineUpgradeInProgress,
+		mgr := MakeUpgradeManager(vdbRec, logger, vdb, vapi.OfflineUpgradeInProgress,
 			func(vdb *vapi.VerticaDB) bool { return true })
 		numPodsDeleted, err := mgr.deletePodsRunningOldImage(ctx, "") // pods from primaries only
 		Expect(err).Should(Succeed())
@@ -149,7 +149,7 @@ var _ = Describe("upgrade", func() {
 		defer test.DeletePods(ctx, k8sClient, vdb)
 		vdb.Spec.Image = NewImage // Change image to force pod deletion
 
-		mgr := MakeUpgradeManager(vdbRec, logger, vdb, nil, vapi.OfflineUpgradeInProgress,
+		mgr := MakeUpgradeManager(vdbRec, logger, vdb, vapi.OfflineUpgradeInProgress,
 			func(vdb *vapi.VerticaDB) bool { return true })
 		numPodsDeleted, err := mgr.deletePodsRunningOldImage(ctx, vdb.Spec.Subclusters[1].Name)
 		Expect(err).Should(Succeed())
@@ -172,7 +172,7 @@ var _ = Describe("upgrade", func() {
 		defer test.DeleteVDB(ctx, k8sClient, vdb)
 		vdb.Spec.Image = NewImage // Change image to force pod deletion
 
-		mgr := MakeUpgradeManager(vdbRec, logger, vdb, nil, vapi.OfflineUpgradeInProgress,
+		mgr := MakeUpgradeManager(vdbRec, logger, vdb, vapi.OfflineUpgradeInProgress,
 			func(vdb *vapi.VerticaDB) bool { return true })
 		Expect(mgr.startUpgrade(ctx)).Should(Equal(ctrl.Result{}))
 		Expect(mgr.setUpgradeStatus(ctx, "doing the change")).Should(Succeed())
@@ -198,7 +198,7 @@ var _ = Describe("upgrade", func() {
 
 		statusMsgs := []string{"msg1", "msg2", "msg3"}
 
-		mgr := MakeUpgradeManager(vdbRec, logger, vdb, nil, vapi.OfflineUpgradeInProgress,
+		mgr := MakeUpgradeManager(vdbRec, logger, vdb, vapi.OfflineUpgradeInProgress,
 			func(vdb *vapi.VerticaDB) bool { return true })
 		Expect(mgr.postNextStatusMsg(ctx, statusMsgs, 1)).Should(Succeed()) // no-op