Implement Node Label Functionality for pgdump Restores (#585)

* Implemented node label functionality for pgdump restores.  Also,
refactored node label validation, and corrected a defect in which
and node label with an empty value was considered valid.

* Make pgbasebackup the default for the "pgo backup" command

Author: andrewlecuyer

apiserver/backrestservice/backrestimpl.go

@@ -445,24 +445,14 @@ func getRestoreParams(request *msgs.RestoreRequest, ns string, cluster crv1.Pgcl
 	spec.Parameters[util.LABEL_PGBACKREST_REPO_PATH] = "/backrestrepo/" + request.FromCluster + "-backrest-shared-repo"
 	spec.Parameters[util.LABEL_PGBACKREST_REPO_HOST] = request.FromCluster + "-backrest-shared-repo"
 
+	// validate & parse nodeLabel if exists
 	if request.NodeLabel != "" {
-		parts := strings.Split(request.NodeLabel, "=")
-		if len(parts) != 2 {
-			return nil, errors.New(request.NodeLabel + " node label does not follow key=value format")
-		}
 
-		keyValid, valueValid, err := apiserver.IsValidNodeLabel(parts[0], parts[1])
-		if err != nil {
+		if err := apiserver.ValidateNodeLabel(request.NodeLabel); err != nil {
 			return nil, err
 		}
 
-		if !keyValid {
-			return nil, errors.New(request.NodeLabel + " key was not valid .. check node labels for correct values to specify")
-		}
-		if !valueValid {
-			return nil, errors.New(request.NodeLabel + " node label value was not valid .. check node labels for correct values to specify")
-		}
-
+		parts := strings.Split(request.NodeLabel, "=")
 		spec.Parameters[util.LABEL_NODE_LABEL_KEY] = parts[0]
 		spec.Parameters[util.LABEL_NODE_LABEL_VALUE] = parts[1]
 

apiserver/clusterservice/clusterimpl.go

@@ -20,14 +20,15 @@ import (
 	"errors"
 	"fmt"
 
-	log "github.com/Sirupsen/logrus"
-	crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
-	"github.com/crunchydata/postgres-operator/apiserver"
 	"io/ioutil"
 	"strconv"
 	"strings"
 	"time"
 
+	log "github.com/Sirupsen/logrus"
+	crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
+	"github.com/crunchydata/postgres-operator/apiserver"
+
 	msgs "github.com/crunchydata/postgres-operator/apiservermsgs"
 	"github.com/crunchydata/postgres-operator/config"
 	"github.com/crunchydata/postgres-operator/kubeapi"
@@ -652,34 +653,20 @@ func CreateCluster(request *msgs.CreateClusterRequest, ns string) msgs.CreateClu
 			log.Debug("primary node labels used from pgo.yaml")
 		}
 
+		// validate & parse nodeLabel if exists
 		if request.NodeLabel != "" {
-			parts := strings.Split(request.NodeLabel, "=")
-			if len(parts) != 2 {
-				resp.Status.Code = msgs.Error
-				resp.Status.Msg = request.NodeLabel + " node label does not follow key=value format"
-				return resp
-			}
 
-			keyValid, valueValid, err := apiserver.IsValidNodeLabel(parts[0], parts[1])
-			if err != nil {
+			if err = apiserver.ValidateNodeLabel(request.NodeLabel); err != nil {
 				resp.Status.Code = msgs.Error
 				resp.Status.Msg = err.Error()
 				return resp
 			}
 
-			if !keyValid {
-				resp.Status.Code = msgs.Error
-				resp.Status.Msg = request.NodeLabel + " key was not valid .. check node labels for correct values to specify"
-				return resp
-			}
-			if !valueValid {
-				resp.Status.Code = msgs.Error
-				resp.Status.Msg = request.NodeLabel + " node label value was not valid .. check node labels for correct values to specify"
-				return resp
-			}
-			log.Debug("primary node labels used from user entered flag")
+			parts := strings.Split(request.NodeLabel, "=")
 			userLabelsMap[util.LABEL_NODE_LABEL_KEY] = parts[0]
 			userLabelsMap[util.LABEL_NODE_LABEL_VALUE] = parts[1]
+
+			log.Debug("primary node labels used from user entered flag")
 		}
 
 		if request.ReplicaStorageConfig != "" {

apiserver/clusterservice/scaleimpl.go

@@ -17,6 +17,9 @@ limitations under the License.
 
 import (
 	"fmt"
+	"strconv"
+	"strings"
+
 	log "github.com/Sirupsen/logrus"
 	crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
 	"github.com/crunchydata/postgres-operator/apiserver"
@@ -28,8 +31,6 @@ import (
 	"k8s.io/api/extensions/v1beta1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"strconv"
-	"strings"
 )
 
 // ScaleCluster ...
@@ -117,36 +118,20 @@ func ScaleCluster(name, replicaCount, resourcesConfig, storageConfig, nodeLabel,
 		log.Debug("using pgo.yaml ReplicaNodeLabel for replica creation")
 	}
 
-	//validate nodeLabel
+	// validate & parse nodeLabel if exists
 	if nodeLabel != "" {
-		parts = strings.Split(nodeLabel, "=")
-		if len(parts) != 2 {
-			response.Status.Code = msgs.Error
-			response.Status.Msg = nodeLabel + " node label does not follow key=value format"
-			return response
-		}
 
-		keyValid, valueValid, err := apiserver.IsValidNodeLabel(parts[0], parts[1])
-		if err != nil {
+		if err = apiserver.ValidateNodeLabel(nodeLabel); err != nil {
 			response.Status.Code = msgs.Error
 			response.Status.Msg = err.Error()
 			return response
 		}
 
-		if !keyValid {
-			response.Status.Code = msgs.Error
-			response.Status.Msg = nodeLabel + " key was not valid .. check node labels for correct values to specify"
-			return response
-		}
-		if !valueValid {
-			response.Status.Code = msgs.Error
-			response.Status.Msg = nodeLabel + " node label value was not valid .. check node labels for correct values to specify"
-			return response
-		}
+		parts := strings.Split(nodeLabel, "=")
 		spec.UserLabels[util.LABEL_NODE_LABEL_KEY] = parts[0]
 		spec.UserLabels[util.LABEL_NODE_LABEL_VALUE] = parts[1]
-		log.Debug("using user entered node label for replica creation")
 
+		log.Debug("using user entered node label for replica creation")
 	}
 
 	labels := make(map[string]string)

apiserver/pgdumpservice/pgdumpimpl.go

@@ -18,6 +18,9 @@ limitations under the License.
 import (
 	"errors"
 	"fmt"
+	"strconv"
+	"strings"
+
 	log "github.com/Sirupsen/logrus"
 	crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
 	"github.com/crunchydata/postgres-operator/apiserver"
@@ -27,8 +30,6 @@ import (
 	"k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"strconv"
-	"strings"
 )
 
 const pgDumpCommand = "pgdump"
@@ -446,7 +447,13 @@ func Restore(request *msgs.PgRestoreRequest, ns string) msgs.PgRestoreResponse {
 
 	// pgtask := getRestoreParams(cluster)
 
-	pgtask := buildPgTaskForRestore(taskName, crv1.PgtaskpgRestore, request)
+	pgtask, err := buildPgTaskForRestore(taskName, crv1.PgtaskpgRestore, request)
+	if err != nil {
+		resp.Status.Code = msgs.Error
+		resp.Status.Msg = err.Error()
+		return resp
+	}
+
 	existingTask := crv1.Pgtask{}
 
 	//delete any existing pgtask with the same name
@@ -482,7 +489,7 @@ func Restore(request *msgs.PgRestoreRequest, ns string) msgs.PgRestoreResponse {
 }
 
 // builds out a pgTask structure that can be handed to kube
-func buildPgTaskForRestore(taskName string, action string, request *msgs.PgRestoreRequest) *crv1.Pgtask {
+func buildPgTaskForRestore(taskName string, action string, request *msgs.PgRestoreRequest) (*crv1.Pgtask, error) {
 
 	var newInstance *crv1.Pgtask
 	var storageSpec crv1.PgStorageSpec
@@ -508,6 +515,21 @@ func buildPgTaskForRestore(taskName string, action string, request *msgs.PgResto
 
 	spec.Parameters[util.LABEL_PGRESTORE_PORT] = apiserver.Pgo.Cluster.Port
 	spec.Parameters[util.LABEL_CCP_IMAGE_TAG_KEY] = apiserver.Pgo.Cluster.CCPImageTag
+
+	// validate & parse nodeLabel if exists
+	if request.NodeLabel != "" {
+
+		if err := apiserver.ValidateNodeLabel(request.NodeLabel); err != nil {
+			return nil, err
+		}
+
+		parts := strings.Split(request.NodeLabel, "=")
+		spec.Parameters[util.LABEL_NODE_LABEL_KEY] = parts[0]
+		spec.Parameters[util.LABEL_NODE_LABEL_VALUE] = parts[1]
+
+		log.Debug("Restore node labels used from user entered flag")
+	}
+
 	spec.StorageSpec = storageSpec
 
 	newInstance = &crv1.Pgtask{
@@ -516,7 +538,7 @@ func buildPgTaskForRestore(taskName string, action string, request *msgs.PgResto
 		},
 		Spec: spec,
 	}
-	return newInstance
+	return newInstance, nil
 }
 
 // TODO: Needed?

apiserver/root.go

@@ -415,6 +415,29 @@ func IsValidStorageName(name string) bool {
 	return ok
 }
 
+// ValidateNodeLabel
+// returns error if node label is invalid
+func ValidateNodeLabel(nodeLabel string) error {
+	parts := strings.Split(nodeLabel, "=")
+	if len(parts) != 2 {
+		return errors.New(nodeLabel + " node label does not follow key=value format")
+	}
+
+	keyValid, valueValid, err := IsValidNodeLabel(parts[0], parts[1])
+	if err != nil {
+		return err
+	}
+
+	if !keyValid {
+		return errors.New(nodeLabel + " key was not valid .. check node labels for correct values to specify")
+	}
+	if !valueValid {
+		return errors.New(nodeLabel + " node label value was not valid .. check node labels for correct values to specify")
+	}
+
+	return nil
+}
+
 // IsValidNodeLabel
 // returns bool for key validity
 // returns bool for value validity
@@ -430,14 +453,13 @@ func IsValidNodeLabel(key, value string) (bool, bool, error) {
 		return false, false, err
 	}
 
-	var v string
 	for _, node := range nodes.Items {
-		v = node.ObjectMeta.Labels[key]
-		if v != "" {
+
+		if val, exists := node.ObjectMeta.Labels[key]; exists {
 			keyValid = true
-		}
-		if v == value {
-			valueValid = true
+			if val == value {
+				valueValid = true
+			}
 		}
 	}
 
@@ -499,27 +521,15 @@ func validateWithKube() {
 
 	for _, n := range configNodeLabels {
 
+		//parse & validate pgo.yaml node labels if set
 		if n != "" {
-			parts := strings.Split(n, "=")
-			if len(parts) != 2 {
-				log.Error(n + " node label in pgo.yaml does not follow key=value format")
-				os.Exit(2)
-			}
 
-			keyValid, valueValid, err := IsValidNodeLabel(parts[0], parts[1])
-			if err != nil {
+			if err := ValidateNodeLabel(n); err != nil {
+				log.Error(n + " node label specified in pgo.yaml is invalid")
 				log.Error(err)
 				os.Exit(2)
 			}
 
-			if !keyValid {
-				log.Error(n + " key not a valid node label key in pgo.yaml")
-				os.Exit(2)
-			}
-			if !valueValid {
-				log.Error(n + "value not a valid node label value in pgo.yaml ")
-				os.Exit(2)
-			}
 			log.Debugf("%s is a valid pgo.yaml node label default", n)
 		}
 	}

apiservermsgs/pgdumpmsgs.go

@@ -15,9 +15,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import (
 //crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
-)
 
 type CreatepgDumpBackupResponse struct {
 	Results []string
@@ -49,4 +47,5 @@ type PgRestoreRequest struct {
 	FromPVC     string
 	RestoreOpts string
 	PITRTarget  string
+	NodeLabel   string
 }

conf/postgres-operator/pgrestore-job.json

@@ -86,7 +86,8 @@
                         ]
                     }
                 ],
-                "restartPolicy": "Never"
+        {{.NodeSelector}}
+                        "restartPolicy": "Never"
             }
         }
     }

hugo/content/Design/_index.md

@@ -176,7 +176,7 @@ on PG 9.6/9.5 systems, the command you will use is *select pg_xlog_replay_resume
  * there is currently no Operator validation of user entered pgBackRest command options, you will need to make sure to enter these correctly, if not the pgBackRest restore command can fail.
  * the restore workflow does not perform a backup after the restore nor does it verify that any replicas are in a working status after the restore, it is possible you might have to take actions on the replica to get them back to replicating with the new restored primary.
  * pgbackrest.org suggests running a pgbackrest backup after a restore, this needs to be done by the DBA as part of a restore
- * when performing a restore, the **node-label** option can be utilized to target a specific node for both the pgBackRest restore job and the new (i.e. restored) primary deployment that is then created for the cluster.  If a node label is not specified, the restore job will not target any specific node, and the restored primary deployment will inherit any node label's defined for the original primary deployment.
+ * when performing a pgBackRest restore, the **node-label** flag can be utilized to target a specific node for both the pgBackRest restore job and the new (i.e. restored) primary deployment that is then created for the cluster.  If a node label is not specified, the restore job will not target any specific node, and the restored primary deployment will inherit any node label's defined for the original primary deployment.
 
 ## PGO Scheduler
 

operator/pgdump/restore.go

@@ -18,6 +18,8 @@ package pgdump
 import (
 	"bytes"
 	"encoding/json"
+	"os"
+
 	log "github.com/Sirupsen/logrus"
 	crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
 	"github.com/crunchydata/postgres-operator/kubeapi"
@@ -27,7 +29,6 @@ import (
 	v1batch "k8s.io/api/batch/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"os"
 )
 
 type restorejobTemplateFields struct {
@@ -45,6 +46,7 @@ type restorejobTemplateFields struct {
 	CCPImagePrefix      string
 	CCPImageTag         string
 	PgPort              string
+	NodeSelector        string
 }
 
 // Restore ...
@@ -90,6 +92,7 @@ func Restore(namespace string, clientset *kubernetes.Clientset, restclient *rest
 		PITRTarget:          task.Spec.Parameters[util.LABEL_PGRESTORE_PITR_TARGET],
 		CCPImagePrefix:      operator.Pgo.Cluster.CCPImagePrefix,
 		CCPImageTag:         operator.Pgo.Cluster.CCPImageTag,
+		NodeSelector:        operator.GetAffinity(task.Spec.Parameters["NodeLabelKey"], task.Spec.Parameters["NodeLabelValue"], "In"),
 	}
 
 	var doc2 bytes.Buffer

pgo/cmd/backup.go

@@ -60,7 +60,7 @@ var backupCmd = &cobra.Command{
 
 				createBackrestBackup(args, Namespace)
 
-			case labelutil.LABEL_BACKUP_TYPE_BASEBACKUP:
+			case "", labelutil.LABEL_BACKUP_TYPE_BASEBACKUP:
 
 				createBackup(args, Namespace)