K8SPG-1056 Detect PostgreSQL distribution and include it in telemetry#1664
Open
hors wants to merge 1 commit into
Open
K8SPG-1056 Detect PostgreSQL distribution and include it in telemetry#1664hors wants to merge 1 commit into
hors wants to merge 1 commit into
Conversation
Exec `postgres -V` in the database container alongside the existing
Patroni version check to determine whether the cluster runs Percona
Server for PostgreSQL ("percona") or upstream PostgreSQL ("community").
The result is stored in status.postgres.distribution and forwarded to
the telemetry metadata struct so the version service can receive it
once the API contract is extended on that end.
hors
requested review from
egegunes,
gkech,
mayankshah1607,
nmarukovich,
oksana-grishchenko and
pooknull
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds PostgreSQL distribution detection to the operator by executing postgres -V in the database container, storing the result in status.postgres.distribution, and threading that value into the telemetry metadata struct used by the version service integration.
Changes:
- Extend
PostgresStatuswith a newdistributionfield and update CRD schemas accordingly. - Detect distribution (
perconavscommunity) during Patroni version reconciliation viapostgres -Vexec. - Add
Distributionto the telemetry meta struct and populate it from cluster status.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go | Adds status.postgres.distribution field to the API type. |
| percona/version/version.go | Extends telemetry Meta struct with Distribution. |
| percona/controller/pgcluster/version.go | Populates telemetry meta Distribution from CR status. |
| percona/controller/pgcluster/patroniversion.go | Implements distribution detection and persists it to CR status. |
| deploy/cw-bundle.yaml | Updates CRD schema to include status.postgres.distribution. |
| deploy/crd.yaml | Updates CRD schema to include status.postgres.distribution. |
| deploy/bundle.yaml | Updates CRD schema to include status.postgres.distribution. |
| config/crd/bases/pgv2.percona.com_perconapgclusters.yaml | Updates base CRD schema for the new status field. |
| build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml | Updates generated CRD schema for the new status field. |
Comment on lines
+59
to
+62
| distribution, err := r.getPostgresDistribution(ctx, &p, naming.ContainerDatabase) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to get postgres distribution") | ||
| } |
Comment on lines
+332
to
+345
| func (r *PGClusterReconciler) getPostgresDistribution(ctx context.Context, pod *corev1.Pod, containerName string) (string, error) { | ||
| var stdout, stderr bytes.Buffer | ||
| execCli, err := clientcmd.NewClient() | ||
| if err != nil { | ||
| return "", errors.Wrap(err, "failed to create exec client") | ||
| } | ||
| if err := execCli.Exec(ctx, pod, containerName, nil, &stdout, &stderr, "postgres", "-V"); err != nil { | ||
| return "", errors.Wrap(err, "exec") | ||
| } | ||
| if strings.Contains(stdout.String(), "Percona Server for PostgreSQL") { | ||
| return "percona", nil | ||
| } | ||
| return "community", nil | ||
| } |
| return errors.Wrap(err, "failed to get patroni version") | ||
| } | ||
|
|
||
| distribution, err := r.getPostgresDistribution(ctx, &p, naming.ContainerDatabase) |
Collaborator
commit: 15d9300 |
egegunes
reviewed
Jul 1, 2026
egegunes
left a comment
egegunes
left a comment
Contributor
There was a problem hiding this comment.
LGTM in general, there's one copilot comment that needs to be addressed
Comment on lines
+332
to
+345
| func (r *PGClusterReconciler) getPostgresDistribution(ctx context.Context, pod *corev1.Pod, containerName string) (string, error) { | ||
| var stdout, stderr bytes.Buffer | ||
| execCli, err := clientcmd.NewClient() | ||
| if err != nil { | ||
| return "", errors.Wrap(err, "failed to create exec client") | ||
| } | ||
| if err := execCli.Exec(ctx, pod, containerName, nil, &stdout, &stderr, "postgres", "-V"); err != nil { | ||
| return "", errors.Wrap(err, "exec") | ||
| } | ||
| if strings.Contains(stdout.String(), "Percona Server for PostgreSQL") { | ||
| return "percona", nil | ||
| } | ||
| return "community", nil | ||
| } |
gkech
reviewed
Jul 1, 2026
| return "", errors.Wrap(err, "exec") | ||
| } | ||
| if strings.Contains(stdout.String(), "Percona Server for PostgreSQL") { | ||
| return "percona", nil |
Contributor
There was a problem hiding this comment.
instead of using hardcoded values, I would prefer these to be constants
| if err := execCli.Exec(ctx, pod, containerName, nil, &stdout, &stderr, "postgres", "-V"); err != nil { | ||
| return "", errors.Wrap(err, "exec") | ||
| } | ||
| if strings.Contains(stdout.String(), "Percona Server for PostgreSQL") { |
Contributor
There was a problem hiding this comment.
would also like to link through a comment which part of the docker image defines this particular string so we can have a refernece on the linking
| return pods, nil | ||
| } | ||
|
|
||
| func (r *PGClusterReconciler) getPostgresDistribution(ctx context.Context, pod *corev1.Pod, containerName string) (string, error) { |
Contributor
There was a problem hiding this comment.
So the idea is that everything that is not percona is community, we will never return an empty string, right?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Solution:
Exec
postgres -Vin the database container alongside the existing Patroni version check to determine whether the cluster runs Percona Server for PostgreSQL ("percona") or upstream PostgreSQL ("community"). The result is stored in status.postgres.distribution and forwarded to the telemetry metadata struct so the version service can receive it once the API contract is extended on that end.CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability