* addressed Liran's comments.

Signed-off-by: Dean Amar <Dean.Amar@ibm.com>

Author: dean-amar

cmd/cliutil/flags.go

@@ -7,17 +7,10 @@ SPDX-License-Identifier: Apache-2.0
 package cliutil
 
 import (
-	"time"
-
 	"github.com/spf13/cobra"
 )
 
 // SetDefaultFlags registers the --config flag on the given command.
 func SetDefaultFlags(cmd *cobra.Command, configPath *string) {
 	cmd.PersistentFlags().StringVarP(configPath, "config", "c", "", "set the config file path")
 }
-
-// SetDurationFlag registers a duration flag with the given name on the given command.
-func SetDurationFlag(cmd *cobra.Command, name string, duration *time.Duration) {
-	cmd.Flags().DurationVar(duration, name, 5*time.Minute, "Timeout for the initialization operation")
-}

cmd/committer/db_init_cmd.go

@@ -37,33 +37,23 @@ func databaseInitializationCMD() *cobra.Command {
 		},
 	}
 	cliutil.SetDefaultFlags(cmd, &configPath)
-	cliutil.SetDurationFlag(cmd, "timeout", &timeout)
+	// registers a duration flag for database initialization.
+	cmd.Flags().DurationVar(&timeout, "timeout", 5*time.Minute, "Timeout for the initialization operation")
 	return cmd
 }
 
 // runDatabaseInitialization reads the database-initialization config,
 // connects directly to the database, and initializes it.
 func runDatabaseInitialization(cmd *cobra.Command, configPath string, timeout time.Duration) error {
-	dbConfig, _, err := config.ReadDBInitYamlAndSetupLogging(config.NewViperWithDBInitDefaults(), configPath)
+	dbConfig, err := config.ReadDBInitYamlAndSetupLogging(config.NewViperWithDBInitDefaults(), configPath)
 	if err != nil {
 		return err
 	}
 
 	ctx, cancel := context.WithTimeout(cmd.Context(), timeout)
 	defer cancel()
 
-	pool, err := db.NewPool(ctx, dbConfig)
-	if err != nil {
-		return errors.Wrap(err, "failed to connect to database")
-	}
-	defer pool.Close()
-
-	tablePreSplitTablets, err := db.GetTablePreSplitTablets(ctx, pool, dbConfig)
-	if err != nil {
-		return errors.Wrap(err, "failed to determine database type")
-	}
-
-	if err := db.SetupSystemTablesAndNamespaces(ctx, pool, dbConfig.Retry, tablePreSplitTablets); err != nil {
+	if err := db.SetupSystemTablesAndNamespaces(ctx, dbConfig); err != nil {
 		return errors.Wrap(err, "failed to initialize state database")
 	}
 

cmd/config/app_config.go

@@ -94,12 +94,12 @@ func ReadLoadGenYamlAndSetupLogging(
 // ReadDBInitYamlAndSetupLogging reading the YAML config file for database initialization.
 func ReadDBInitYamlAndSetupLogging(
 	v *viper.Viper, configPath string,
-) (*db.Config, *serve.Config, error) {
-	cfg, srvCfg, err := readYamlAndSetupLogging[databaseInitConfig](v, configPath)
+) (*db.Config, error) {
+	cfg, _, err := readYamlAndSetupLogging[databaseInitConfig](v, configPath)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
-	return cfg.Database, srvCfg, nil
+	return cfg.Database, nil
 }
 
 // readYamlAndSetupLogging reading the YAML config file of a service.

cmd/config/samples/dbinit.yaml

@@ -23,12 +23,13 @@ database:
     ca-cert-path: /root/artifacts/peerOrganizations/peer-org-0.com/msp/tlscacerts/tlspeer-org-0-CA-cert.pem
   # Maximum size of the database connection pool. Limits concurrent database operations.
   # Should be tuned based on database capacity and expected transaction load.
+  # For database initialization, a small pool is enough since it only performs schema initialization.
   # Default: 20
-  max-connections: 10
+  max-connections: 5
   # Minimum size of the database connection pool. Keeps connections warm for faster operations.
   # Should be less than or equal to max-connections.
   # Default: 1
-  min-connections: 5
+  min-connections: 1
   # Enable load balancing across multiple database endpoints. Should be enabled for
   # YugabyteDB clusters to distribute operations across nodes. Set to false for single-node deployments.
   load-balance: false

docker/images/test_node/run

@@ -43,6 +43,10 @@ if [[ $ops == *"db"* ]]; then
   docker-entrypoint.sh postgres -p 5433 &
 fi
 
+if [[ $ops == *"init-db"* ]]; then
+  "$BINS_PATH/committer" init-db --config "$CONFIGS_PATH/dbinit.yaml"
+fi
+
 if [[ $ops == *"orderer"* ]]; then
   "$BINS_PATH/mock"      start orderer     --config "$CONFIGS_PATH/mock-orderer.yaml" &
 fi
@@ -59,10 +63,6 @@ if [[ $ops == *"loadgen"* ]]; then
   "$BINS_PATH/loadgen" start --config "$CONFIGS_PATH/loadgen.yaml" &
 fi
 
-if [[ $ops == *"init-db"* ]]; then
-  "$BINS_PATH/committer" init-db --config "$CONFIGS_PATH/dbinit.yaml"
-fi
-
 # This just override's the postgres trap.
 function cleanup() {
   jobs

docker/test/common.go

@@ -62,6 +62,7 @@ type (
 		dbType            string
 		dbPassword        string
 		dbEndpointsString string
+		dbInitTimeout     string
 		cmd               []string
 		additionalEnvs    []string
 	}
@@ -102,44 +103,25 @@ func createAndStartContainerAndItsLogs(
 	ctx context.Context,
 	t *testing.T,
 	params createAndStartContainerParameters,
-) {
+) (<-chan container.WaitResponse, <-chan error) {
 	t.Helper()
-	dockerClient, id := createContainer(ctx, t, params)
-	require.NoError(t, dockerClient.ContainerStart(ctx, id, container.StartOptions{}))
-	//nolint:contextcheck // We want to ensure cleanup when the test is done.
-	t.Cleanup(func() {
-		stopAndRemoveContainerByID(context.Background(), t, dockerClient, id)
-	})
-	logs, err := dockerClient.ContainerLogs(ctx, id, container.LogsOptions{
-		ShowStdout: true,
-		ShowStderr: true,
-		Follow:     true,
-	})
+	dockerClient := createDockerClient(t)
+	resp, err := dockerClient.ContainerCreate(
+		ctx, params.config, params.hostConfig, nil, nil, params.name,
+	)
 	require.NoError(t, err)
-	go func() {
-		_, err = io.Copy(os.Stdout, logs)
-		if err != nil {
-			t.Logf("[%s] logs ended with: %v", params.name, err)
-		}
-	}()
-}
-
-// runContainerToCompletion creates, starts, and waits for a one-shot container to finish,
-// asserting a zero exit code.
-func runContainerToCompletion(
-	ctx context.Context,
-	t *testing.T,
-	params createAndStartContainerParameters,
-) {
-	t.Helper()
-	dockerClient, id := createContainer(ctx, t, params)
 	// Subscribe before starting.
 	// If the container finishes and is removed, before
 	// ContainerWait is called, the container ID no longer exists and the exit status is lost.
-	statusCh, errCh := dockerClient.ContainerWait(ctx, id, container.WaitConditionNotRunning)
-	require.NoError(t, dockerClient.ContainerStart(ctx, id, container.StartOptions{}))
-
-	logs, err := dockerClient.ContainerLogs(ctx, id, container.LogsOptions{
+	statusCh, errCh := dockerClient.ContainerWait(ctx, resp.ID, container.WaitConditionNextExit)
+	require.NoError(t, dockerClient.ContainerStart(ctx, resp.ID, container.StartOptions{}))
+	if !params.hostConfig.AutoRemove {
+		//nolint:contextcheck // We want to ensure cleanup when the test is done.
+		t.Cleanup(func() {
+			stopAndRemoveContainerByID(context.Background(), t, dockerClient, resp.ID)
+		})
+	}
+	logs, err := dockerClient.ContainerLogs(ctx, resp.ID, container.LogsOptions{
 		ShowStdout: true,
 		ShowStderr: true,
 		Follow:     true,
@@ -151,25 +133,7 @@ func runContainerToCompletion(
 			t.Logf("[%s] logs ended with: %v", params.name, err)
 		}
 	}()
-
-	select {
-	case err := <-errCh:
-		require.NoError(t, err)
-	case status := <-statusCh:
-		require.Zero(t, status.StatusCode, "container %s failed with exit code %d", params.name, status.StatusCode)
-	}
-}
-
-func createContainer(
-	ctx context.Context,
-	t *testing.T,
-	params createAndStartContainerParameters,
-) (*client.Client, string) {
-	t.Helper()
-	dockerClient := createDockerClient(t)
-	resp, err := dockerClient.ContainerCreate(ctx, params.config, params.hostConfig, nil, nil, params.name)
-	require.NoError(t, err)
-	return dockerClient, resp.ID
+	return statusCh, errCh
 }
 
 func monitorMetric(t *testing.T, metricsPort string, metricsTLS *connection.TLSConfig, waitForCount int) {

docker/test/container_release_image_test.go

@@ -44,7 +44,10 @@ const (
 
 	defaultDBPort     = "5433"
 	configFlag        = "--config"
+	timeoutFlag       = "--timeout"
 	containerRootUser = "0:0"
+
+	dbinit = "dbinit"
 )
 
 // enforcePostgresSSLAndReloadConfigScript enforces SSL-only client connections to a PostgreSQL
@@ -67,14 +70,8 @@ func TestCommitterReleaseImagesWithTLS(t *testing.T) {
 	ctx := t.Context()
 
 	t.Log("creating config-block")
-	v := config.NewViperWithLoadGenDefaults()
-	c, _, err := config.ReadLoadGenYamlAndSetupLogging(v, filepath.Join(localConfigPath, "loadgen.yaml"))
-	require.NoError(t, err)
-	c.LoadProfile.Policy.ArtifactsPath = t.TempDir()
-	_, err = workload.CreateOrExtendConfigBlockWithCrypto(&c.LoadProfile.Policy)
-	require.NoError(t, err)
+	artifactsPath := generateArtifactsPath(t)
 
-	dbInitNode := "dbinit"
 	committerNodes := []string{verifierService, vcService, queryService, coordinatorService, sidecarService}
 
 	for _, dbType := range []string{testdb.YugaDBType, testdb.PostgresDBType} {
@@ -93,8 +90,9 @@ func TestCommitterReleaseImagesWithTLS(t *testing.T) {
 					params := startNodeParameters{
 						networkName:   networkName,
 						tlsMode:       mode,
-						artifactsPath: c.LoadProfile.Policy.ArtifactsPath,
+						artifactsPath: artifactsPath,
 						dbType:        dbType,
+						dbInitTimeout: "30s",
 					}
 
 					for _, node := range append(committerNodes, dbService, ordererService, loadgenService) {
@@ -107,7 +105,8 @@ func TestCommitterReleaseImagesWithTLS(t *testing.T) {
 					// start a secured database node and return the db password.
 					params.dbPassword = startSecuredDatabaseNode(ctx, t, params.asNode(dbService))
 					// init the state DB and verify the operation succeeded.
-					runDatabaseInitWithReleaseImage(ctx, t, params.asNode(dbInitNode))
+					statusChannel, errChannel := runDatabaseInitWithReleaseImage(ctx, t, params.asNode(dbinit))
+					requireSuccessfulExecution(t, statusChannel, errChannel)
 					// start the orderer node.
 					startCommitterNodeWithTestImage(ctx, t, params.asNode(ordererService))
 					// start the committer nodes.
@@ -121,9 +120,7 @@ func TestCommitterReleaseImagesWithTLS(t *testing.T) {
 					// start the load generator node.
 					startLoadgenNodeWithReleaseImage(ctx, t, params.asNode(loadgenService))
 
-					metricsClientTLSConfig := test.NewServiceTLSConfig(
-						c.LoadProfile.Policy.ArtifactsPath, "loadgen", mode,
-					)
+					metricsClientTLSConfig := test.NewServiceTLSConfig(artifactsPath, "loadgen", mode)
 
 					monitorMetric(
 						t,
@@ -137,6 +134,29 @@ func TestCommitterReleaseImagesWithTLS(t *testing.T) {
 	}
 }
 
+// TestDatabaseInitFailureWithoutActiveDB tests that database initialization fails gracefully
+// when the database is not available, using a short timeout.
+func TestDatabaseInitFailureWithoutActiveDB(t *testing.T) {
+	t.Parallel()
+
+	params := startNodeParameters{
+		tlsMode:       connection.NoneTLSMode,
+		dbType:        "none_activated_database",
+		dbInitTimeout: "10s",
+		artifactsPath: generateArtifactsPath(t),
+	}
+	statusCh, errorCh := runDatabaseInitWithReleaseImage(t.Context(), t, params.asNode(dbinit))
+
+	// Expect the container to fail since there's no database available.
+	select {
+	case status := <-statusCh:
+		t.Logf("exited with status code: %v", status.StatusCode)
+		require.NotZero(t, status.StatusCode, "container should have failed but exited with code 0")
+	case err := <-errorCh:
+		require.Error(t, err, "container should have failed but exited with no error")
+	}
+}
+
 // CreateAndStartSecuredDatabaseNode creates a containerized YugabyteDB or PostgreSQL
 // database instance in a secure mode.
 func startSecuredDatabaseNode(ctx context.Context, t *testing.T, params startNodeParameters) string {
@@ -185,17 +205,25 @@ func startSecuredDatabaseNode(ctx context.Context, t *testing.T, params startNod
 }
 
 // runDatabaseInitWithReleaseImage runs init-db command in a temporary container.
-func runDatabaseInitWithReleaseImage(ctx context.Context, t *testing.T, params startNodeParameters) {
+func runDatabaseInitWithReleaseImage(
+	ctx context.Context, t *testing.T, params startNodeParameters,
+) (<-chan container.WaitResponse, <-chan error) {
 	t.Helper()
 
 	dbInitConfigPath := filepath.Join(containerConfigPath, params.node)
 	t.Logf("Starting %s as container with user %s.\n", committerReleaseImage, containerRootUser)
 
-	runContainerToCompletion(ctx, t, createAndStartContainerParameters{
+	statusCh, errCh := createAndStartContainerAndItsLogs(ctx, t, createAndStartContainerParameters{
 		config: &container.Config{
 			Image: committerReleaseImage,
-			Cmd:   []string{initDBCommand, configFlag, fmt.Sprintf("%s.yaml", dbInitConfigPath)},
-			User:  containerRootUser,
+			Cmd: []string{
+				initDBCommand,
+				configFlag,
+				fmt.Sprintf("%s.yaml", dbInitConfigPath),
+				timeoutFlag,
+				params.dbInitTimeout,
+			},
+			User: containerRootUser,
 			Env: []string{
 				"SC_DBINIT_DATABASE_PASSWORD=" + params.dbPassword,
 				"SC_DBINIT_DATABASE_USERNAME=" + params.dbUsername(),
@@ -208,14 +236,15 @@ func runDatabaseInitWithReleaseImage(ctx context.Context, t *testing.T, params s
 			Binds: []string{
 				fmt.Sprintf(
 					"%s.yaml:/%s.yaml",
-					filepath.Join(mustGetWD(t), localConfigPath, "dbinit"), dbInitConfigPath,
+					filepath.Join(mustGetWD(t), localConfigPath, params.node), dbInitConfigPath,
 				),
 				fmt.Sprintf("%s:%s", params.artifactsPath, containerArtifactsPath),
 			},
 			AutoRemove: true,
 		},
 		name: assembleContainerName(initDBCommand, params.tlsMode, params.dbType),
 	})
+	return statusCh, errCh
 }
 
 // startCommitterNodeWithReleaseImage starts a committer node using the release image.
@@ -365,10 +394,40 @@ func startCommitterNodeWithTestImage(
 	})
 }
 
+// generateArtifactsPath loads a loadgen config, create crypto materials and return their path.
+func generateArtifactsPath(t *testing.T) string {
+	t.Helper()
+	t.Log("creating config-block")
+	v := config.NewViperWithLoadGenDefaults()
+	c, _, err := config.ReadLoadGenYamlAndSetupLogging(v, filepath.Join(localConfigPath, "loadgen.yaml"))
+	require.NoError(t, err)
+	c.LoadProfile.Policy.ArtifactsPath = t.TempDir()
+	_, err = workload.CreateOrExtendConfigBlockWithCrypto(&c.LoadProfile.Policy)
+	require.NoError(t, err)
+	return c.LoadProfile.Policy.ArtifactsPath
+}
+
 // mustGetWD returns the current working directory.
 func mustGetWD(t *testing.T) string {
 	t.Helper()
 	wd, err := os.Getwd()
 	require.NoError(t, err)
 	return wd
 }
+
+func requireSuccessfulExecution(
+	t *testing.T, statusCh <-chan container.WaitResponse, errCh <-chan error,
+) {
+	t.Helper()
+	select {
+	case err := <-errCh:
+		require.NoError(t, err)
+	case status := <-statusCh:
+		require.Zero(
+			t,
+			status.StatusCode,
+			"container failed with exit code %d",
+			status.StatusCode,
+		)
+	}
+}

integration/runner/runtime.go

@@ -549,7 +549,6 @@ func (c *CommitterRuntime) ValidateExpectedResultsInCommittedBlock(t *testing.T,
 	persistedTxIDsStatus := make([]*committerpb.TxStatus, 0, len(expected.TxIDs))
 	duplicateTxIDsStatus := make([]*committerpb.TxStatus, 0, len(expected.TxIDs))
 	for i, tID := range expected.TxIDs {
-		//nolint:gosec // int -> uint32.
 		s := committerpb.NewTxStatus(expected.Statuses[i], tID, blk.Header.Number, uint32(i))
 		if s.Status == committerpb.Status_REJECTED_DUPLICATE_TX_ID {
 			duplicateTxIDsStatus = append(duplicateTxIDsStatus, s)

service/sidecar/test_exports.go

@@ -39,7 +39,6 @@ func RequireNotifications( //nolint:revive // argument-limit.
 		if !IsStatusStoredInDB(s) {
 			continue
 		}
-		//nolint:gosec // int -> uint32.
 		expected = append(expected, &committerpb.TxStatus{
 			Ref:    committerpb.NewTxRef(txIDs[i], expectedBlockNumber, uint32(i)),
 			Status: s,