online upgrade with package installation and pending pods #961

qindotguan · 2024-10-17T11:35:11Z

Fixes two issues during the online upgrade process:

Try to restart the pending nodes and report a warning event to remind the user to check the DB configuration.
To print a hint that package installation failure may be due to the lack of memory resources.

An e2e test was added to cover the test with package installation and a pending pod.

roypaulin · 2024-10-17T13:08:02Z

pkg/controllers/vdb/onlineupgrade_reconciler.go

+
+	mainPFacts := r.PFacts[vapi.MainCluster]
+	// All nodes in the main cluster must be up before running online upgrade
+	if mainPFacts.getUpNodeCount() != len(mainPFacts.Detail) {


We want all the pods to be running before starting online upgrade. They don't all have to be up.

It seems not working after changed to countRunningAndInstalled(). When the pending nodes just became running (not up yet), the upgrade process continued to duplicate the main cluster but returned VAddnode error. That running node and the new added nodes never came up.

In that case, don't even check the number of upnode. Check if there is any not running pod and requeue if there is. Then Just call the restart reconciler, if any node is down it will start it, if no node is down it will do nothing.

Thanks for the instruction. It is updated accordingly.

roypaulin · 2024-10-18T11:35:29Z

pkg/controllers/vdb/onlineupgrade_reconciler.go

+	mainPFacts := r.PFacts[vapi.MainCluster]
+	// All nodes in the main cluster must be up before running online upgrade
+	if mainPFacts.getUpNodeCount() != len(mainPFacts.Detail) {
+		r.VRec.Eventf(r.VDB, corev1.EventTypeWarning, events.NotAllNodesUp,


You don't need an event. Just log something.

HaoYang0000 · 2024-10-21T08:16:03Z

pkg/vadmin/install_packages_at.go

@@ -34,7 +35,8 @@ func (a *Admintools) InstallPackages(ctx context.Context, opts ...installpackage

 	status := genInstallPackageStatus(stdout)
 	if err != nil && len(status.Packages) == 0 {
-		_, logErr := a.logFailure("install_package", events.InstallPackagesFailed, stdout, err)
+		pkgErr := errors.New(err.Error() + "This may due to lack of memory resources.")


Do we have a way to distinguish the cause of the error? Simply adding a string here can be misleading for future debugging.

Thanks. Now it only logs a message if 0 package was installed.

roypaulin · 2024-10-22T16:41:55Z

pkg/controllers/vdb/upgrade.go

 		i.Rec.Eventf(i.Vdb, corev1.EventTypeNormal, events.IncompatibleUpgradeRequested,
 			"Requested upgrade is incompatible with the Vertica deployment. Falling back to %s upgrade.", actualUpgradeAsText)
+
+		if i.Vdb.Spec.UpgradePolicy == "Online" {


Let's move this before the event recorder.

roypaulin · 2024-10-22T18:01:26Z

The main now has the lint fix.

roypaulin · 2024-10-23T12:29:31Z

pkg/controllers/vdb/onlineupgrade_reconciler.go

@@ -165,6 +165,8 @@ func (r *OnlineUpgradeReconciler) Reconcile(ctx context.Context, _ *ctrl.Request

 	// Functions to perform when the image changes.  Order matters.
 	funcs := []func(context.Context) (ctrl.Result, error){
+		// Requeue if not all nodes are running
+		r.requeuePodsNotRunning,


Add a status msg in onlineUpgradeStatusMsgs for this step.

Let's put them after postStartOnlineUpgradeMsg

roypaulin · 2024-10-23T12:35:40Z

tests/e2e-leg-9/online-upgrade-with-package-install/02-assert.yaml

This new test does too many things already validated by other tests. I think what you want to check is that we do not pass the first function in online upgrade if some pods are pending so make some pod pending, start online upgrade, wait a little and check the upgradeStatus to verify that we are still stuck in that function, make the pending pods run and now wait until we create the new subclusters and the test can end there.

Good suggestion. I removed the duplicated checks and renamed the test to online-upgrade-pods-pending.

…ding

roypaulin · 2024-10-24T15:32:36Z

pkg/controllers/vdb/onlineupgrade_reconciler.go

@@ -165,6 +165,8 @@ func (r *OnlineUpgradeReconciler) Reconcile(ctx context.Context, _ *ctrl.Request

 	// Functions to perform when the image changes.  Order matters.
 	funcs := []func(context.Context) (ctrl.Result, error){
+		// Requeue if not all nodes are running
+		r.requeuePodsNotRunning,


Let's put them after postStartOnlineUpgradeMsg

roypaulin · 2024-10-24T15:37:08Z

tests/e2e-leg-9/online-upgrade-pods-pending/26-assert.yaml

+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: v-base-upgrade-sec2
+status:
+  availableReplicas: 0
+  replicas: 1


It does not necessarily mean the pods are pending. You need to validate the pod itself:

Suggested change

apiVersion: apps/v1

kind: StatefulSet

metadata:

name: v-base-upgrade-sec2

status:

availableReplicas: 0

replicas: 1

apiVersion: v1

kind: Pod

metadata:

name: v-base-upgrade-sec2-0

status:

phase: pending

Thanks. This is helpful.

roypaulin · 2024-10-24T15:38:27Z

tests/e2e-leg-9/online-upgrade-pods-pending/29-set-pod-running.yaml

Add an assert file where you check if the pod is running

roypaulin · 2024-10-24T15:44:37Z

tests/e2e-leg-9/online-upgrade-pods-pending/27-initiate-upgrade.yaml

Add an assert that checks the upgradeStatus:

apiVersion: v1 kind: Pod metadata: name: v-base-upgrade-sec2-0 status: phase: pending --- apiVersion: vertica.com/v1beta1 kind: VerticaDB metadata: name: v-base-upgrade status: upgradeStatus: "Requeue as pods not running"

Updated according to the comments. Please help to review again.

roypaulin · 2024-10-28T11:13:52Z

tests/e2e-leg-9/online-upgrade-pods-pending/setup-vdb/base/setup-vdb.yaml

@@ -24,7 +24,7 @@ metadata:
 spec:
  image: kustomize-vertica-image
  dbName: repUP
-  initPolicy: Create
+  initPolicy: CreateSkipPackageInstall


As the test online-upgrade-pods-pending stops at the steps of wait-for-scs-mimic, the package installation (at the step of a new Vertica version is installed) is not covered in the test. So we can either

run the test with CreateSkipPackageInstall

add the upgrade steps back to cover the package installation

qindotguan added 11 commits October 14, 2024 06:22

make sure all nodes are up for online upgrade

82cf6ac

add checkNodesUp into the online upgrade reconciler

2c41466

online upgrade with package install and a pending pod

a90a8c2

pending pods donnot have currentReplicas

6111835

add step to let pod running

a24e665

restart the main cluster if not all nodes are up

03bd47d

add hint message if package installation failed

521a504

update the event message

5e036bb

remove the event checking of SandboxSubclusterStart

f8b3273

Merge branch 'main' into qguan/online-upgrade-packages

cc711b9

update the expected sandbox and subcluster list

9e39dc9

qindotguan requested review from roypaulin, cchen-vertica, fenic-fawkes, HaoYang0000 and LiboYu2 as code owners October 17, 2024 11:35

roypaulin reviewed Oct 17, 2024

View reviewed changes

reorder the subcluster list in the test assert files

6a5957f

roypaulin reviewed Oct 18, 2024

View reviewed changes

HaoYang0000 reviewed Oct 21, 2024

View reviewed changes

qindotguan added 5 commits October 21, 2024 14:47

requeue if found pods not running

4771f75

use log but not event

4efd782

get pods not in running status

ba02231

log error message only if no pacakge installed

1324be0

Merge branch 'main' into qguan/online-upgrade-packages

dbda6e9

roypaulin reviewed Oct 22, 2024

View reviewed changes

qindotguan added 2 commits October 22, 2024 14:33

Merge branch 'main' into qguan/online-upgrade-packages

1d62c4b

move online upgrade fall back log before the event

03ffeb9

roypaulin reviewed Oct 23, 2024

View reviewed changes

qindotguan added 6 commits October 23, 2024 17:04

Add requeuePodsNotRunningMsg in onlineUpgradeStatusMsgs

87b65e1

remove duplicated checks in other tests

0d2c12e

rename online-upgrade-with-package-install to online-upgrade-pods-pen…

fbacb19

…ding

Merge branch 'main' into qguan/online-upgrade-packages

8fad603

revert the change to the devcluster

a668a0f

remove extra dot in finish package installation msg

031998c

roypaulin reviewed Oct 24, 2024

View reviewed changes

qindotguan added 4 commits October 25, 2024 04:05

move requeue pending after postStartOnlineUpgradeMsg

bb882b4

validate pod pending and requeue status

e1106a4

update pending

bd814df

test with initPolicy CreateSkipPackageInstall

ef2164c

roypaulin reviewed Oct 28, 2024

View reviewed changes

qindotguan added 3 commits October 29, 2024 05:50

Merge branch 'main' into qguan/online-upgrade-packages

2a3f068

export anyPodsNotRunning as podfacts moved to a separated pacakge

e6c8398

rename anyPodsNotRunning to AnyPodsNotRunning

60fb071

roypaulin approved these changes Oct 30, 2024

View reviewed changes

qindotguan merged commit a3abb8e into main Oct 30, 2024
39 checks passed

qindotguan deleted the qguan/online-upgrade-packages branch October 30, 2024 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

online upgrade with package installation and pending pods #961

online upgrade with package installation and pending pods #961

qindotguan commented Oct 17, 2024

roypaulin Oct 17, 2024

qindotguan Oct 18, 2024

roypaulin Oct 18, 2024 •

edited

Loading

qindotguan Oct 21, 2024

roypaulin Oct 18, 2024

HaoYang0000 Oct 21, 2024

qindotguan Oct 21, 2024

roypaulin Oct 22, 2024

roypaulin commented Oct 22, 2024

roypaulin Oct 23, 2024

qindotguan Oct 23, 2024

roypaulin Oct 24, 2024

roypaulin Oct 23, 2024

qindotguan Oct 23, 2024

roypaulin Oct 24, 2024

roypaulin Oct 24, 2024

qindotguan Oct 25, 2024

roypaulin Oct 24, 2024

roypaulin Oct 24, 2024

qindotguan Oct 25, 2024

roypaulin Oct 28, 2024

qindotguan Oct 29, 2024 •

edited

Loading

online upgrade with package installation and pending pods #961

online upgrade with package installation and pending pods #961

Conversation

qindotguan commented Oct 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roypaulin Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roypaulin commented Oct 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qindotguan Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

roypaulin Oct 18, 2024 •

edited

Loading

qindotguan Oct 29, 2024 •

edited

Loading