PWX-30419: Use reconcile loop to check pre-flight status. Timeout after 15m. #1070

jrivera-px · 2023-06-09T18:32:39Z

What this PR does / why we need it:
When executing pre-flight check we would wait for the checks to complete blocking the reconcile loop. This prevented any other state change from being processed. So this change corrected that. Use cluster conditions to determine if we are still executing a pre-flight. If so check the status each time though the loop. Add a 15 min timeout based on the time the DS has been running.
Which issue(s) this PR fixes (optional)
Closes #
PWX-30419
Special notes for your reviewer:

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

…WX-28826

* Updating CSV to use 23.3.1 released image * Update for 23.3.1 release * Controller gen vendor Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> * PWX-29389 Add CRD for portworx diags collection Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> * PWX-29409: Ignore zones with no nodes (#1008) In disaggregated mode, there could be zones in which no storage nodes might be present. Such a zone would make the maxSNPZ value to be 0. CHanging the behavior to ignore 0 nodes in a zone for maxSNPZ calculation. Signed-off-by: Naveen Revanna <nrevanna@purestorage.com> --------- Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Signed-off-by: Naveen Revanna <nrevanna@purestorage.com> Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com> Co-authored-by: Jiafeng Liao <jliao@purestorage.com> Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

…emove 'wait' code. Signed-off-by: Jose Rivera <jose@portworx.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

…that don't work since Validate was removed from the controller.validate() func. PWX-30373 to try and fix later. Signed-off-by: Jose Rivera <jose@portworx.com>

… check failure to trigger the needed workflow. Signed-off-by: Jose Rivera <jose@portworx.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

…space. Signed-off-by: Jose Rivera <jose@portworx.com>

…running CBT namespace. Signed-off-by: Jose Rivera <jose@portworx.com>

…ctionality correctly. Signed-off-by: Jose Rivera <jose@portworx.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

… exists. Signed-off-by: Jose Rivera <jose@portworx.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

piyush-nimbalkar · 2023-07-26T21:56:38Z

pkg/controller/storagecluster/storagecluster.go

+
+	// Skip over driver Validate() if an error has occurred
+	if err == nil {
+		err = c.driverValidate(toUpdate)


Can we return an cluster condition constant showing the state of the preflight pods?

piyush-nimbalkar · 2023-07-26T21:57:02Z

pkg/controller/storagecluster/storagecluster.go

+		if toUpdate.Annotations[pxutil.AnnotationPreflightCheck] != "false" { // In progress
+			condition.Status = corev1.ClusterConditionStatusInProgress
+			logrus.Infof("storage cluster preflight in progress...")
+
+		} else {
+			condition.Status = corev1.ClusterConditionStatusCompleted
+		}


Update the condition based on the output from driverValidate.

piyush-nimbalkar · 2023-07-26T22:00:36Z

pkg/controller/storagecluster/storagecluster.go

@@ -412,55 +412,59 @@ func (c *Controller) validateCloudStorageLabelKey(cluster *corev1.StorageCluster
 	return nil
 }

-func (c *Controller) runPreflightCheck(cluster *corev1.StorageCluster) error {
+func bringUpStorageCluster(cluster *corev1.StorageCluster) (bool, error) {


We don't need to return an error here.

piyush-nimbalkar · 2023-07-26T22:05:55Z

pkg/controller/storagecluster/storagecluster.go

@@ -657,6 +714,15 @@ func (c *Controller) syncStorageCluster(
 		}
 	}

+	if startOk, err := bringUpStorageCluster(cluster); err == nil {


nit: isPreflightComplete?

piyush-nimbalkar · 2023-07-26T22:17:15Z

drivers/storage/portworx/portworx.go

+	if err != nil {
+		if errors.IsNotFound(err) {
+			condition.Status = corev1.ClusterConditionStatusInProgress
+			condition.Message = "pre-flight: in progress..."


*Preflight pods haven't started yet.

Signed-off-by: Jose Rivera <jose@portworx.com>

…termin status.. Signed-off-by: Jose Rivera <jose@portworx.com>

…ault condition on CloudDrivePermission error. Signed-off-by: Jose Rivera <jose@portworx.com>

piyush-nimbalkar

@jrivera-px can we UTs for these changes either in this PR or a different one?

piyush-nimbalkar · 2023-08-04T17:44:49Z

drivers/storage/portworx/portworx.go

+	if total != 0 && completed == total {
+		logrus.Infof("pre-flight: checks completed...")
+	} else {
+		if age >= 15*time.Minute {


nit: Extract the duration as a constant?

piyush-nimbalkar · 2023-08-04T17:53:01Z

pkg/controller/storagecluster/storagecluster.go

@@ -842,7 +920,7 @@ func (c *Controller) gcNeeded(obj client.Object) bool {

 	if obj.GetObjectKind().GroupVersionKind().Kind == "ConfigMap" {
 		if obj.GetName() == "px-attach-driveset-lock" ||
-			strings.HasPrefix(obj.GetName(), "px-bringup-queue-lock") {
+			strings.HasPrefix(obj.GetName(), "pxb-ringup-queue-lock") {


I assume this is a typo.

yeah transpose the chars on accident..

Signed-off-by: Jose Rivera <jose@portworx.com>

…imeout. Signed-off-by: Jose Rivera <jose@portworx.com>

Signed-off-by: Jose Rivera <jose@portworx.com>

…er 15m. (#1070) * PWX-28826 Boilerplace Signed-off-by: Harsh Desai <hadesai@purestorage.com> * more boilerplate Signed-off-by: Harsh Desai <hadesai@purestorage.com> * PWX-28826: Pre-flight check for DMthin. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Add comments and move StorageNode cleanup. Signed-off-by: Jose Rivera <jose@portworx.com> * Passed checks should be Info events. Signed-off-by: Jose Rivera <jose@portworx.com> * Passed checks should be Info events. (#1010) Signed-off-by: Jose Rivera <jose@portworx.com> * Pwx 28826 (#1011) * Pwx 28826 (#1012) * PWX-28826: Update with the latest master changes. (#1013) * Updating CSV to use 23.3.1 released image * Update for 23.3.1 release * Controller gen vendor Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> * PWX-29389 Add CRD for portworx diags collection Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> * PWX-29409: Ignore zones with no nodes (#1008) In disaggregated mode, there could be zones in which no storage nodes might be present. Such a zone would make the maxSNPZ value to be 0. CHanging the behavior to ignore 0 nodes in a zone for maxSNPZ calculation. Signed-off-by: Naveen Revanna <nrevanna@purestorage.com> --------- Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Signed-off-by: Naveen Revanna <nrevanna@purestorage.com> Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com> Co-authored-by: Jiafeng Liao <jliao@purestorage.com> Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com> * Add PassPreFlight event tag and logging Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Check status of portworx container in pre-flight pod and remove 'wait' code. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Fix unit test. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Fix unit test. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: PR review changes and fix portworx_test.go UTs Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: fix gomack Validate calls. Also comment out the two tests that don't work since Validate was removed from the controller.validate() func. PWX-30373 to try and fix later. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30373: Re-add back in the commented out tests and add K8s version check failure to trigger the needed workflow. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Exit pre-check wait if running CBT namespace. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Add 5 min timeout to pre-flight status check. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Exit GetPreFlightStatus() with success if running CBT namespace. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Don't automatically enable dmthin via pre-flight check if running CBT namespace. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30373: Revert UT and integration test hacks. Need to mock the functionality correctly. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Increase pre-flight daemonset ready wait to 10mins. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: fix 'TestValidate' UT. Don't error if pre-flight daemonset exists. Signed-off-by: Jose Rivera <jose@portworx.com> * Use reconcile loop to check pre-flight status. Timeout after 15m. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Startup check was not handling if preflight was not enabled. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Re-enable preflight for integration tests. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: disable preflight for integration tests. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Only do preflight if AWS. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30496: if '-T dmthin' exists in stc before preflight is ran and preflight fails don't start. Add default metadata dev for dmthin if it does not exist. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Use condition set to determine of CheckCloudDrivePermission check is to be run. Handle skip annotation and pod cleanup. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Remove debug logging Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30419: Remove more debug logging. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30419: Revert to correct version. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30419: Fix unit test. Need to enable pre-flight. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30419: Fix TestEKSPreflightCheck unit test. Pre-flight now includes DMthin checks so need to handle that. Signed-off-by: Jose Rivera <jose@portworx.com> * update from master, conflict fix. * PWX-28826: Use cluster condition to determine pre-flight status. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: return cluster condition from driverValidate as that to determin status.. Signed-off-by: Jose Rivera <jose@portworx.com> * Add err message to default condition message and make sure to use default condition on CloudDrivePermission error. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Add a constant for pre-flight timeout value. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: pre-flight timeout should prevent startup. Add event for timeout. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Fix unit tests. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-28826: Use progress string. Signed-off-by: Jose Rivera <jose@portworx.com> --------- Signed-off-by: Harsh Desai <hadesai@purestorage.com> Signed-off-by: Jose Rivera <jose@portworx.com> Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Signed-off-by: Naveen Revanna <nrevanna@purestorage.com> Co-authored-by: Harsh Desai <hadesai@purestorage.com> Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com> Co-authored-by: Jiafeng Liao <jliao@purestorage.com> Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>

…er 15m. (#1070) (#1188) * PWX-28826 Boilerplace * more boilerplate * PWX-28826: Pre-flight check for DMthin. * PWX-28826: Add comments and move StorageNode cleanup. * Passed checks should be Info events. * Passed checks should be Info events. (#1010) * Pwx 28826 (#1011) * Pwx 28826 (#1012) * PWX-28826: Update with the latest master changes. (#1013) * Updating CSV to use 23.3.1 released image * Update for 23.3.1 release * Controller gen vendor * PWX-29389 Add CRD for portworx diags collection * PWX-29409: Ignore zones with no nodes (#1008) In disaggregated mode, there could be zones in which no storage nodes might be present. Such a zone would make the maxSNPZ value to be 0. CHanging the behavior to ignore 0 nodes in a zone for maxSNPZ calculation. --------- * Add PassPreFlight event tag and logging * PWX-28826: Check status of portworx container in pre-flight pod and remove 'wait' code. * PWX-28826: Fix unit test. * PWX-28826: Fix unit test. * PWX-28826: PR review changes and fix portworx_test.go UTs * PWX-28826: fix gomack Validate calls. Also comment out the two tests that don't work since Validate was removed from the controller.validate() func. PWX-30373 to try and fix later. * PWX-30373: Re-add back in the commented out tests and add K8s version check failure to trigger the needed workflow. * PWX-28826: Exit pre-check wait if running CBT namespace. * PWX-28826: Add 5 min timeout to pre-flight status check. * PWX-28826: Exit GetPreFlightStatus() with success if running CBT namespace. * PWX-28826: Don't automatically enable dmthin via pre-flight check if running CBT namespace. * PWX-30373: Revert UT and integration test hacks. Need to mock the functionality correctly. * PWX-28826: Increase pre-flight daemonset ready wait to 10mins. * PWX-28826: fix 'TestValidate' UT. Don't error if pre-flight daemonset exists. * Use reconcile loop to check pre-flight status. Timeout after 15m. * PWX-28826: Startup check was not handling if preflight was not enabled. * PWX-28826: Re-enable preflight for integration tests. * PWX-28826: disable preflight for integration tests. * PWX-28826: Only do preflight if AWS. * PWX-30496: if '-T dmthin' exists in stc before preflight is ran and preflight fails don't start. Add default metadata dev for dmthin if it does not exist. * PWX-28826: Use condition set to determine of CheckCloudDrivePermission check is to be run. Handle skip annotation and pod cleanup. * PWX-28826: Remove debug logging * PWX-30419: Remove more debug logging. * PWX-30419: Revert to correct version. * PWX-30419: Fix unit test. Need to enable pre-flight. * PWX-30419: Fix TestEKSPreflightCheck unit test. Pre-flight now includes DMthin checks so need to handle that. * update from master, conflict fix. * PWX-28826: Use cluster condition to determine pre-flight status. * PWX-28826: return cluster condition from driverValidate as that to determin status.. * Add err message to default condition message and make sure to use default condition on CloudDrivePermission error. * PWX-28826: Add a constant for pre-flight timeout value. * PWX-28826: pre-flight timeout should prevent startup. Add event for timeout. * PWX-28826: Fix unit tests. * PWX-28826: Use progress string. --------- Signed-off-by: Harsh Desai <hadesai@purestorage.com> Signed-off-by: Jose Rivera <jose@portworx.com> Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Signed-off-by: Naveen Revanna <nrevanna@purestorage.com> Co-authored-by: Harsh Desai <hadesai@purestorage.com> Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com> Co-authored-by: Jiafeng Liao <jliao@purestorage.com> Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com> Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>

harsh-px and others added 30 commits March 28, 2023 16:07

PWX-28826 Boilerplace

069761b

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

more boilerplate

f8b43c0

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

PWX-28826: Pre-flight check for DMthin.

29d1d99

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Add comments and move StorageNode cleanup.

f82f027

Signed-off-by: Jose Rivera <jose@portworx.com>

Passed checks should be Info events.

740fc60

Signed-off-by: Jose Rivera <jose@portworx.com>

Passed checks should be Info events. (#1010)

b930040

Signed-off-by: Jose Rivera <jose@portworx.com>

Merge branch 'PWX-28826' of github.com:libopenstorage/operator into P…

e06c9f4

…WX-28826

Pwx 28826 (#1011)

6a0e863

Pwx 28826 (#1012)

94c6226

Merge branch 'master' into PWX-28826

4cb501e

Add PassPreFlight event tag and logging

0ff38ae

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Check status of portworx container in pre-flight pod and r…

5d0843e

…emove 'wait' code. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Fix unit test.

766939d

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Fix unit test.

2b4e04b

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: PR review changes and fix portworx_test.go UTs

c82bd43

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: fix gomack Validate calls. Also comment out the two tests …

02a5f6c

…that don't work since Validate was removed from the controller.validate() func. PWX-30373 to try and fix later. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-30373: Re-add back in the commented out tests and add K8s version…

ed4aa58

… check failure to trigger the needed workflow. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Exit pre-check wait if running CBT namespace.

e21de49

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Add 5 min timeout to pre-flight status check.

ff94003

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Exit GetPreFlightStatus() with success if running CBT name…

c931dae

…space. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Don't automatically enable dmthin via pre-flight check if …

3a4b027

…running CBT namespace. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-30373: Revert UT and integration test hacks. Need to mock the fun…

b55a756

…ctionality correctly. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Increase pre-flight daemonset ready wait to 10mins.

6ec969e

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: fix 'TestValidate' UT. Don't error if pre-flight daemonset…

528ad6d

… exists. Signed-off-by: Jose Rivera <jose@portworx.com>

Merge branch 'master' into PWX-28826

7bd5f40

Use reconcile loop to check pre-flight status. Timeout after 15m.

0ab2abf

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Startup check was not handling if preflight was not enabled.

5978e4f

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Re-enable preflight for integration tests.

9cd7f81

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: disable preflight for integration tests.

541f6be

Signed-off-by: Jose Rivera <jose@portworx.com>

jrivera-px added 4 commits July 7, 2023 19:53

Merge branch 'master' into PWX-28826-use-reconcile-loop

50b829a

Merge branch 'master' into PWX-28826-use-reconcile-loop

f883477

Merge branch 'master' into PWX-28826-use-reconcile-loop

4d6cab8

Merge branch 'master' into PWX-28826-use-reconcile-loop

92d7b5e

piyush-nimbalkar reviewed Jul 26, 2023

View reviewed changes

jrivera-px added 8 commits July 27, 2023 18:18

Merge branch 'master' into PWX-28826-use-reconcile-loop

64b0819

Merge branch 'master' into PWX-28826-use-reconcile-loop

79b8905

Merge branch 'master' into PWX-28826-use-reconcile-loop

fa1097c

Merge branch 'master' into PWX-28826-use-reconcile-loop

a8c8f0e

update from master, conflict fix.

55a7d3d

Merge branch 'master' into PWX-28826-use-reconcile-loop

ba25d86

Merge branch 'master' into PWX-28826-use-reconcile-loop

5a9933f

PWX-28826: Use cluster condition to determine pre-flight status.

418da07

Signed-off-by: Jose Rivera <jose@portworx.com>

jrivera-px requested a review from piyush-nimbalkar August 3, 2023 22:28

jrivera-px added 3 commits August 4, 2023 00:55

PWX-28826: return cluster condition from driverValidate as that to de…

73d4808

…termin status.. Signed-off-by: Jose Rivera <jose@portworx.com>

Merge branch 'master' into PWX-28826-use-reconcile-loop

1c7b4b3

Add err message to default condition message and make sure to use def…

8d6cf9f

…ault condition on CloudDrivePermission error. Signed-off-by: Jose Rivera <jose@portworx.com>

piyush-nimbalkar approved these changes Aug 4, 2023

View reviewed changes

github-actions bot added stage/approved and removed stage/in-review-stale labels Aug 4, 2023

jrivera-px added 5 commits August 4, 2023 18:59

Merge branch 'master' into PWX-28826-use-reconcile-loop

8ed780b

PWX-28826: Add a constant for pre-flight timeout value.

602f01b

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: pre-flight timeout should prevent startup. Add event for t…

eb1862b

…imeout. Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Fix unit tests.

ee9bc1d

Signed-off-by: Jose Rivera <jose@portworx.com>

PWX-28826: Use progress string.

e6659b5

Signed-off-by: Jose Rivera <jose@portworx.com>

jrivera-px merged commit 0298ebb into master Aug 7, 2023
6 of 8 checks passed

jrivera-px mentioned this pull request Aug 7, 2023

PWX-30419: Use reconcile loop to check pre-flight status. Timeout aft… #1188

Merged

piyush-nimbalkar deleted the PWX-28826-use-reconcile-loop branch September 15, 2023 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PWX-30419: Use reconcile loop to check pre-flight status. Timeout after 15m. #1070

PWX-30419: Use reconcile loop to check pre-flight status. Timeout after 15m. #1070

jrivera-px commented Jun 9, 2023

piyush-nimbalkar Jul 26, 2023

piyush-nimbalkar Jul 26, 2023

piyush-nimbalkar Jul 26, 2023

piyush-nimbalkar Jul 26, 2023

piyush-nimbalkar Jul 26, 2023

piyush-nimbalkar left a comment

piyush-nimbalkar Aug 4, 2023

piyush-nimbalkar Aug 4, 2023

jrivera-px Aug 4, 2023

PWX-30419: Use reconcile loop to check pre-flight status. Timeout after 15m. #1070

PWX-30419: Use reconcile loop to check pre-flight status. Timeout after 15m. #1070

Conversation

jrivera-px commented Jun 9, 2023

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

piyush-nimbalkar left a 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