Skip to content

Conversation

pooknull
Copy link
Contributor

@pooknull pooknull commented May 16, 2025

K8SPSMDB-1072 Powered by Pull Request Badge

https://perconadev.atlassian.net/browse/K8SPSMDB-1072

DESCRIPTION

This PR adds the following section to the .spec.backup.tasks[]:

        retention:
          count: 3 # same as "keep" when ".type: count" is used
          type: count # currently, this is the only valid value for this field
          deleteFromStorage: true # whether to add the "percona.com/delete-backup" finalizer to new backups

If both .keep and .retention.count are specified, the second will be used.

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?
  • Are OpenShift compare files changed for E2E tests (compare/*-oc.yml)?

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 MongoDB version?
  • Does the change support oldest and newest supported Kubernetes version?

@pull-request-size pull-request-size bot added the size/L 100-499 lines label May 16, 2025
@github-actions github-actions bot added the tests label May 16, 2025
Comment on lines 934 to 946
type BackupTaskSpecRetentionType string

const (
BackupTaskSpecRetentionTypeCount = "count"
)

type BackupTaskSpecRetention struct {
Count int `json:"count,omitempty"`
// +kubebuilder:validation:Enum={count}
Type string `json:"type,omitempty"`
DeleteFromStorage *bool `json:"deleteFromStorage,omitempty"`
}

Copy link
Contributor

@gkech gkech May 26, 2025

Choose a reason for hiding this comment

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

I believe that we could have a 1-1 mapping from keep to retention without using keep at all in our codebase in a similar way we implemented it in PXC. This way, when we will officially deprecate the keep option, we will not have to change the actual logic.

Check here: percona/percona-xtradb-cluster-operator@1553be6#diff-012d1e07c013a6332356a3ab856e8b0db6825aa9021d56c68ea5114faee14705

Copy link
Contributor Author

Choose a reason for hiding this comment

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

type BackupTaskSpec struct {
Name string `json:"name"`
Enabled bool `json:"enabled"`
Keep int `json:"keep,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mark keep as deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hors hors added this to the v1.21.0 milestone May 27, 2025
@pooknull pooknull marked this pull request as ready for review May 28, 2025 11:44
gkech
gkech previously approved these changes May 29, 2025
@JNKPercona
Copy link
Collaborator

Test name Status
arbiter passed
balancer passed
cross-site-sharded passed
custom-replset-name passed
custom-tls passed
custom-users-roles passed
custom-users-roles-sharded passed
data-at-rest-encryption passed
data-sharded passed
demand-backup passed
demand-backup-eks-credentials-irsa passed
demand-backup-fs passed
demand-backup-incremental passed
demand-backup-incremental-sharded passed
demand-backup-physical-parallel passed
demand-backup-physical-aws passed
demand-backup-physical-azure passed
demand-backup-physical-gcp passed
demand-backup-physical-minio passed
demand-backup-physical-sharded-parallel passed
demand-backup-physical-sharded-aws passed
demand-backup-physical-sharded-azure passed
demand-backup-physical-sharded-gcp passed
demand-backup-physical-sharded-minio passed
demand-backup-sharded passed
expose-sharded passed
finalizer passed
ignore-labels-annotations passed
init-deploy passed
ldap passed
ldap-tls passed
limits passed
liveness passed
mongod-major-upgrade passed
mongod-major-upgrade-sharded passed
monitoring-2-0 passed
monitoring-pmm3 passed
multi-cluster-service passed
multi-storage passed
non-voting-and-hidden passed
one-pod passed
operator-self-healing-chaos passed
pitr passed
pitr-physical passed
pitr-sharded passed
pitr-physical-backup-source passed
preinit-updates passed
pvc-resize passed
recover-no-primary passed
replset-overrides passed
rs-shard-migration passed
scaling passed
scheduled-backup passed
security-context passed
self-healing-chaos passed
service-per-pod passed
serviceless-external-nodes passed
smart-update passed
split-horizon passed
stable-resource-version passed
storage passed
tls-issue-cert-manager passed
upgrade passed
upgrade-consistency passed
upgrade-consistency-sharded-tls passed
upgrade-sharded passed
users passed
version-service passed
We run 68 out of 68

commit: 4ba045e
image: perconalab/percona-server-mongodb-operator:PR-1928-4ba045ea

@gkech
Copy link
Contributor

gkech commented May 30, 2025

We should also create a pr for adding these new options to our helm chart.

@hors hors merged commit 388aa92 into main Jun 2, 2025
17 checks passed
@hors hors deleted the K8SPSMDB-1072 branch June 2, 2025 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L 100-499 lines tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants