PWX-30496: if '-T dmthin' exists in stc before preflight is ran and p…

…reflight fails don't start. (#1019) * 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> * Only run preflight if AWS. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30496: If the user intended to use dmthin. The '-T dmthin' will exist in the stc before preflight is ran. If preflight fails in this case don't start. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30496: If preflight enables DMthin add a 64G metadata drive. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30496: Review fixes. Signed-off-by: Jose Rivera <jose@portworx.com> * PWX-30496: add the metadata device in both cases where the user has passed -T dmthin or we added it for them. 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>
libopenstorage · Apr 18, 2023 · ce2fce1 · ce2fce1
1 parent f10adb4
commit ce2fce1
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 13 deletions.
diff --git a/drivers/storage/portworx/portworx.go b/drivers/storage/portworx/portworx.go
@@ -134,14 +134,16 @@ func (p *portworx) Validate(cluster *corev1.StorageCluster) error {
 	defer func() {
 		// Clean up the pre-flight pods
 		logrus.Infof("pre-flight: cleaning pre-flight ds...")
-		err = preFlighter.DeletePreFlight()
-		if err != nil {
-			logrus.Errorf("pre-flight: error deleting pre-flight: %v", err)
+		if derr := preFlighter.DeletePreFlight(); derr != nil {
+			logrus.Errorf("pre-flight: error deleting pre-flight: %v", derr)
 		}
 	}()
 
 	// Process all the StorageNode.Status.Checks
-	if storageNodes, err := p.storageNodesList(cluster); err == nil {
+	var storageNodes []*corev1.StorageNode
+
+	storageNodes, err = p.storageNodesList(cluster)
+	if err == nil {
 		err = preFlighter.ProcessPreFlightResults(p.recorder, storageNodes)
 		if err != nil {
 			logrus.Errorf("pre-flight: Error processing results: %v", err)
@@ -150,7 +152,7 @@ func (p *portworx) Validate(cluster *corev1.StorageCluster) error {
 		logrus.Errorf("pre-flight incomplete: Error getting storage node list: %v", err)
 	}
 
-	return nil
+	return err
 }
 func (p *portworx) initializeComponents() {
 	for _, comp := range component.GetAll() {

diff --git a/drivers/storage/portworx/portworx_test.go b/drivers/storage/portworx/portworx_test.go
@@ -160,7 +160,8 @@ func TestValidate(t *testing.T) {
 	require.NotEmpty(t, recorder.Events)
 	<-recorder.Events // Pop first event which is Default telemetry enabled event
 	require.Contains(t, <-recorder.Events,
-		fmt.Sprintf("%v %v %s", v1.EventTypeNormal, util.PassPreFlight, "Enabling DMthin"))
+		fmt.Sprintf("%v %v %s", v1.EventTypeNormal, util.PassPreFlight, "Enabling PX-StoreV2"))
+	require.Contains(t, *cluster.Spec.CloudStorage.SystemMdDeviceSpec, DefCmetaData)
 }
 
 func TestGetSelectorLabels(t *testing.T) {

diff --git a/drivers/storage/portworx/preflight.go b/drivers/storage/portworx/preflight.go
@@ -3,6 +3,7 @@ package portworx
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -29,6 +30,8 @@ const (
 	pxPreFlightDaemonSetName          = "px-pre-flight"
 	// PxPreFlightServiceAccountName name of portworx pre flight service account
 	PxPreFlightServiceAccountName = "px-pre-flight"
+	// DefCmetaData default metadata cloud device for DMthin
+	DefCmetaData = "type=gp3,size=64"
 )
 
 // PreFlightPortworx provides a set of APIs to uninstall portworx
@@ -50,8 +53,12 @@ type preFlightPortworx struct {
 	cluster   *corev1.StorageCluster
 	k8sClient client.Client
 	podSpec   v1.PodSpec
+	hardFail  bool
 }
 
+// Existing dmThin strings
+var dmthinRegex = regexp.MustCompile("(?i)(dmthin|PX-StoreV2|px-store-v2)")
+
 // NewPreFlighter returns an implementation of PreFlightPortworx interface
 func NewPreFlighter(
 	cluster *corev1.StorageCluster,
@@ -180,8 +187,29 @@ func (u *preFlightPortworx) RunPreFlight() error {
 	   	u.cluster.Annotations[pxutil.AnnotationMiscArgs] = strings.TrimSpace(miscArgs)
 	*/
 
-	// Add --pre-flight check for DMthin
-	preflightDS.Spec.Template.Spec.Containers[0].Args = append([]string{"--pre-flight", "-T", "dmthin"},
+	checkArgs := func(args []string) {
+		for i, arg := range args {
+			if arg == "-T" {
+				if dmthinRegex.Match([]byte(args[i+1])) {
+					u.hardFail = true
+				}
+			}
+		}
+	}
+
+	// Check for pre-existing DMthin in container args
+	checkArgs(u.podSpec.Containers[0].Args)
+
+	if !u.hardFail {
+		// If Dmthin param does not exist add it
+		preflightDS.Spec.Template.Spec.Containers[0].Args = append([]string{"-T", "dmthin"},
+			preflightDS.Spec.Template.Spec.Containers[0].Args...)
+	} else {
+		logrus.Infof("runPreflight: running pre-flight with existing PX-StoreV2 param, hard fail check enabled")
+	}
+
+	// Add pre-flight param
+	preflightDS.Spec.Template.Spec.Containers[0].Args = append([]string{"--pre-flight"},
 		preflightDS.Spec.Template.Spec.Containers[0].Args...)
 
 	if u.cluster.Spec.ImagePullSecret != nil && *u.cluster.Spec.ImagePullSecret != "" {
@@ -245,9 +273,30 @@ func (u *preFlightPortworx) ProcessPreFlightResults(recorder record.EventRecorde
 	}
 
 	if passed {
-		// Enable DMthin via misc args
-		u.cluster.Annotations[pxutil.AnnotationMiscArgs] = strings.TrimSpace(u.cluster.Annotations[pxutil.AnnotationMiscArgs] + " -T dmthin")
-		k8sutil.InfoEvent(recorder, u.cluster, util.PassPreFlight, "Enabling DMthin")
+		if !u.hardFail { // Enable DMthin via misc args if not enabled already
+			u.cluster.Annotations[pxutil.AnnotationMiscArgs] = strings.TrimSpace(u.cluster.Annotations[pxutil.AnnotationMiscArgs] + " -T dmthin")
+			k8sutil.InfoEvent(recorder, u.cluster, util.PassPreFlight, "Enabling PX-StoreV2")
+		} else {
+			k8sutil.InfoEvent(recorder, u.cluster, util.PassPreFlight, "PX-StoreV2 currently enabled")
+		}
+
+		// Add 64G metadata drive.
+		if u.cluster.Spec.CloudStorage == nil {
+			u.cluster.Spec.CloudStorage = &corev1.CloudStorageSpec{}
+		}
+
+		if u.cluster.Spec.CloudStorage.SystemMdDeviceSpec == nil {
+			cmetaData := DefCmetaData
+			u.cluster.Spec.CloudStorage.SystemMdDeviceSpec = &cmetaData
+		}
+	} else {
+		if !u.hardFail { // Enable DMthin via misc args if not enabled already
+			k8sutil.InfoEvent(recorder, u.cluster, util.PassPreFlight, "Not enabling PX-StoreV2")
+		} else { // hardFail is enabled, fail if any pre-flight check fails.
+			err := fmt.Errorf("PX-StoreV2 pre-check failed")
+			k8sutil.WarningEvent(recorder, u.cluster, util.FailedPreFlight, err.Error())
+			return err
+		}
 	}
 
 	return nil

diff --git a/pkg/controller/storagecluster/storagecluster.go b/pkg/controller/storagecluster/storagecluster.go
@@ -455,7 +455,8 @@ func (c *Controller) runPreflightCheck(cluster *corev1.StorageCluster) error {
 		}
 
 		// Run driver specific pre-flights
-		if err = c.Driver.Validate(toUpdate); err != nil {
+		err = c.Driver.Validate(toUpdate)
+		if err != nil {
 			logrus.WithError(err).Errorf("pre-flight validate failed")
 		}
 
@@ -472,7 +473,7 @@ func (c *Controller) runPreflightCheck(cluster *corev1.StorageCluster) error {
 				}
 			}
 		} else {
-			logrus.WithError(err).Errorf("Failed to get StorageNodes used for validate.")
+			logrus.WithError(serr).Errorf("Failed to get StorageNodes used for validate.")
 		}
 	}