postgres database controllers#1760
Conversation
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 4 committers have signed the CLA. |
dbf7479 to
33a8d1d
Compare
| if apierrors.IsNotFound(getPGClusterErr) { | ||
| logger.Info("PostgresCluster deleted, skipping reconciliation") | ||
| return ctrl.Result{}, nil | ||
| } |
There was a problem hiding this comment.
Please move the logic to pkg/postgresql or pkg/splunk/postgresql . we tried to avoid doing changes in controller code as this code is mostly auto-generated. upgrade becomes hard as and when we might have to auto-generate the code
There was a problem hiding this comment.
logic moved, followed the pattern already used with other controllers
| // PostgresDatabaseReconciler reconciles a PostgresDatabase object | ||
| type PostgresDatabaseReconciler struct { | ||
| client.Client | ||
| Scheme *runtime.Scheme |
There was a problem hiding this comment.
add Recorder record.EventRecorder helps you generating events
| For(&enterprisev4.PostgresDatabase{}).WithEventFilter(predicate.GenerationChangedPredicate{}). | ||
| Owns(&cnpgv1.Database{}). | ||
| Owns(&corev1.Secret{}). | ||
| Named("postgresdatabase"). |
There was a problem hiding this comment.
missing how many concurrent reconciliation you want to do.
MaxConcurrentReconciles: enterpriseApi.TotalWorker
| return ctrl.NewControllerManagedBy(mgr). | ||
| For(&enterprisev4.PostgresDatabase{}).WithEventFilter(predicate.GenerationChangedPredicate{}). | ||
| Owns(&cnpgv1.Database{}). | ||
| Owns(&corev1.Secret{}). |
There was a problem hiding this comment.
I am unsure if you contorller owns secret here. check the other controllers for reference
Watches(&corev1.Secret{},
handler.EnqueueRequestForOwner(
mgr.GetScheme(),
mgr.GetRESTMapper(),
&enterpriseApi.ClusterManager{},
)).
also look into predicates
There was a problem hiding this comment.
we levarage predicates now for filtering event's out
| logger.Info("Finalizer removed, cleanup complete") | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
missing metric data collection, check other controllers. we might have to improve that code, but that explains what we are trying to do
There was a problem hiding this comment.
Will be working on this in the next sprint
| return ctrl.NewControllerManagedBy(mgr). | ||
| For(&enterprisev4.PostgresCluster{}). | ||
| Owns(&cnpgv1.Cluster{}). | ||
| Owns(&cnpgv1.Pooler{}). |
There was a problem hiding this comment.
I am unsure if PostgresCluster owns Pooler, configmap and secret here for all the Poolers/configmaps/secret. check example Watches in other contorller
There was a problem hiding this comment.
Yes, postgrescluster is owner of pooler, configmap and secrets. If I understand correctly, you use this pattern to control what we trigger:
Watches(&corev1.Pod{},
handler.EnqueueRequestForOwner(
mgr.GetScheme(),
mgr.GetRESTMapper(),
&enterpriseApiV3.LicenseMaster{},
)).
and this is an equivalent to Owns we used. Isnt it?
| postgresCluster := &enterprisev4.PostgresCluster{} | ||
|
|
||
| BeforeEach(func() { | ||
| By("creating the custom resource for the Kind PostgresCluster") |
There was a problem hiding this comment.
Add a test case where you do not own the postgresql database but they also use postgresql operator to create and manage database. how will this controller respond to it.
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // ManagedRole represents a PostgreSQL role to be created and managed in the cluster. |
There was a problem hiding this comment.
add annotations for maintenance mode support, where we do not want operator to do anything when customer /support is debugging postgresql database/cluster
There was a problem hiding this comment.
Do you mean <kind>.enterprise.splunk.com/paused annotations or maintenance_mode field in IDX/SHC spec?
There was a problem hiding this comment.
Could you please share the reference doc for me to understand this pattern and what and how we should use this mechanism?
| logger.Info("PostgresCluster is being deleted, cleanup complete") | ||
| return ctrl.Result{}, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
add support for maintenance mode where if user has set annotations you will return without running any logic in reconciliation loop
There was a problem hiding this comment.
any reference, how to achieve this?
| Instances *int32 `json:"instances,omitempty"` | ||
|
|
||
| // Storage is the size of persistent volume for each instance. | ||
| // Cannot be decreased after cluster creation (PostgreSQL limitation). |
There was a problem hiding this comment.
Could there be a validation for that?
There was a problem hiding this comment.
It can, but it will require k8s admission webhooks - we decided to postpone it for now, but we have a ticket for it. Do we have already standard how to use the admission webhooks with SOK?
There was a problem hiding this comment.
Yes, we have a standard for validating webhooks. https://github.com/splunk/splunk-operator/blob/develop/docs/ValidationWebhook.md
| // "restart" - tolerate brief downtime (suitable for development) | ||
| // "switchover" - minimal downtime via automated failover (production-grade) | ||
| // | ||
| // NOTE: When using "switchover", ensure clusterConfig.instances > 1. |
There was a problem hiding this comment.
Could there be a validation for that?
There was a problem hiding this comment.
yes, in admission webhook. Work postponed for now, but we have a ticket for in in our board
Version 3.0.0 was inadvertently removed by automated PR (commit 1139fcf) when 3.1.0 was added. Customers reported the version missing from `helm search repo` results. Changes: - Restored 3.0.0 packaged chart from git tag - Added 3.0.0 entries to docs/index.yaml for both splunk-enterprise and splunk-operator charts - Chart digest: db5890e3bcc95f9ca7298873cc08b4a5d1ee86ccb4ad4e5334a0ab5d7a5fdb5e Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update index.yaml with correct SHA256 digests matching actual tarballs: - splunk-enterprise-3.0.0: ae82f6c8edee4d827817fe6c9c6447c422a03c59595a0f6e779cef847a83b611 - splunk-operator-3.0.0: bd318b1f4022421a3fd429b186ca344c61d04a3c2bbdd5cc535d960773558e44 Resolves comment from vivekr-splunk on PR #1832 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit addresses four vulnerability findings from the container scanner: - VULN-71179: Upgrade github.com/aws/aws-sdk-go-v2/service/s3 from v1.84.1 to v1.99.0 - VULN-71171: Upgrade go.opentelemetry.io/otel/sdk from v1.40.0 to v1.43.0 - VULN-71183: Upgrade github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream from v1.6.11 to v1.7.8 - VULN-71182: Upgrade Go stdlib from v1.25.8 to v1.26.2 All dependency updates meet or exceed the minimum versions required by the container scanner. Unit tests pass with 83.6% composite coverage. Jira: VULN-71179, VULN-71171, VULN-71183, VULN-71182 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Update Go version references in build configuration files to match the go.mod update for VULN-71182 compliance. Files updated: - Dockerfile: golang:1.25.8 -> golang:1.26.2 - Dockerfile.distroless: golang:1.25.8 -> golang:1.26.2 - .env: GO_VERSION=1.25.8 -> GO_VERSION=1.26.2 GitHub Actions workflows automatically use GO_VERSION from .env, so no workflow file changes are needed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add PostgreSQL observability telemetry exposure
Introduce a feature gate registry backed by k8s.io/component-base/featuregate so new capabilities can be gated behind --feature-gates=<Gate>=true|false with Alpha/Beta/GA lifecycle. Migrate the validation webhook toggle from the ENABLE_VALIDATION_WEBHOOK env var to a ValidationWebhook feature gate with backwards-compatible env var support. - Add pkg/config/featuregates.go registry with SetupFeatureGates and IsFeatureGateEnabled helpers, plus unit tests - Wire feature gate flags into cmd/main.go and log effective gate states at startup - Use os.LookupEnv for ENABLE_VALIDATION_WEBHOOK so the deprecation warning fires whenever the env var is set, not just when it equals "true" - Update kustomization files with SPLUNK_GENERAL_TERMS value - Add docs/FeatureGates.md with usage and instructions for adding new gates - Update docs/ValidationWebhook.md with CLI-over-env-var precedence note
2eb9368 to
34097e9
Compare
logging changed, pureness fix attempt removed redundant sync at the end incremental state building with limited redundancy logging align cluster unit adj merge adjustments event emmision placed back, fortified with tests allign with requirements on state building review and rebase changes merge alignment review changes
* Embed values * Embed values * Move fixtures * Fix copyrights
Fix container scan vulnerabilities (VULN-71179, VULN-71171, VULN-71183, VULN-71182)
Restore helm chart version 3.0.0 to repository index
Add splunkOperator.featureGates map to values so operators can enable feature gates via helm install/upgrade without per-gate chart modifications. A helper template formats the map into a single --feature-gates=Key1=true,Key2=false arg; the deployment template conditionally includes it when the map is non-empty. Also introduces helm-unittest with Makefile targets (helm-lint, helm-test, helm-check) and fixes a pre-existing nindent bug in the extraEnvs rendering.
Cluster sync adj in p&a flavour
…gic-patch-cleanup Cleanup introduced cluster smelly returns
…nk-operator into feature/database-controllers
* pg_hba validation * comment fix * webhook resources fix * integration tests * fix tests * structure fix * fix tests * docs fix
d64c9dd to
22e4978
Compare
9ff69a7 to
4f42692
Compare
Description
Introduces the MVP PostgreSQL contoller stack to the Splunk Operator. Rather than
implementing PostgreSQL cluster management from scratch, this PR builds a Kubernetes-native
wrapper on top of CloudNativePG (CNPG) — a production-grade
PostgreSQL operator. The Splunk Operator layer adds three new CRDs and two new controllers
that translate high-level declarative intent (cluster classes, logical databases, managed
roles) into CNPG primitives, while handling the Splunk-specific lifecycle concerns:
credential provisioning, connection metadata publishing, and multi-phase status tracking.
Key Changes
api/v4/postgresclusterclass_types.go(new)Defines the Go API schema for
PostgresClusterClass— a cluster-scoped CRD thatstandardises PostgreSQL cluster configuration (instance count, storage, Postgres version,
resource limits, pg_hba rules, connection pooler) across environments. Operators define
devandprodclasses; teams reference them by name.api/v4/postgrescluster_types.go(new)Defines the Go API schema for
PostgresCluster— the namespaced CRD teams use to requesta PostgreSQL cluster. Declares spec fields for merging class defaults with per-cluster
overrides, and status fields for provisioner reference, connection pooler state, and
managed role tracking.
api/v4/postgresdatabase_types.go(new)Defines the Go API schema for
PostgresDatabase— the CRD that declares one or morelogical databases on a
PostgresCluster, including reclaim policy and the list ofdatabase definitions. Status tracks per-database secret and ConfigMap references.
internal/controller/postgresoperator_common_types.go(new)Shared constants, phase enums, condition types, and condition reasons used across both
controllers. Single source of truth for retry delays, finalizer names, endpoint suffixes,
and all status strings.
internal/controller/postgrescluster_controller.go(new)Reconciles
PostgresClusterCRs against CNPG:PostgresClusterspec into a CNPGcluster, merging any defaults inherited from the referenced
PostgresClusterClasswithper-cluster overrides
PgBouncer) lifecycle alongside the cluster,enabling high-throughput workloads to share a bounded pool of database connections
preventing orphaned infrastructure
ClusterReady,PoolerReady) so thePostgresDatabasecontroller and operators can wait on a confirmed healthy cluster beforeproceeding
internal/controller/postgresdatabase_controller.go(new)Reconciles
PostgresDatabaseCRs against CNPG:PostgresClusteris healthy before creating any database resources,so partial provisioning is never left in an inconsistent state
each role is scoped to its own database to enforce least-privilege access
Secrets,ready to be mounted by workloads that need database access
when available) into a
ConfigMapper database, giving consuming services a stablereference without hardcoding hostnames
deletion policy that controls whether the database is physically dropped when the CR is
deleted
ClusterReady,SecretsReady,ConfigMapsReady,UsersReady,DatabasesReady) so operators can track exactly which phase reconciliationis in at any point
cmd/main.goBoth new controllers registered with the manager.
config/rbac/Nine new RBAC files: admin, editor, and viewer roles for
PostgresCluster,PostgresClusterClass, andPostgresDatabase.config/samples/Five sample CRs:
devandprodcluster classes,defaultandoverrideclusterinstances, and a
PostgresDatabaseexample.Testing and Verification
Manual Testing
End-to-end flow verified by applying the full CR chain in order:
1. Verify the
PostgresClusteris ready2. Verify CNPG provisioned the underlying cluster
3. Verify the
PostgresDatabasestatus and conditions4. Verify CNPG
DatabaseCRs were created for each declared database5. Inspect Secrets and ConfigMaps created per database
6. Connect to the database using credentials and endpoints from the ConfigMap and Secret
Additional scenarios verified:
pooler-rw-hostandpooler-ro-hostkeys appear in ConfigMap only when the clusterhas connection pooling enabled
PostgresDatabaseCR triggers clean-up respecting thedeletionPolicydeclared per database (
Retainpreserves data,Deletedrops the database)Automated Tests — Planned Next
Unit and integration tests covering the full reconciliation lifecycle, error paths, and
status condition transitions are scheduled as the next deliverable for this feature branch.
Related Issues
Jira tickets, GitHub issues, Support tickets...
PR Checklist