Skip to content

Commit

Permalink
Address review comments - pass 1, add dbMigrationApproach attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
@rhkp
rhkp committed Oct 3, 2024
1 parent 0ee0e34 commit 0af328a
Show file tree
Hide file tree
Showing 24 changed files with 191 additions and 134 deletions.
9 changes: 6 additions & 3 deletions api/v1alpha08/sonataflow_persistence_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,13 @@ type PersistenceOptionsSpec struct {
// +optional
PostgreSQL *PersistencePostgreSQL `json:"postgresql,omitempty"`

// Whether to migrate database on service startup?
// DB Migration approach for service?
// job: use job based approach
// service: service itself shall migrate the db
// none: no db migration needed
// +optional
// +default: false
MigrateDBOnStartUp bool `json:"migrateDBOnStartUp"`
// +kubebuilder:default:=service
DBMigrationApproach string `json:"dbMigrationApproach,omitempty"`
}

// PersistencePostgreSQL configure postgresql connection for service(s).
Expand Down
10 changes: 0 additions & 10 deletions api/v1alpha08/sonataflowplatform_services_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,6 @@ import (

// ServicesPlatformSpec describes the desired service configuration for workflows without the `sonataflow.org/profile: dev` annotation.
type ServicesPlatformSpec struct {
// true = Use DB Migration Job with DB Migrator tool image
// false = Use built-in DB migration capability within services e.g. DI/JS, use MigrateDBOnStartUp flag
// +optional
// +default: true
JobBasedDbMigrationDI bool `json:"jobBasedDbMigrationDI,omitempty"`

// +optional
// +default: true
JobBasedDbMigrationJS bool `json:"jobBasedDbMigrationJS,omitempty"`

// Deploys the Data Index service for use by workflows without the `sonataflow.org/profile: dev` annotation.
// +optional
DataIndex *DataIndexServiceSpec `json:"dataIndex,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha08/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 16 additions & 13 deletions bundle/manifests/sonataflow.org_sonataflowplatforms.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -665,9 +665,14 @@ spec:
by default.
maxProperties: 2
properties:
migrateDBOnStartUp:
description: Whether to migrate database on service startup?
type: boolean
dbMigrationApproach:
default: service
description: |-
DB Migration approach for service?
job: use job based approach
service: service itself shall migrate the db
none: no db migration needed
type: string
postgresql:
description: Connect configured services to a postgresql
database.
Expand Down Expand Up @@ -8696,13 +8701,6 @@ spec:
type: string
type: object
type: object
jobBasedDbMigrationDI:
description: |-
true = Use DB Migration Job with DB Migrator tool image
false = Use built-in DB migration capability within services e.g. DI/JS, use MigrateDBOnStartUp flag
type: boolean
jobBasedDbMigrationJS:
type: boolean
jobService:
description: 'Deploys the Job service for use by workflows without
the `sonataflow.org/profile: dev` annotation.'
Expand All @@ -8716,9 +8714,14 @@ spec:
by default.
maxProperties: 2
properties:
migrateDBOnStartUp:
description: Whether to migrate database on service startup?
type: boolean
dbMigrationApproach:
default: service
description: |-
DB Migration approach for service?
job: use job based approach
service: service itself shall migrate the db
none: no db migration needed
type: string
postgresql:
description: Connect configured services to a postgresql
database.
Expand Down
11 changes: 8 additions & 3 deletions bundle/manifests/sonataflow.org_sonataflows.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1997,9 +1997,14 @@ spec:
for the workflow
maxProperties: 2
properties:
migrateDBOnStartUp:
description: Whether to migrate database on service startup?
type: boolean
dbMigrationApproach:
default: service
description: |-
DB Migration approach for service?
job: use job based approach
service: service itself shall migrate the db
none: no db migration needed
type: string
postgresql:
description: Connect configured services to a postgresql database.
maxProperties: 2
Expand Down
29 changes: 16 additions & 13 deletions config/crd/bases/sonataflow.org_sonataflowplatforms.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -665,9 +665,14 @@ spec:
by default.
maxProperties: 2
properties:
migrateDBOnStartUp:
description: Whether to migrate database on service startup?
type: boolean
dbMigrationApproach:
default: service
description: |-
DB Migration approach for service?
job: use job based approach
service: service itself shall migrate the db
none: no db migration needed
type: string
postgresql:
description: Connect configured services to a postgresql
database.
Expand Down Expand Up @@ -8696,13 +8701,6 @@ spec:
type: string
type: object
type: object
jobBasedDbMigrationDI:
description: |-
true = Use DB Migration Job with DB Migrator tool image
false = Use built-in DB migration capability within services e.g. DI/JS, use MigrateDBOnStartUp flag
type: boolean
jobBasedDbMigrationJS:
type: boolean
jobService:
description: 'Deploys the Job service for use by workflows without
the `sonataflow.org/profile: dev` annotation.'
Expand All @@ -8716,9 +8714,14 @@ spec:
by default.
maxProperties: 2
properties:
migrateDBOnStartUp:
description: Whether to migrate database on service startup?
type: boolean
dbMigrationApproach:
default: service
description: |-
DB Migration approach for service?
job: use job based approach
service: service itself shall migrate the db
none: no db migration needed
type: string
postgresql:
description: Connect configured services to a postgresql
database.
Expand Down
11 changes: 8 additions & 3 deletions config/crd/bases/sonataflow.org_sonataflows.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1997,9 +1997,14 @@ spec:
for the workflow
maxProperties: 2
properties:
migrateDBOnStartUp:
description: Whether to migrate database on service startup?
type: boolean
dbMigrationApproach:
default: service
description: |-
DB Migration approach for service?
job: use job based approach
service: service itself shall migrate the db
none: no db migration needed
type: string
postgresql:
description: Connect configured services to a postgresql database.
maxProperties: 2
Expand Down
2 changes: 1 addition & 1 deletion container-builder/api/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/controller/platform/dbMigratorJob.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@ func NewDBMigratorJobData(ctx context.Context, client client.Client, platform *o
quarkusDatasourceJobsservicePassword := ""
quarkusFlywayJobsserviceSchemas := ""

migrateDbDataindex := services.IsJobBasedDBMigrationDI(platform)
migrateDbDataindex := pshDI.IsJobsBasedDBMigration()
if migrateDbDataindex {
quarkusDatasourceDataindexJdbcUrl = platform.Spec.Services.DataIndex.Persistence.PostgreSQL.JdbcUrl
quarkusDatasourceDataindexUsername, _ = services.GetSecretKeyValueString(ctx, client, platform.Spec.Services.DataIndex.Persistence.PostgreSQL.SecretRef.Name, platform.Spec.Services.DataIndex.Persistence.PostgreSQL.SecretRef.UserKey, platform)
quarkusDatasourceDataindexPassword, _ = services.GetSecretKeyValueString(ctx, client, platform.Spec.Services.DataIndex.Persistence.PostgreSQL.SecretRef.Name, platform.Spec.Services.DataIndex.Persistence.PostgreSQL.SecretRef.PasswordKey, platform)
quarkusFlywayDataindexSchemas = getDBSchemaName(platform.Spec.Services.DataIndex.Persistence.PostgreSQL, "defaultDi")
}
migrateDbJobsservice := services.IsJobBasedDBMigrationJS(platform)
migrateDbJobsservice := pshJS.IsJobsBasedDBMigration()
if migrateDbJobsservice {
quarkusDatasourceJobsserviceJdbcUrl = platform.Spec.Services.JobService.Persistence.PostgreSQL.JdbcUrl
quarkusDatasourceJobsserviceUsername, _ = services.GetSecretKeyValueString(ctx, client, platform.Spec.Services.JobService.Persistence.PostgreSQL.SecretRef.Name, platform.Spec.Services.JobService.Persistence.PostgreSQL.SecretRef.UserKey, platform)
Expand Down
29 changes: 1 addition & 28 deletions internal/controller/platform/k8s.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,34 +61,7 @@ func (action *serviceAction) CanHandle(platform *operatorapi.SonataFlowPlatform)
return platform.Status.IsReady()
}

// checkNReportInconsistentDBMigrationFlags emit warning in logs, if Job based migration is true and also DI/JS migrateDBOnStartUp is true, does not return error
func (action *serviceAction) checkNReportInconsistentDBMigrationFlags(platform *operatorapi.SonataFlowPlatform, pshDI services.PlatformServiceHandler, pshJS services.PlatformServiceHandler) {
isJobBasedDBMigrationDI := services.IsJobBasedDBMigrationDI(platform)
isJobBasedDBMigrationJS := services.IsJobBasedDBMigrationJS(platform)
isJobBasedDBMigration := isJobBasedDBMigrationDI || isJobBasedDBMigrationJS

migrateDBOnStartUpDI := false
migrateDBOnStartUpJS := false
if pshDI.IsPersistenceSetInSpec() {
migrateDBOnStartUpDI = platform.Spec.Services.DataIndex.Persistence.MigrateDBOnStartUp
}

if pshJS.IsPersistenceSetInSpec() {
migrateDBOnStartUpJS = platform.Spec.Services.JobService.Persistence.MigrateDBOnStartUp
}

isServiceBasedDBMigration := migrateDBOnStartUpDI || migrateDBOnStartUpJS

// Check if both job based and service based db migration is specified
if (isJobBasedDBMigrationDI && migrateDBOnStartUpDI) || (isJobBasedDBMigrationJS && migrateDBOnStartUpJS) {
klog.V(log.W).InfoS("Both job based DB migration and service based DB migration flags detected, which may cause unexpected errors or behaviours, please check SonataFlowPlatform deployment: ", "jobBasedDBMigrationDI", isJobBasedDBMigrationDI, "jobBasedDBMigrationJS", isJobBasedDBMigrationJS, "migrateDBOnStartUpDI", migrateDBOnStartUpDI, "migrateDBOnStartUpJS", migrateDBOnStartUpJS)
} else if !isJobBasedDBMigration && !isServiceBasedDBMigration {
klog.V(log.I).InfoS("No job based or service based db migration flags specified, the services will expect the tables needed by them in the configured database: ", "jobBasedDBMigrationDI", isJobBasedDBMigrationDI, "jobBasedDBMigrationJS", isJobBasedDBMigrationJS, "migrateDBOnStartUpDI", migrateDBOnStartUpDI, "migrateDBOnStartUpJS", migrateDBOnStartUpJS)
}
}

func (action *serviceAction) createOrUpdateDBMigrationJob(ctx context.Context, client client.Client, platform *operatorapi.SonataFlowPlatform, pshDI services.PlatformServiceHandler, pshJS services.PlatformServiceHandler) error {
action.checkNReportInconsistentDBMigrationFlags(platform, pshDI, pshJS)
dbMigratorJob, err := NewDBMigratorJobData(ctx, action.client, platform, pshDI, pshJS)
if err != nil {
klog.V(log.E).InfoS("Error extracting db-migration job data: ", "error", err)
Expand Down Expand Up @@ -131,7 +104,7 @@ func (action *serviceAction) Handle(ctx context.Context, platform *operatorapi.S
psJS := services.NewJobServiceHandler(platform)

// Invoke DB Migration only if both or either DI/JS services are requested, in addition to jobBasedDBMigration
if (services.IsJobBasedDBMigrationDI(platform) || services.IsJobBasedDBMigrationJS(platform)) && (psDI.IsServiceSetInSpec() || psJS.IsServiceSetInSpec()) {
if (psDI.IsServiceSetInSpec() && psDI.IsJobsBasedDBMigration()) || (psJS.IsServiceSetInSpec() && psJS.IsJobsBasedDBMigration()) {
klog.V(log.I).InfoS("Starting DB Migration Job: ")
err := action.createOrUpdateDBMigrationJob(ctx, action.client, platform, psDI, psJS)
if err != nil {
Expand Down
Loading

0 comments on commit 0af328a

Please sign in to comment.