From de70522b528e84f4a56a7ffab4d7461a90573270 Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Thu, 29 Feb 2024 12:26:31 +0200 Subject: [PATCH] issue-729, PG cluster configurations added to sync job --- .secrets.baseline | 14 +- apis/clusterresources/v1beta1/structs.go | 8 - .../v1beta1/zz_generated.deepcopy.go | 21 --- apis/clusters/v1beta1/postgresql_types.go | 22 ++- controllers/clusters/postgresql_controller.go | 150 +++++++++++------- ...pi_postgre_sql_configuration_v2_service.go | 5 +- 6 files changed, 116 insertions(+), 104 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 76297bc58..6d77b357e 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -147,7 +147,7 @@ "filename": "apis/clusterresources/v1beta1/structs.go", "hashed_secret": "e03127a337f643c1c0eb2b0e8683e9140e19120d", "is_verified": false, - "line_number": 65 + "line_number": 57 } ], "apis/clusterresources/v1beta1/zz_generated.deepcopy.go": [ @@ -156,7 +156,7 @@ "filename": "apis/clusterresources/v1beta1/zz_generated.deepcopy.go", "hashed_secret": "44e17306b837162269a410204daaa5ecee4ec22c", "is_verified": false, - "line_number": 548 + "line_number": 547 } ], "apis/clusters/v1beta1/cadence_types.go": [ @@ -301,21 +301,21 @@ "filename": "apis/clusters/v1beta1/postgresql_types.go", "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db", "is_verified": false, - "line_number": 369 + "line_number": 370 }, { "type": "Secret Keyword", "filename": "apis/clusters/v1beta1/postgresql_types.go", "hashed_secret": "a3d7d4a96d18c8fc5a1cf9c9c01c45b4690b4008", "is_verified": false, - "line_number": 375 + "line_number": 376 }, { "type": "Secret Keyword", "filename": "apis/clusters/v1beta1/postgresql_types.go", "hashed_secret": "a57ce131bd944bdf8ba2f2f93e179dc416ed0315", "is_verified": false, - "line_number": 438 + "line_number": 439 } ], "apis/clusters/v1beta1/redis_types.go": [ @@ -556,7 +556,7 @@ "filename": "controllers/clusters/postgresql_controller.go", "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db", "is_verified": false, - "line_number": 1221 + "line_number": 1202 } ], "controllers/clusters/zookeeper_controller_test.go": [ @@ -1128,5 +1128,5 @@ } ] }, - "generated_at": "2024-02-28T14:20:52Z" + "generated_at": "2024-02-29T10:26:26Z" } diff --git a/apis/clusterresources/v1beta1/structs.go b/apis/clusterresources/v1beta1/structs.go index 7f4632506..69b7440ed 100644 --- a/apis/clusterresources/v1beta1/structs.go +++ b/apis/clusterresources/v1beta1/structs.go @@ -17,8 +17,6 @@ limitations under the License. package v1beta1 import ( - "encoding/json" - "github.com/instaclustr/operator/pkg/apiextensions" "k8s.io/apimachinery/pkg/types" @@ -38,12 +36,6 @@ type PeeringStatus struct { CDCID string `json:"cdcId,omitempty"` } -type PatchRequest struct { - Operation string `json:"op"` - Path string `json:"path"` - Value json.RawMessage `json:"value"` -} - type FirewallRuleSpec struct { ClusterID string `json:"clusterId,omitempty"` Type string `json:"type"` diff --git a/apis/clusterresources/v1beta1/zz_generated.deepcopy.go b/apis/clusterresources/v1beta1/zz_generated.deepcopy.go index e19cde096..9591a5b12 100644 --- a/apis/clusterresources/v1beta1/zz_generated.deepcopy.go +++ b/apis/clusterresources/v1beta1/zz_generated.deepcopy.go @@ -22,7 +22,6 @@ limitations under the License. package v1beta1 import ( - "encoding/json" "github.com/instaclustr/operator/pkg/apiextensions" "k8s.io/apimachinery/pkg/runtime" ) @@ -1551,26 +1550,6 @@ func (in *Operation) DeepCopy() *Operation { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PatchRequest) DeepCopyInto(out *PatchRequest) { - *out = *in - if in.Value != nil { - in, out := &in.Value, &out.Value - *out = make(json.RawMessage, len(*in)) - copy(*out, *in) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PatchRequest. -func (in *PatchRequest) DeepCopy() *PatchRequest { - if in == nil { - return nil - } - out := new(PatchRequest) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PeeringSpec) DeepCopyInto(out *PeeringSpec) { *out = *in diff --git a/apis/clusters/v1beta1/postgresql_types.go b/apis/clusters/v1beta1/postgresql_types.go index c7de6362d..fd7f10e0b 100644 --- a/apis/clusters/v1beta1/postgresql_types.go +++ b/apis/clusters/v1beta1/postgresql_types.go @@ -285,7 +285,8 @@ func (pgs *PgSpec) IsEqual(iPG PgSpec) bool { return pgs.GenericClusterSpec.Equals(&iPG.GenericClusterSpec) && pgs.SynchronousModeStrict == iPG.SynchronousModeStrict && pgs.DCsEqual(iPG.DataCentres) && - slices.EqualsUnordered(pgs.Extensions, iPG.Extensions) + slices.EqualsUnordered(pgs.Extensions, iPG.Extensions) && + pgs.ClusterConfigurationsEqual(iPG.ClusterConfigurations) } func (pgs *PgSpec) DCsEqual(instaModels []*PgDataCentre) bool { @@ -550,10 +551,10 @@ func (pdc *PgDataCentre) PGBouncerFromInstAPI(instaModels []*models.PGBouncer) { } } -func (pgs *PgSpec) ClusterConfigurationsFromInstAPI(instaModels []*models.ClusterConfigurations) { +func (pgs *PgSpec) ClusterConfigurationsFromInstAPI(instaModels []*models.ConfigurationProperties) { pgs.ClusterConfigurations = make(map[string]string, len(instaModels)) for _, instaModel := range instaModels { - pgs.ClusterConfigurations[instaModel.ParameterName] = instaModel.ParameterValue + pgs.ClusterConfigurations[instaModel.Name] = instaModel.Value } } @@ -595,3 +596,18 @@ func (pgs *PgStatus) DCsEqual(o []*PgDataCentreStatus) bool { return true } + +func (pgs *PgSpec) ClusterConfigurationsEqual(configs map[string]string) bool { + if len(pgs.ClusterConfigurations) != len(configs) { + return false + } + + for k, v := range pgs.ClusterConfigurations { + param, ok := configs[k] + if !ok || v != param { + return false + } + } + + return true +} diff --git a/controllers/clusters/postgresql_controller.go b/controllers/clusters/postgresql_controller.go index 43125caeb..bdd9ca5b9 100644 --- a/controllers/clusters/postgresql_controller.go +++ b/controllers/clusters/postgresql_controller.go @@ -227,7 +227,7 @@ func (r *PostgreSQLReconciler) handleCreateCluster( err := r.createCluster(ctx, pg, l) if err != nil { r.EventRecorder.Eventf(pg, models.Warning, models.CreationFailed, - "Failed to create PostgreSQL cluster. Reason: %w", err, + "Failed to create PostgreSQL cluster. Reason: %v", err, ) return reconcile.Result{}, err } @@ -250,9 +250,9 @@ func (r *PostgreSQLReconciler) handleCreateCluster( return reconcile.Result{}, err } - err = r.startClusterStatusJob(pg) + err = r.StartSyncJob(pg) if err != nil { - l.Error(err, "Cannot start PostgreSQL cluster status check job", + l.Error(err, "Cannot start PostgreSQL sync job", "cluster ID", pg.Status.ID, ) @@ -269,6 +269,25 @@ func (r *PostgreSQLReconciler) handleCreateCluster( "Cluster sync job is started", ) + err = r.startClusterConfigurationsJob(pg) + if err != nil { + l.Error(err, "Cannot start PostgreSQL cluster configurations job", + "cluster ID", pg.Status.ID, + ) + + r.EventRecorder.Eventf( + pg, models.Warning, models.CreationFailed, + "Cluster configurations job is failed. Reason: %v", + err, + ) + return reconcile.Result{}, err + } + + r.EventRecorder.Eventf( + pg, models.Normal, models.Created, + "Cluster configurations job is started", + ) + if pg.Spec.DataCentres[0].CloudProvider == models.ONPREMISES { return models.ExitReconcile, nil } @@ -318,6 +337,11 @@ func (r *PostgreSQLReconciler) handleUpdateCluster(ctx context.Context, pg *v1be iPg := &v1beta1.PostgreSQL{} iPg.FromInstAPI(instaModel) + err = r.mergeClusterConfigurationsFromInstAPI(pg.Status.ID, iPg) + if err != nil { + return reconcile.Result{}, err + } + if pg.Annotations[models.ExternalChangesAnnotation] == models.True || r.RateLimiter.NumRequeues(req) == rlimiter.DefaultMaxTries { return handleExternalChanges[v1beta1.PgSpec](r.EventRecorder, r.Client, pg, iPg, l) @@ -405,8 +429,9 @@ func (r *PostgreSQLReconciler) handleUpdateCluster(ctx context.Context, pg *v1be ) } + patch := pg.NewPatch() pg.Annotations[models.ResourceStateAnnotation] = models.UpdatedEvent - err = r.patchClusterMetadata(ctx, pg, l) + err = r.Patch(ctx, pg, patch) if err != nil { l.Error(err, "Cannot patch PostgreSQL resource metadata", "cluster name", pg.Spec.Name, @@ -533,9 +558,10 @@ func (r *PostgreSQLReconciler) handleDeleteCluster( r.Scheduler.RemoveJob(pg.GetJobID(scheduler.BackupsChecker)) r.Scheduler.RemoveJob(pg.GetJobID(scheduler.SyncJob)) + patch := pg.NewPatch() controllerutil.RemoveFinalizer(pg, models.DeletionFinalizer) pg.Annotations[models.ResourceStateAnnotation] = models.DeletedEvent - err = r.patchClusterMetadata(ctx, pg, l) + err = r.Patch(ctx, pg, patch) if err != nil { l.Error( err, "Cannot patch PostgreSQL resource metadata after finalizer removal", @@ -654,8 +680,9 @@ func (r *PostgreSQLReconciler) handleUpdateDefaultUserPassword( return reconcile.Result{}, err } + patch := pg.NewPatch() pg.Annotations[models.ResourceStateAnnotation] = models.UpdatedEvent - err = r.patchClusterMetadata(ctx, pg, l) + err = r.Patch(ctx, pg, patch) if err != nil { l.Error(err, "Cannot patch PostgreSQL resource metadata", "cluster name", pg.Spec.Name, @@ -695,8 +722,8 @@ func (r *PostgreSQLReconciler) startClusterOnPremisesIPsJob(pg *v1beta1.PostgreS return nil } -func (r *PostgreSQLReconciler) startClusterStatusJob(pg *v1beta1.PostgreSQL) error { - job := r.newWatchStatusJob(pg) +func (r *PostgreSQLReconciler) StartSyncJob(pg *v1beta1.PostgreSQL) error { + job := r.newSyncJob(pg) err := r.Scheduler.ScheduleJob(pg.GetJobID(scheduler.SyncJob), scheduler.ClusterStatusInterval, job) if err != nil { @@ -717,7 +744,7 @@ func (r *PostgreSQLReconciler) startClusterBackupsJob(pg *v1beta1.PostgreSQL) er return nil } -func (r *PostgreSQLReconciler) newWatchStatusJob(pg *v1beta1.PostgreSQL) scheduler.Job { +func (r *PostgreSQLReconciler) newSyncJob(pg *v1beta1.PostgreSQL) scheduler.Job { l := log.Log.WithValues("syncJob", pg.GetJobID(scheduler.SyncJob), "clusterID", pg.Status.ID) return func() error { @@ -796,9 +823,12 @@ func (r *PostgreSQLReconciler) newWatchStatusJob(pg *v1beta1.PostgreSQL) schedul } } - equals := pg.Spec.IsEqual(iPg.Spec) + err = r.mergeClusterConfigurationsFromInstAPI(pg.Status.ID, iPg) + if err != nil { + return fmt.Errorf("failed to get cluster configurations, err: %w", err) + } - if equals && pg.Annotations[models.ExternalChangesAnnotation] == models.True { + if pg.Annotations[models.ExternalChangesAnnotation] == models.True && pg.Spec.IsEqual(iPg.Spec) { patch := pg.NewPatch() delete(pg.Annotations, models.ExternalChangesAnnotation) err := r.Patch(context.Background(), pg, patch) @@ -811,7 +841,7 @@ func (r *PostgreSQLReconciler) newWatchStatusJob(pg *v1beta1.PostgreSQL) schedul ) } else if pg.Status.CurrentClusterOperationStatus == models.NoOperation && pg.Annotations[models.ResourceStateAnnotation] != models.UpdatingEvent && - !equals { + !pg.Spec.IsEqual(iPg.Spec) { patch := pg.NewPatch() pg.Annotations[models.ExternalChangesAnnotation] = models.True @@ -1166,55 +1196,6 @@ func (r *PostgreSQLReconciler) reconcileClusterConfigurations( return nil } -func (r *PostgreSQLReconciler) patchClusterMetadata( - ctx context.Context, - pgCluster *v1beta1.PostgreSQL, - l logr.Logger, -) error { - patchRequest := []*v1beta1.PatchRequest{} - - annotationsPayload, err := json.Marshal(pgCluster.Annotations) - if err != nil { - return err - } - - annotationsPatch := &v1beta1.PatchRequest{ - Operation: models.ReplaceOperation, - Path: models.AnnotationsPath, - Value: json.RawMessage(annotationsPayload), - } - patchRequest = append(patchRequest, annotationsPatch) - - finalizersPayload, err := json.Marshal(pgCluster.Finalizers) - if err != nil { - return err - } - - finzlizersPatch := &v1beta1.PatchRequest{ - Operation: models.ReplaceOperation, - Path: models.FinalizersPath, - Value: json.RawMessage(finalizersPayload), - } - patchRequest = append(patchRequest, finzlizersPatch) - - patchPayload, err := json.Marshal(patchRequest) - if err != nil { - return err - } - - err = r.Patch(ctx, pgCluster, client.RawPatch(types.JSONPatchType, patchPayload)) - if err != nil { - return err - } - - l.Info("PostgreSQL cluster patched", - "Cluster name", pgCluster.Spec.Name, - "Finalizers", pgCluster.Finalizers, - "Annotations", pgCluster.Annotations, - ) - return nil -} - func (r *PostgreSQLReconciler) findSecretObject(secret client.Object) []reconcile.Request { s := secret.(*k8sCore.Secret) @@ -1352,3 +1333,50 @@ func (r *PostgreSQLReconciler) handleExternalDelete(ctx context.Context, pg *v1b return nil } + +func (r *PostgreSQLReconciler) mergeClusterConfigurationsFromInstAPI(id string, iPG *v1beta1.PostgreSQL) error { + iConfigs, err := r.API.GetPostgreSQLConfigs(id) + if err != nil { + return err + } + + for _, config := range iConfigs { + iPG.Spec.ClusterConfigurationsFromInstAPI(config.ConfigurationProperties) + } + + return nil +} + +const pgClusterConfigurationsJob = "pgClusterConfigurations" + +func (r *PostgreSQLReconciler) startClusterConfigurationsJob(pg *v1beta1.PostgreSQL) error { + job := r.newClusterConfigurationsJob(pg) + + return r.Scheduler.ScheduleJob(pg.GetJobID(pgClusterConfigurationsJob), scheduler.ClusterStatusInterval, job) +} + +func (r *PostgreSQLReconciler) newClusterConfigurationsJob(pg *v1beta1.PostgreSQL) scheduler.Job { + return func() error { + err := r.Get(context.Background(), client.ObjectKeyFromObject(pg), pg) + if err != nil { + return err + } + + if pg.Status.State != models.RunningStatus { + return nil + } + + err = r.reconcileClusterConfigurations(pg.Status.ID, pg.Spec.ClusterConfigurations, nil) + if err != nil { + return err + } + + r.Scheduler.RemoveJob(pg.GetJobID(pgClusterConfigurationsJob)) + + r.EventRecorder.Event(pg, models.Normal, models.Created, + "Cluster was configured", + ) + + return nil + } +} diff --git a/pkg/instaclustr/mock/server/go/api_postgre_sql_configuration_v2_service.go b/pkg/instaclustr/mock/server/go/api_postgre_sql_configuration_v2_service.go index ab01d51fa..fff7ed4d8 100644 --- a/pkg/instaclustr/mock/server/go/api_postgre_sql_configuration_v2_service.go +++ b/pkg/instaclustr/mock/server/go/api_postgre_sql_configuration_v2_service.go @@ -31,10 +31,7 @@ func (s *PostgreSQLConfigurationV2APIService) ClusterManagementV2DataSourcesPost // TODO - update ClusterManagementV2DataSourcesPostgresqlClusterClusterIdConfigurationsGet with the required logic for this service method. // Add api_postgre_sql_configuration_v2_service.go to the .openapi-generator-ignore to avoid overwriting this service implementation when updating open api generation. - // TODO: Uncomment the next line to return response Response(200, []PostgresqlConfigurationPropertiesV2{}) or use other options such as http.Ok ... - // return Response(200, []PostgresqlConfigurationPropertiesV2{}), nil - - return Response(http.StatusNotImplemented, nil), errors.New("ClusterManagementV2DataSourcesPostgresqlClusterClusterIdConfigurationsGet method not implemented") + return Response(200, []PostgresqlConfigurationPropertiesV2{}), nil } // ClusterManagementV2ResourcesApplicationsPostgresqlConfigurationsV2ConfigurationIdDelete - Reset a configuration