Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying sidecars and env vars for the repo host #1080

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jsierles
Copy link
Contributor

@jsierles jsierles commented Mar 11, 2025

CHANGE DESCRIPTION

Problem:

The repo host env vars and sidecars were not configurable.

Solution:

This PR simply adds support for env vars and sidecars in the Postgres CRD. Changes to the Helm chart are needed, and will come in a separate PR.

CHECKLIST

Jira

  • Is the Jira ticket created and referenced properly?
  • Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
  • Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

  • Is an E2E test/test case added for the new feature/change?
  • Are unit tests added where appropriate?

Config/Logging/Testability

  • Are all needed new/changed options added to default YAML files?
  • Are all needed new/changed options added to the Helm Chart?
  • Did we add proper logging messages for operator actions?
  • Did we ensure compatibility with the previous version or cluster upgrade process?
  • Does the change support oldest and newest supported PG version?
  • Does the change support oldest and newest supported Kubernetes version?

@CLAassistant
Copy link

CLAassistant commented Mar 11, 2025

CLA assistant check
All committers have signed the CLA.


// Environment variables to be set in the pgBackRest repo host container.
// +optional
Env []corev1.EnvVar `json:"env,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good day, @jsierles. What do you think about using Secret for this need? We have it for our PXC operator https://github.com/percona/percona-xtradb-cluster-operator/blob/main/deploy/cr.yaml#L472 I think it is better because env vars can have sensitive information and it is not good to set values via CR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, we can add a separate secret option for each component (not just for the pgBackRest repo host) as we have in PXCO. This could be useful for you in the future as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems fine. In our case we're not setting sensitive information, but we can change it to use secrets.

@hors hors added the community label Mar 11, 2025
@JNKPercona
Copy link
Collaborator

Test name Status
custom-extensions passed
custom-tls passed
demand-backup passed
finalizers passed
init-deploy passed
monitoring passed
one-pod passed
operator-self-healing passed
pitr passed
scaling passed
scheduled-backup failure
self-healing passed
sidecars passed
start-from-backup passed
tablespaces passed
telemetry-transfer passed
upgrade-consistency passed
upgrade-minor passed
users passed
We run 19 out of 19

commit: e878215
image: perconalab/percona-postgresql-operator:PR-1080-e878215c9

Copy link
Collaborator

@hors hors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsierles JFYI, we are almost finished with the development cycle for the MySQL operator. For the next two or even three weeks, we will be focusing on PGO. If you want to include more changes in PGO 2.7.0, please try to submit PRs during this period.

@jsierles
Copy link
Contributor Author

OK, we will submit! Will there be a safe upgrade path from 2.5.0 to 2.7.0? We're debating now with which version to launch. Right now the challenge seems to be about the CRD updates potentially causing issues (like sidecars being replaced with containers).

@hors
Copy link
Collaborator

hors commented Mar 11, 2025

OK, we will submit! Will there be a safe upgrade path from 2.5.0 to 2.7.0? We're debating now with which version to launch. Right now the challenge seems to be about the CRD updates potentially causing issues (like sidecars being replaced with containers).

PGO v2.6.0 will be available tomorrow, and I suggest using the latest version.
All new options take effect only when using the new CR version. For example, you can run Operator 2.6.0 with CR version 2.5.0, and all existing options will continue to work. Once you upgrade the CR version, you can start using the new options.

Regarding updates, we recommend that users follow a sequential upgrade path (e.g., 2.5.0 → 2.6.0 → 2.7.0) rather than skipping versions. Additionally, we aim to deprecate old options gradually rather than replacing them outright.

@jsierles
Copy link
Contributor Author

"CR version" refers to the deploy/crd.yaml file for each release?

@hors
Copy link
Collaborator

hors commented Mar 11, 2025

"CR version" refers to the deploy/crd.yaml file for each release?

I'm referring to this line. Previously, versioning was handled differently, but now we maintain a single version in CRDs and manage headline versioning within the operator code.

The main goal is to prevent unnecessary restarts when updating the operator. For example, if a new environment variable is introduced in version 2.7.0, without crVersion checking, all clusters managed by the operator would restart as soon as the operator image is updated. However, by handling this in the operator code, the new environment variable is only added when crVersion >= 2.7.0.

This means that updating the operator image alone won’t trigger cluster restarts or introduce new settings unexpectedly. When you're ready for a database update, you can manually bump crVersion and make any additional changes in the CR, triggering a controlled restart where all new options take effect.

This approach also allows us to manage feature availability e.g., Feature A can be enabled only for crVersion >= 2.7.0. As a result, different PostgreSQL cluster versions can coexist under the same operator, making updates more predictable and manageable.

@jsierles
Copy link
Contributor Author

jsierles commented Mar 11, 2025

OK - one question though. Can't only one version of the CRD exist in the k8s cluster? Or will specifying an earlier version simply ignore certain fields? And, does this mean you wouldn't need to run multiple operators, at least for a while? Right now we run cluster wide operators so want to be careful with updates that could affect many clusters.

@hors
Copy link
Collaborator

hors commented Mar 11, 2025

OK - one question though. Can't only one version of the CRD exist in the k8s cluster? Or will specifying an earlier version simply ignore certain fields? And, does this mean you wouldn't need to run multiple operators, at least for a while? Right now we run cluster wide operators so want to be careful with updates that could affect many clusters.

yes, CRD has a cluster scope, but the idea is that the latest version of CRD should work with the previous ver of the PG cluster. E.g., you can have CRD ver 2.7.0, and it should work with CR versions 2.7.0 , 2.6.0, and 2.5.0. We support the last three versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants