From f7b037c4b50f4877828c0e6dac5520b1ae9919eb Mon Sep 17 00:00:00 2001 From: rhkp Date: Mon, 30 Sep 2024 13:43:44 -0400 Subject: [PATCH] Further improvements and add jobs permissions --- .../sonataflowplatform_services_types.go | 10 +++++--- .../sonataflow.org_sonataflowplatforms.yaml | 6 +++-- .../sonataflow.org_sonataflowplatforms.yaml | 6 +++-- config/rbac/role.yaml | 12 ++++++++++ internal/controller/platform/dbMigratorJob.go | 4 ++-- internal/controller/platform/k8s.go | 24 ++++++++++++------- .../controller/platform/services/services.go | 11 +++++++-- .../sonataflowplatform_controller.go | 1 + operator.yaml | 18 ++++++++++++-- test/e2e/platform_test.go | 11 ++++----- .../01-sonataflow_platform.yaml | 3 ++- 11 files changed, 78 insertions(+), 28 deletions(-) diff --git a/api/v1alpha08/sonataflowplatform_services_types.go b/api/v1alpha08/sonataflowplatform_services_types.go index 903336c00..bb302e4d2 100644 --- a/api/v1alpha08/sonataflowplatform_services_types.go +++ b/api/v1alpha08/sonataflowplatform_services_types.go @@ -21,10 +21,14 @@ 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 + // false = Use built-in DB migration capability within services e.g. DI/JS, use MigrateDBOnStartUp flag // +optional - // +default: false - JobBasedDbMigration bool `json:"jobBasedDbMigration,omitempty"` + // +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 diff --git a/bundle/manifests/sonataflow.org_sonataflowplatforms.yaml b/bundle/manifests/sonataflow.org_sonataflowplatforms.yaml index ce8cfa33b..1a63d083a 100644 --- a/bundle/manifests/sonataflow.org_sonataflowplatforms.yaml +++ b/bundle/manifests/sonataflow.org_sonataflowplatforms.yaml @@ -8696,10 +8696,12 @@ spec: type: string type: object type: object - jobBasedDbMigration: + 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 + 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 diff --git a/config/crd/bases/sonataflow.org_sonataflowplatforms.yaml b/config/crd/bases/sonataflow.org_sonataflowplatforms.yaml index 99f7c5bf6..e29226011 100644 --- a/config/crd/bases/sonataflow.org_sonataflowplatforms.yaml +++ b/config/crd/bases/sonataflow.org_sonataflowplatforms.yaml @@ -8696,10 +8696,12 @@ spec: type: string type: object type: object - jobBasedDbMigration: + 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 + 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 diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 66e33fa72..167f61955 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -4,6 +4,18 @@ kind: ClusterRole metadata: name: manager-role rules: +- apiGroups: + - batch + resources: + - jobs + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - sonataflow.org resources: diff --git a/internal/controller/platform/dbMigratorJob.go b/internal/controller/platform/dbMigratorJob.go index 9a73f4f1e..d0d106e3d 100644 --- a/internal/controller/platform/dbMigratorJob.go +++ b/internal/controller/platform/dbMigratorJob.go @@ -92,14 +92,14 @@ func NewDBMigratorJobData(ctx context.Context, client client.Client, platform *o quarkusDatasourceJobsservicePassword := "" quarkusFlywayJobsserviceSchemas := "" - migrateDbDataindex := pshDI.IsServiceEnabledInSpec() + migrateDbDataindex := services.IsJobBasedDBMigrationDI(platform) 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 := pshJS.IsServiceEnabledInSpec() + migrateDbJobsservice := services.IsJobBasedDBMigrationJS(platform) 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) diff --git a/internal/controller/platform/k8s.go b/internal/controller/platform/k8s.go index 729c4fe07..51b707072 100644 --- a/internal/controller/platform/k8s.go +++ b/internal/controller/platform/k8s.go @@ -63,19 +63,27 @@ func (action *serviceAction) CanHandle(platform *operatorapi.SonataFlowPlatform) // 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) { - isJobBasedDBMigration := services.IsJobBasedDBMigration(platform) - diMigrateDBOnStartUp := false - jsMigrateDBOnStartUp := false + isJobBasedDBMigrationDI := services.IsJobBasedDBMigrationDI(platform) + isJobBasedDBMigrationJS := services.IsJobBasedDBMigrationJS(platform) + isJobBasedDBMigration := isJobBasedDBMigrationDI || isJobBasedDBMigrationJS + + migrateDBOnStartUpDI := false + migrateDBOnStartUpJS := false if pshDI.IsPersistenceSetInSpec() { - diMigrateDBOnStartUp = platform.Spec.Services.DataIndex.Persistence.MigrateDBOnStartUp + migrateDBOnStartUpDI = platform.Spec.Services.DataIndex.Persistence.MigrateDBOnStartUp } if pshJS.IsPersistenceSetInSpec() { - jsMigrateDBOnStartUp = platform.Spec.Services.JobService.Persistence.MigrateDBOnStartUp + migrateDBOnStartUpJS = platform.Spec.Services.JobService.Persistence.MigrateDBOnStartUp } - if isJobBasedDBMigration && (diMigrateDBOnStartUp || jsMigrateDBOnStartUp) { - klog.V(log.W).InfoS("Inconsistent DB migration flags detected and it may cause unexpected errors or behaviours, please check SonataFlowPlatform deployment: ", "jobBasedDBMigration", isJobBasedDBMigration, "diMigrateDBOnStartUp", diMigrateDBOnStartUp, "jsMigrateDBOnStartUp", jsMigrateDBOnStartUp) + 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) } } @@ -123,7 +131,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.IsJobBasedDBMigration(platform) && (psDI.IsServiceSetInSpec() || psJS.IsServiceSetInSpec()) { + if (services.IsJobBasedDBMigrationDI(platform) || services.IsJobBasedDBMigrationJS(platform)) && (psDI.IsServiceSetInSpec() || psJS.IsServiceSetInSpec()) { klog.V(log.I).InfoS("Starting DB Migration Job: ") err := action.createOrUpdateDBMigrationJob(ctx, action.client, platform, psDI, psJS) if err != nil { diff --git a/internal/controller/platform/services/services.go b/internal/controller/platform/services/services.go index ecefb4835..1490bd9f4 100644 --- a/internal/controller/platform/services/services.go +++ b/internal/controller/platform/services/services.go @@ -518,9 +518,16 @@ func isServicesSet(platform *operatorapi.SonataFlowPlatform) bool { return platform != nil && platform.Spec.Services != nil } -func IsJobBasedDBMigration(platform *operatorapi.SonataFlowPlatform) bool { +func IsJobBasedDBMigrationDI(platform *operatorapi.SonataFlowPlatform) bool { if platform != nil && platform.Spec.Services != nil { - return platform.Spec.Services.JobBasedDbMigration + return platform.Spec.Services.JobBasedDbMigrationDI + } + return false +} + +func IsJobBasedDBMigrationJS(platform *operatorapi.SonataFlowPlatform) bool { + if platform != nil && platform.Spec.Services != nil { + return platform.Spec.Services.JobBasedDbMigrationJS } return false } diff --git a/internal/controller/sonataflowplatform_controller.go b/internal/controller/sonataflowplatform_controller.go index b5fbbe269..8d680bee7 100644 --- a/internal/controller/sonataflowplatform_controller.go +++ b/internal/controller/sonataflowplatform_controller.go @@ -69,6 +69,7 @@ type SonataFlowPlatformReconciler struct { //+kubebuilder:rbac:groups=sonataflow.org,resources=sonataflowplatforms,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=sonataflow.org,resources=sonataflowplatforms/status,verbs=get;update;patch //+kubebuilder:rbac:groups=sonataflow.org,resources=sonataflowplatforms/finalizers,verbs=update +//+kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;update;patch;delete // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. diff --git a/operator.yaml b/operator.yaml index aad711e4d..bc960539f 100644 --- a/operator.yaml +++ b/operator.yaml @@ -9230,10 +9230,12 @@ spec: type: string type: object type: object - jobBasedDbMigration: + 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 + 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 @@ -27798,6 +27800,18 @@ kind: ClusterRole metadata: name: sonataflow-operator-manager-role rules: +- apiGroups: + - batch + resources: + - jobs + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - sonataflow.org resources: diff --git a/test/e2e/platform_test.go b/test/e2e/platform_test.go index ffcd1f278..bdc06d52d 100644 --- a/test/e2e/platform_test.go +++ b/test/e2e/platform_test.go @@ -93,7 +93,7 @@ var _ = Describe("Platform Use Cases :: ", Label("platform"), Ordered, func() { cmd.Stdin = bytes.NewBuffer(manifests) _, err := utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - + By("Wait for SonatatFlowPlatform CR to complete deployment") // wait for service deployments to be ready EventuallyWithOffset(1, func() error { @@ -101,7 +101,7 @@ var _ = Describe("Platform Use Cases :: ", Label("platform"), Ordered, func() { _, err = utils.Run(cmd) return err }, 10*time.Minute, 5).Should(Succeed()) - + By("Evaluate status of all service's health endpoint") cmd = exec.Command("kubectl", "get", "pod", "-l", "app.kubernetes.io/name in (jobs-service,data-index-service)", "-n", targetNamespace, "-ojsonpath={.items[*].metadata.name}") output, err := utils.Run(cmd) @@ -111,9 +111,8 @@ var _ = Describe("Platform Use Cases :: ", Label("platform"), Ordered, func() { } }) }) - + }) - var _ = Context("with platform services", func() { @@ -249,7 +248,7 @@ var _ = Describe("Platform Use Cases :: ", Label("platform"), Ordered, func() { cmd.Stdin = bytes.NewBuffer(manifests) _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - + By("Wait for SonatatFlowPlatform CR to complete deployment") // wait for service deployments to be ready EventuallyWithOffset(1, func() error { @@ -266,7 +265,7 @@ var _ = Describe("Platform Use Cases :: ", Label("platform"), Ordered, func() { verifyHealthStatusInPod(pn, targetNamespace) } }, - FEntry("Job Service and Data Index come up with job based db migration", test.GetSonataFlowE2EPlatformPersistenceSampleDataDirectory("job_based_db_migration")), + Entry("Job Service and Data Index come up with job based db migration", test.GetSonataFlowE2EPlatformPersistenceSampleDataDirectory("job_based_db_migration")), ) DescribeTable("when deploying a SonataFlowPlatform CR with brokers", func(testcaseDir string) { diff --git a/test/testdata/platform/persistence/job_based_db_migration/sonataflow-platform/01-sonataflow_platform.yaml b/test/testdata/platform/persistence/job_based_db_migration/sonataflow-platform/01-sonataflow_platform.yaml index 41dd0612c..3d57660cc 100644 --- a/test/testdata/platform/persistence/job_based_db_migration/sonataflow-platform/01-sonataflow_platform.yaml +++ b/test/testdata/platform/persistence/job_based_db_migration/sonataflow-platform/01-sonataflow_platform.yaml @@ -21,7 +21,8 @@ spec: strategyOptions: KanikoBuildCacheEnabled: "true" services: - jobBasedDbMigration: true + jobBasedDbMigrationDI: true + jobBasedDbMigrationJS: true dataIndex: enabled: true persistence: