Fixed vdb-gen multi-path issue (#949)

When we generate a VDB CR from an existing database, this database
probably has multiple DEPOT or DATA paths. This PR removed the error for
that kind of database in vdb-gen.

config/manifests/bases/verticadb-operator.clusterserviceversion.yaml

@@ -1110,6 +1110,16 @@ spec:
       - description: Conditions for VerticaDB
         displayName: Conditions
         path: conditions
+      - description: The details about the last created restore point
+        displayName: Restore Point
+        path: restorePoint
+      - description: Name of the archive that this restore point was created in.
+        displayName: Archive
+        path: restorePoint.archive
+      - displayName: End Timestamp
+        path: restorePoint.endTimestamp
+      - displayName: Start Timestamp
+        path: restorePoint.startTimestamp
       - description: The sandbox statuses
         displayName: Sandboxes
         path: sandboxes
@@ -2041,6 +2051,16 @@ spec:
           vertica cluster.
         displayName: Install Count
         path: installCount
+      - description: The details about the last created restore point
+        displayName: Restore Point
+        path: restorePoint
+      - description: Name of the archive that this restore point was created in.
+        displayName: Archive
+        path: restorePoint.archive
+      - displayName: End Timestamp
+        path: restorePoint.endTimestamp
+      - displayName: Start Timestamp
+        path: restorePoint.startTimestamp
       - description: The sandbox statuses
         displayName: Sandboxes
         path: sandboxes

pkg/reviveplanner/analyze.go

@@ -238,6 +238,11 @@ func (p *Planner) extractPathPrefixFromVNodePath(path string) (string, bool) {
 	// Path will come in the form: <prefix>/<dbname>/v_<dbname>_<nodenum>_<pathType>
 	// This function will return <prefix>.
 	dbName := p.Parser.GetDatabaseName()
+	return ExtractNodePathPrefix(path, dbName)
+}
+
+// ExtractNodePathPrefix will extract out the prefix of a vertica POSIX path.
+func ExtractNodePathPrefix(path, dbName string) (string, bool) {
 	r := regexp.MustCompile(fmt.Sprintf(`(.*)/%s/v_%s_node[0-9]{4}_`, dbName, strings.ToLower(dbName)))
 	m := r.FindStringSubmatch(path)
 	const ExpectedMatches = 2

pkg/vdbgen/vdb.go

@@ -36,6 +36,7 @@ import (
 	vmeta "github.com/vertica/vertica-kubernetes/pkg/meta"
 	"github.com/vertica/vertica-kubernetes/pkg/names"
 	"github.com/vertica/vertica-kubernetes/pkg/paths"
+	vrevive "github.com/vertica/vertica-kubernetes/pkg/reviveplanner"
 	vversion "github.com/vertica/vertica-kubernetes/pkg/version"
 )
 
@@ -459,12 +460,12 @@ func (d *DBGenerator) setRequestSize(ctx context.Context) error {
 // setCommunalPath will query the catalog and set the communal path in the vdb.
 func (d *DBGenerator) setCommunalPath(ctx context.Context) error {
 	var communalPath string
-	extractPath := func(nodeName, nodePath sql.NullString) error {
+	extractPath := func(nodeName, nodePath sql.NullString) (bool, error) {
 		if !nodePath.Valid {
-			return errors.New("node path is NULL")
+			return false, errors.New("node path is NULL")
 		}
 		communalPath = nodePath.String
-		return nil
+		return true, nil
 	}
 
 	if err := d.queryPathForUsage(ctx, "DATA", extractPath); err != nil {
@@ -478,7 +479,7 @@ func (d *DBGenerator) setCommunalPath(ctx context.Context) error {
 // This will return the common prefix amongst all nodes.  It will return an
 // error if nodes have different paths.
 func (d *DBGenerator) queryPathForUsage(ctx context.Context, usage string,
-	extractFunc func(nodeName, nodePath sql.NullString) error) error {
+	extractFunc func(nodeName, nodePath sql.NullString) (bool, error)) error {
 	var q string
 	const CatalogUsage = "CATALOG"
 	// There isn't one table that we can query to get all storage locations. The
@@ -506,9 +507,13 @@ func (d *DBGenerator) queryPathForUsage(ctx context.Context, usage string,
 			return fmt.Errorf("failed running '%s': %w", q, err)
 		}
 
-		if err := extractFunc(nodeName, nodePath); err != nil {
+		stop, err := extractFunc(nodeName, nodePath)
+		if err != nil {
 			return err
 		}
+		if stop {
+			break
+		}
 	}
 
 	return nil
@@ -520,28 +525,39 @@ func (d *DBGenerator) queryPathForUsage(ctx context.Context, usage string,
 func (d *DBGenerator) queryLocalPath(ctx context.Context, usage string) (string, error) {
 	var commonPrefix string
 
-	extractCommonPrefix := func(nodeName, nodePath sql.NullString) error {
+	extractCommonPrefix := func(nodeName, nodePath sql.NullString) (bool, error) {
 		workingDir := nodePath.String
+		// There could be multiple non-catalog local paths, and we only need to find one
+		// that was auto-generated by Vertica and extract its common prefix.
+		if usage != "CATALOG" && filepath.IsAbs(workingDir) {
+			cleanDir := filepath.Clean(workingDir)
+			curCommonPrefix, ok := vrevive.ExtractNodePathPrefix(cleanDir, d.Opts.DBName)
+			if !ok {
+				return false, nil
+			} else {
+				commonPrefix = curCommonPrefix
+				return true, nil
+			}
+		}
+
 		// When we query the dir for catalog usage, we use a different query
 		// that has slightly different output. The table we use puts a /Catalog
 		// suffix on the end of the path. We want to take that off before
 		// proceeding.
-		if usage == "CATALOG" {
-			workingDir = path.Dir(workingDir)
-		}
+		workingDir = path.Dir(workingDir)
 		// Extract out the common prefix from the nodePath.  nodePath will be
 		// something like /data/vertdb/v_vertdb_node0001_data.  We want to
 		// remove the node specific suffix.
 		curCommonPrefix := path.Dir(path.Dir(workingDir))
 		// Check if the prefix matches.  If it doesn't then an error is returned
 		// as paths across all nodes must be homogenous.
 		if len(commonPrefix) > 0 && commonPrefix != curCommonPrefix {
-			return fmt.Errorf(
-				"location path for usage '%s' must be the same across all nodes -- path '%s' does not share the common prefix from other nodes '%s'",
-				usage, nodePath.String, commonPrefix)
+			return false, fmt.Errorf(
+				"catalog location path must be the same across all nodes -- path %q does not share the common prefix from other nodes %q",
+				nodePath.String, commonPrefix)
 		}
 		commonPrefix = curCommonPrefix
-		return nil
+		return false, nil
 	}
 
 	if err := d.queryPathForUsage(ctx, usage, extractCommonPrefix); err != nil {

pkg/vdbgen/vdb_test.go

@@ -401,16 +401,18 @@ var _ = Describe("vdb", func() {
 		createMock()
 		defer deleteMock()
 
-		dbGen := DBGenerator{Conn: db}
+		dbGen := DBGenerator{Conn: db, Opts: &Options{DBName: "vertdb"}}
 
 		mock.ExpectQuery(Queries[StorageLocationKey]).
 			WithArgs("DATA,TEMP").
 			WillReturnRows(sqlmock.NewRows([]string{"node_name", "location_path"}).
+				AddRow("v_vertdb_node0001", "/custom/data/storage/location").
 				AddRow("v_vertdb_node0001", "/data/vertdb/v_vertdb_node0001_data").
 				AddRow("v_vertdb_node0002", "/data/vertdb/v_vertdb_node0002_data"))
 		mock.ExpectQuery(Queries[StorageLocationKey]).
 			WithArgs("DEPOT").
 			WillReturnRows(sqlmock.NewRows([]string{"node_name", "location_path"}).
+				AddRow("v_vertdb_node0001", "/custom/data/storage/location").
 				AddRow("v_vertdb_node0001", "/home/dbadmin/depot/vertdb/v_vertdb_node0001_data").
 				AddRow("v_vertdb_node0002", "/home/dbadmin/depot/vertdb/v_vertdb_node0002_data"))
 		mock.ExpectQuery(Queries[DiskStorageLocationKey]).
@@ -427,17 +429,29 @@ var _ = Describe("vdb", func() {
 		Expect(mock.ExpectationsWereMet()).Should(Succeed())
 	})
 
-	It("should raise an error if the local paths are different on two nodes", func() {
+	It("should raise an error if catalog paths are different on two nodes", func() {
 		createMock()
 		defer deleteMock()
 
-		dbGen := DBGenerator{Conn: db}
+		dbGen := DBGenerator{Conn: db, Opts: &Options{DBName: "vertdb"}}
 
 		mock.ExpectQuery(Queries[StorageLocationKey]).
 			WithArgs("DATA,TEMP").
 			WillReturnRows(sqlmock.NewRows([]string{"node_name", "location_path"}).
-				AddRow("v_vertdb_node0001", "/data1/vertdb/v_vertdb_node0001_data").
-				AddRow("v_vertdb_node0002", "/data2/vertdb/v_vertdb_node0002_data"))
+				AddRow("v_vertdb_node0001", "/custom/data/storage/location").
+				AddRow("v_vertdb_node0001", "/data/vertdb/v_vertdb_node0001_data").
+				AddRow("v_vertdb_node0002", "/data/vertdb/v_vertdb_node0002_data"))
+		mock.ExpectQuery(Queries[StorageLocationKey]).
+			WithArgs("DEPOT").
+			WillReturnRows(sqlmock.NewRows([]string{"node_name", "location_path"}).
+				AddRow("v_vertdb_node0001", "/custom/data/storage/location").
+				AddRow("v_vertdb_node0001", "/home/dbadmin/depot/vertdb/v_vertdb_node0001_data").
+				AddRow("v_vertdb_node0002", "/home/dbadmin/depot/vertdb/v_vertdb_node0002_data"))
+		mock.ExpectQuery(Queries[DiskStorageLocationKey]).
+			WithArgs("CATALOG").
+			WillReturnRows(sqlmock.NewRows([]string{"node_name", "location_path"}).
+				AddRow("v_vertdb_node0001", "/catalog1/vertdb/v_vertdb_node0001_catalog/Catalog").
+				AddRow("v_vertdb_node0002", "/catalog2/vertdb/v_vertdb_node0002_catalog/Catalog"))
 
 		Expect(dbGen.setLocalPaths(ctx)).ShouldNot(Succeed())
 		Expect(mock.ExpectationsWereMet()).Should(Succeed())

tests/e2e-leg-4/vdb-gen/22-create-storage-path.yaml

@@ -0,0 +1,17 @@
+# (c) Copyright [2021-2024] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: kubectl exec --namespace $NAMESPACE v-vdb-gen-sc1-0 -c server -- vsql -w superuser -c "create location '/tmp/test' ALL NODES USAGE 'DATA,TEMP';"

tests/e2e-leg-4/vdb-gen/51-verify-storage-location-exists.yaml

@@ -0,0 +1,24 @@
+# (c) Copyright [2021-2024] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  # verify the storage location is still in database
+  - script: |
+      val=$(kubectl exec -n $NAMESPACE v-vdb-gen-revive-sc1-0 -c server -- vsql -w superuser -tAc "select count(*) from storage_locations where location_path = '/tmp/test' and location_usage = 'DATA,TEMP'"); \
+      if [ $val == 0 ]; then \
+        exit 1; \
+      fi
+  # verify the directory is created in the pod
+  - command: kubectl exec -n $NAMESPACE v-vdb-gen-revive-sc1-0 -c server -- test -d /tmp/test