refactor by reviews

Author: anencore94

Makefile

@@ -59,7 +59,7 @@ update:
 
 # Deploy Katib v1beta1 manifests using Kustomize into a k8s cluster.
 deploy:
-	bash scripts/v1beta1/deploy.sh
+	bash scripts/v1beta1/deploy.sh $(WITH_DATABASE_TYPE)
 
 # Undeploy Katib v1beta1 manifests using Kustomize from a k8s cluster
 undeploy:

manifests/v1beta1/components/postgres/postgres.yaml

@@ -22,7 +22,7 @@ spec:
     spec:
       containers:
         - name: katib-postgres
-          image: postgres:14.3-alpine
+          image: postgres:14.5-alpine
           envFrom:
             - secretRef:
                 name: katib-postgres-secrets

manifests/v1beta1/installs/katib-standalone-postgres/kustomization.yaml

@@ -32,15 +32,10 @@ images:
   - name: docker.io/kubeflowkatib/cert-generator
     newName: docker.io/kubeflowkatib/cert-generator
     newTag: latest
-patches:
-  - patch: |-
-      - op: replace
-        path: /spec/template/spec/containers/0/env
-        value:
-          - name: DB_NAME
-            value: "postgres"
-          - name: DB_PASSWORD
-            value: "katib"
-    target:
+patchesJson6902:
+  - target:
+      group: apps
+      version: v1
       kind: Deployment
-      labelSelector: "katib.kubeflow.org/component=db-manager"
+      name: katib-db-manager
+    path: ./patches/db-manager.yaml

manifests/v1beta1/installs/katib-standalone-postgres/patches/db-manager.yaml

@@ -0,0 +1,8 @@
+---
+- op: replace
+  path: /spec/template/spec/containers/0/env
+  value:
+    - name: DB_NAME
+      value: "postgres"
+    - name: DB_PASSWORD
+      value: "katib"

pkg/db/v1beta1/common/connection.go

@@ -34,7 +34,6 @@ func OpenSQLConn(driverName string, dataSourceName string, interval time.Duratio
 		select {
 		case <-ticker.C:
 			if db, err := sql.Open(driverName, dataSourceName); err == nil {
-				// if db, err := sql.Open(driverName, dataSourceName); err == nil {
 				if err = db.Ping(); err == nil {
 					return db, nil
 				}

pkg/db/v1beta1/common/const.go

@@ -16,7 +16,12 @@ limitations under the License.
 
 package common
 
+import "time"
+
 const (
+	ConnectInterval = 5 * time.Second
+	ConnectTimeout  = 60 * time.Second
+
 	DBUserEnvName     = "DB_USER"
 	DBNameEnvName     = "DB_NAME"
 	DBPasswordEnvName = "DB_PASSWORD"

pkg/db/v1beta1/mysql/mysql.go

@@ -38,9 +38,6 @@ const (
 	//dbNameTmpl   = "root:%s@tcp(%s:%s)/%s?timeout=5s"
 	dbNameTmpl   = "%s:%s@tcp(%s:%s)/%s?timeout=5s"
 	mysqlTimeFmt = "2006-01-02 15:04:05.999999"
-
-	connectInterval = 5 * time.Second
-	connectTimeout  = 60 * time.Second
 )
 
 type dbConn struct {
@@ -77,7 +74,7 @@ func NewWithSQLConn(db *sql.DB) (common.KatibDBInterface, error) {
 }
 
 func NewDBInterface() (common.KatibDBInterface, error) {
-	db, err := common.OpenSQLConn(dbDriver, getDbName(), connectInterval, connectTimeout)
+	db, err := common.OpenSQLConn(dbDriver, getDbName(), common.ConnectInterval, common.ConnectTimeout)
 	if err != nil {
 		return nil, fmt.Errorf("DB open failed: %v", err)
 	}

pkg/db/v1beta1/postgres/postgres.go

@@ -33,12 +33,7 @@ import (
 	"k8s.io/klog"
 )
 
-const (
-	dbDriver = "postgres"
-
-	connectInterval = 5 * time.Second
-	connectTimeout  = 60 * time.Second
-)
+const dbDriver = "postgres"
 
 type dbConn struct {
 	db *sql.DB
@@ -65,7 +60,7 @@ func getDbName() string {
 }
 
 func NewDBInterface() (common.KatibDBInterface, error) {
-	db, err := common.OpenSQLConn(dbDriver, getDbName(), connectInterval, connectTimeout)
+	db, err := common.OpenSQLConn(dbDriver, getDbName(), common.ConnectInterval, common.ConnectTimeout)
 	if err != nil {
 		return nil, fmt.Errorf("DB open failed: %v", err)
 	}
@@ -160,7 +155,7 @@ func (d *dbConn) GetObservationLog(trialName string, metricName string, startTim
 		formattedEndTime := e_time.UTC().Format(time.RFC3339Nano)
 		qstr += fmt.Sprintf(" AND time <= $%d", index_of_qparam)
 		qfield = append(qfield, formattedEndTime)
-		// index_of_qparam += 1
+		// index_of_qparam += 1  // if any other filters are added, this should be incremented
 	}
 
 	rows, err := d.db.Query(base_stmt+qstr+" ORDER BY time", qfield...)

test/e2e/v1beta1/scripts/gh-actions/setup-katib.sh

@@ -62,17 +62,14 @@ if "$DEPLOY_TRAINING_OPERATOR"; then
 fi
 
 echo "Deploying Katib"
-cd ../../../../../ && bash scripts/v1beta1/deploy.sh "$WITH_DATABASE_TYPE" && cd -
+cd ../../../../../ && WITH_DATABASE_TYPE=$WITH_DATABASE_TYPE make deploy && cd -
 
 # Wait until all Katib pods is running.
 TIMEOUT=120s
 kubectl wait --for=condition=complete --timeout=${TIMEOUT} -l katib.kubeflow.org/component=cert-generator -n kubeflow job ||
   (kubectl get pods -n kubeflow && kubectl describe pods -n kubeflow && exit 1)
 
-kubectl wait --for=condition=ready --timeout=${TIMEOUT} -l "katib.kubeflow.org/component in ($WITH_DATABASE_TYPE)" -n kubeflow pod ||
-  (kubectl get pods -n kubeflow && kubectl describe pods -n kubeflow && exit 1)
-
-kubectl wait --for=condition=ready --timeout=${TIMEOUT} -l "katib.kubeflow.org/component in (controller,db-manager,ui)" -n kubeflow pod ||
+kubectl wait --for=condition=ready --timeout=${TIMEOUT} -l "katib.kubeflow.org/component in ($WITH_DATABASE_TYPE,controller,db-manager,ui)" -n kubeflow pod ||
   (kubectl get pods -n kubeflow && kubectl describe pods -n kubeflow && exit 1)
 
 # Wait until all Katib pods is actually ready.