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

Take volume snapshot after full backup #3970

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

dsessler7
Copy link
Contributor

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change, as applicable?
  • Have you tested your changes on all related environments with successful results, as applicable?
    • Have you added automated tests?

Type of Changes:

  • New feature
  • Bug fix
  • Documentation
  • Testing enhancement
  • Other

What is the current behavior (link to any open issues here)?

We currently do not have any volume snapshot capability.

What is the new behavior (if this is a feature change)?

  • Breaking change (fix or feature that would cause existing functionality to change)

If the feature is turned on, both via feature gate and postgrescluster spec, the operator will now take a volume snapshot of the primary pgdata volume after a full backup completes. The operator will keep one ReadyToUse snapshot around, deleting older snapshots.

Other Information:

@dsessler7 dsessler7 changed the title Clone from snapshot Take volume snapshot after full backup Aug 15, 2024
@@ -9,6 +9,9 @@ PGMONITOR_DIR ?= hack/tools/pgmonitor
PGMONITOR_VERSION ?= v4.11.0
QUERIES_CONFIG_DIR ?= hack/tools/queries

EXTERNAL_SNAPSHOTTER_DIR ?= hack/tools/external-snapshotter
EXTERNAL_SNAPSHOTTER_VERSION ?= v8.0.1
Copy link
Member

Choose a reason for hiding this comment

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

🤔 Interesting. go get pulls v8.0.0 because of the client/v8.0.0 tag here.

I wonder if we can somehow use the downloaded Go module for this.

Copy link
Member

Choose a reason for hiding this comment

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

I see that CRDs are downloaded with the Go module:

$ go list -f '{{.Dir}}' -m all | grep external
~/go/pkg/mod/github.com/kubernetes-csi/external-snapshotter/client/[email protected]

$ ls -1 ~/go/pkg/mod/github.com/kubernetes-csi/external-snapshotter/client/[email protected]/config/crd/
groupsnapshot.storage.k8s.io_volumegroupsnapshotclasses.yaml
groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml
groupsnapshot.storage.k8s.io_volumegroupsnapshots.yaml
kustomization.yaml
snapshot.storage.k8s.io_volumesnapshotclasses.yaml
snapshot.storage.k8s.io_volumesnapshotcontents.yaml
snapshot.storage.k8s.io_volumesnapshots.yaml

But, I don't see module information during tests, very likely because of https://www.github.com/golang/go/issues/33976.

	info, ok := debug.ReadBuildInfo()
	assert.Assert(t, ok)
	t.Logf("%#v", info)
test.go:14: &debug.BuildInfo{GoVersion:"go1.22.4", Path:"", Main:debug.Module{Path:"", Version:"", Sum:"", Replace:(*debug.Module)(nil)}, Deps:[]*debug.Module(nil), Settings:[]debug.BuildSetting(nil)}

@@ -270,6 +273,7 @@ func PGBackRestCronJobLabels(clusterName, repoName, backupType string) labels.Se
cronJobLabels := map[string]string{
LabelPGBackRestRepo: repoName,
LabelPGBackRestCronJob: backupType,
LabelPGBackRestBackup: string(BackupScheduled),
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

Comment on lines +244 to +246
for _, job := range jobs.Items {
if job.Status.Succeeded > 0 &&
latestCompleteBackupJob.Status.CompletionTime.Before(job.Status.CompletionTime) {
Copy link
Member

Choose a reason for hiding this comment

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

🌱 This almost looks like an opportunity for something in slices, slices.MinFunc, maybe? 🤔 I'm not certain the following is any more legible...

untested:

if len(jobs.Items) == 0 {
  return nil
}
job := slices.MinFunc(jobs.Items, func(a, b batchv1.Job) int {
  if a.Succeeded < 1 {
    return 1
  }
  if b.Succeeded < 1 {
    return -1
  }
  return a.Status.CompletionTime.Compare(b.Status.CompletionTime)
})
if job.Succeeded < 1 {
  return nil
}
return &job

…a volume after a every backup. Manage snapshots so that only one ReadyToUse snapshot is kept. Add/adjust tests for volume snapshots feature. Add volume snapshot crds directory to envtest environment setup.
@dsessler7 dsessler7 merged commit 20cc36d into CrunchyData:master Aug 21, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants