From e5ed615db1840db313f25531d59da06e0856ce34 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 | 12 +- apis/clusterresources/v1beta1/structs.go | 8 -- .../v1beta1/zz_generated.deepcopy.go | 21 ---- apis/clusters/v1beta1/postgresql_types.go | 23 +++- apis/clusters/v1beta1/postgresql_webhook.go | 45 +------ .../clusters/v1beta1/zz_generated.deepcopy.go | 11 -- .../clusters.instaclustr.com_postgresqls.yaml | 13 -- .../samples/clusters_v1beta1_postgresql.yaml | 9 +- controllers/clusters/postgresql_controller.go | 111 +++++++----------- ...pi_postgre_sql_configuration_v2_service.go | 5 +- pkg/models/errors.go | 1 + 11 files changed, 79 insertions(+), 180 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 76297bc58..c71078562 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -75,6 +75,10 @@ { "path": "detect_secrets.filters.allowlist.is_line_allowlisted" }, + { + "path": "detect_secrets.filters.common.is_baseline_file", + "filename": ".secrets.baseline" + }, { "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", "min_level": 2 @@ -147,7 +151,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 +160,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": [ @@ -556,7 +560,7 @@ "filename": "controllers/clusters/postgresql_controller.go", "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db", "is_verified": false, - "line_number": 1221 + "line_number": 1183 } ], "controllers/clusters/zookeeper_controller_test.go": [ @@ -1128,5 +1132,5 @@ } ] }, - "generated_at": "2024-02-28T14:20:52Z" + "generated_at": "2024-02-29T10:48:32Z" } 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..3a75d70e8 100644 --- a/apis/clusters/v1beta1/postgresql_types.go +++ b/apis/clusters/v1beta1/postgresql_types.go @@ -85,7 +85,6 @@ type PgSpec struct { //+kubebuilder:Validation:MinItems:=1 //+kubebuilder:Validation:MaxItems=2 DataCentres []*PgDataCentre `json:"dataCentres,omitempty"` - UserRefs []*Reference `json:"userRefs,omitempty" dcomparisonSkip:"true"` //+kubebuilder:validate:MaxItems:=1 ResizeSettings []*ResizeSettings `json:"resizeSettings,omitempty" dcomparisonSkip:"true"` Extensions PgExtensions `json:"extensions,omitempty"` @@ -285,7 +284,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 +550,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 +595,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/apis/clusters/v1beta1/postgresql_webhook.go b/apis/clusters/v1beta1/postgresql_webhook.go index 1cf7c6522..5e2a0ec06 100644 --- a/apis/clusters/v1beta1/postgresql_webhook.go +++ b/apis/clusters/v1beta1/postgresql_webhook.go @@ -21,7 +21,6 @@ import ( "fmt" "unicode" - k8sCore "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -99,13 +98,6 @@ func (pgv *pgValidator) ValidateCreate(ctx context.Context, obj runtime.Object) return err } - if pg.Spec.UserRefs != nil { - err = pgv.validatePostgreSQLUsers(ctx, pg) - if err != nil { - return err - } - } - appVersions, err := pgv.API.ListAppVersions(models.PgAppKind) if err != nil { return fmt.Errorf("cannot list versions for kind: %v, err: %w", @@ -159,6 +151,10 @@ func (pgv *pgValidator) ValidateCreate(ctx context.Context, obj runtime.Object) } } + if pg.Spec.ClusterConfigurations != nil { + return models.ErrPGClusterConfigurationsOnCreationNotAvailable + } + return nil } @@ -193,13 +189,6 @@ func (pgv *pgValidator) ValidateUpdate(ctx context.Context, old runtime.Object, return pgv.ValidateCreate(ctx, pg) } - if pg.Spec.UserRefs != nil { - err := pgv.validatePostgreSQLUsers(ctx, pg) - if err != nil { - return err - } - } - err := pg.Spec.ValidateImmutableFieldsUpdate(oldCluster.Spec) if err != nil { return fmt.Errorf("immutable fields validation error: %v", err) @@ -220,32 +209,6 @@ func (pgv *pgValidator) ValidateUpdate(ctx context.Context, old runtime.Object, return nil } -func (pgv *pgValidator) validatePostgreSQLUsers(ctx context.Context, pg *PostgreSQL) error { - nodeList := &k8sCore.NodeList{} - - err := pgv.K8sClient.List(ctx, nodeList, &client.ListOptions{}) - if err != nil { - return fmt.Errorf("cannot list node list, error: %v", err) - } - - var externalIPExists bool - for _, node := range nodeList.Items { - for _, nodeAddress := range node.Status.Addresses { - if nodeAddress.Type == k8sCore.NodeExternalIP { - externalIPExists = true - break - } - } - } - - if !externalIPExists || pg.Spec.PrivateNetwork { - return fmt.Errorf("cannot create PostgreSQL user, if your cluster is private or has no external ips " + - "you need to configure peering and remove user references from cluster specification") - } - - return nil -} - // ValidateDelete implements webhook.Validator so a webhook will be registered for the type func (pgv *pgValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error { pg, ok := obj.(*PostgreSQL) diff --git a/apis/clusters/v1beta1/zz_generated.deepcopy.go b/apis/clusters/v1beta1/zz_generated.deepcopy.go index ca231d5f8..46f538494 100644 --- a/apis/clusters/v1beta1/zz_generated.deepcopy.go +++ b/apis/clusters/v1beta1/zz_generated.deepcopy.go @@ -2199,17 +2199,6 @@ func (in *PgSpec) DeepCopyInto(out *PgSpec) { } } } - if in.UserRefs != nil { - in, out := &in.UserRefs, &out.UserRefs - *out = make([]*apiextensions.ObjectReference, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(apiextensions.ObjectReference) - **out = **in - } - } - } if in.ResizeSettings != nil { in, out := &in.ResizeSettings, &out.ResizeSettings *out = make([]*ResizeSettings, len(*in)) diff --git a/config/crd/bases/clusters.instaclustr.com_postgresqls.yaml b/config/crd/bases/clusters.instaclustr.com_postgresqls.yaml index c9fb1b389..9758c2207 100644 --- a/config/crd/bases/clusters.instaclustr.com_postgresqls.yaml +++ b/config/crd/bases/clusters.instaclustr.com_postgresqls.yaml @@ -313,19 +313,6 @@ spec: - email type: object type: array - userRefs: - items: - description: ObjectReference is namespaced reference to an object - properties: - name: - type: string - namespace: - type: string - required: - - name - - namespace - type: object - type: array version: type: string type: object diff --git a/config/samples/clusters_v1beta1_postgresql.yaml b/config/samples/clusters_v1beta1_postgresql.yaml index a75259523..39d32346e 100644 --- a/config/samples/clusters_v1beta1_postgresql.yaml +++ b/config/samples/clusters_v1beta1_postgresql.yaml @@ -38,16 +38,13 @@ spec: # - replicationMode: "ASYNCHRONOUS" # interDataCentreReplication: # - isPrimaryDataCentre: false - # clusterConfigurations: - # idle_in_transaction_session_timeout: "2" - # statement_timeout: "1" +# clusterConfigurations: +# idle_in_transaction_session_timeout: "2" +# statement_timeout: "1" # twoFactorDelete: # - email: "rostyslp@netapp.com" # description: "test 222" slaTier: "NON_PRODUCTION" -# userRefs: -# - namespace: default -# name: postgresqluser-sample privateNetwork: false synchronousModeStrict: false # extensions: diff --git a/controllers/clusters/postgresql_controller.go b/controllers/clusters/postgresql_controller.go index 43125caeb..a5087fc60 100644 --- a/controllers/clusters/postgresql_controller.go +++ b/controllers/clusters/postgresql_controller.go @@ -142,7 +142,7 @@ func (r *PostgreSQLReconciler) createFromRestore(pg *v1beta1.PostgreSQL, l logr. pg, models.Normal, models.Created, "Cluster restore request is sent. Original cluster ID: %s, new cluster ID: %s", pg.Spec.PgRestoreFrom.ClusterID, - pg.Status.ID, + id, ) instaModel, err := r.API.GetPostgreSQL(id) @@ -165,18 +165,18 @@ func (r *PostgreSQLReconciler) createPostgreSQL(pg *v1beta1.PostgreSQL, l logr.L return nil, fmt.Errorf("failed to create cluster, err: %v", err) } - r.EventRecorder.Eventf( - pg, models.Normal, models.Created, - "Cluster creation request is sent. Cluster ID: %s", - pg.Status.ID, - ) - var instaModel models.PGCluster err = json.Unmarshal(b, &instaModel) if err != nil { return nil, fmt.Errorf("failed to unmarshal body to models.PGCluster, err: %v", err) } + r.EventRecorder.Eventf( + pg, models.Normal, models.Created, + "Cluster creation request is sent. Cluster ID: %s", + instaModel.ID, + ) + return &instaModel, nil } @@ -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, ) @@ -318,6 +318,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 +410,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 +539,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 +661,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 +703,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 +725,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 +804,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 +822,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 +1177,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 +1314,16 @@ 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 +} 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 diff --git a/pkg/models/errors.go b/pkg/models/errors.go index 0fe345f36..fa8dea793 100644 --- a/pkg/models/errors.go +++ b/pkg/models/errors.go @@ -78,4 +78,5 @@ var ( ErrClusterIsNotReadyToUpdate = errors.New("cluster is not ready to update") ErrKubeVirtAddonNotFound = errors.New("cannot create KubeVirt based resources automatially without KubeVirt operator installed. Please install KubeVirt add-on") ErrOpenSearchNumberOfRacksInvalid = errors.New("number of racks should be between 2 and 5") + ErrPGClusterConfigurationsOnCreationNotAvailable = errors.New("clusterConfigurations is not available on cluster creation") )