Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Collapse generated files in GitHub diffs and omit text patches in Git diffs.
# Use git diff --text to inspect generated changes locally.

# make generate
/api/v1/zz_generated.deepcopy.go linguist-generated=true -diff
/pkg/client/applyconfiguration/** linguist-generated=true -diff

# make manifests
/config/crd/bases/*.yaml linguist-generated=true -diff
/config/rbac/role.yaml linguist-generated=true -diff
/config/webhook/manifests.yaml linguist-generated=true -diff

# make -C charts update-crds
/charts/cloudnative-pg/templates/crds/crds.yaml linguist-generated=true -diff

# make apidoc
/docs/src/cloudnative-pg.v1.md linguist-generated=true -diff
42 changes: 26 additions & 16 deletions internal/management/controller/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1165,22 +1165,9 @@ func (r *InstanceReconciler) reconcilePgBackRestConfig(ctx context.Context, clus
log.FromContext(ctx).Error(err, "Failed to rotate pgbackrest logs, will retry on next reconcile")
}

// Stanza creation is only needed on the primary — the stanza metadata
// lives in S3 and replicas access it directly from there. We re-run
// stanza-create whenever the stanza name changes (not just once), proactively
// initializes the new stanza instead of relying on the lazy WAL-archive
// recovery path. stanza-create is idempotent.
// While suspended, skip stanza-create so the cluster leaves no footprint in
// object storage; it runs on the next reconcile once the flag is cleared
// (e.g. on warm-pool adoption). archive_mode is untouched, so this is
// restart-free.
stanza := cluster.GetPgBackRestStanzaName()
if isPrimary && !cluster.IsPgBackRestSuspended() && stanzaCreateNeeded(r.pgBackRestStanzaCreated.Load(), stanza) {
if err := pgbackrest.StanzaCreate(ctx, stanza); err != nil {
log.FromContext(ctx).Error(err, "Failed to create pgbackrest stanza, will retry on next reconcile")
return nil
}
r.pgBackRestStanzaCreated.Store(&stanza)
if err := r.reconcilePgBackRestStanza(ctx, cluster, configChanged); err != nil {
log.FromContext(ctx).Error(err, "Failed to create pgbackrest stanza, will retry on next reconcile")
return nil
}

// Publish the generation whose pgbackrest configuration is now fully
Expand All @@ -1190,6 +1177,29 @@ func (r *InstanceReconciler) reconcilePgBackRestConfig(ctx context.Context, clus
return nil
}

// reconcilePgBackRestStanza initializes the repository after a configuration change.
// Failed initialization remains pending even after the config file is up to date.
func (r *InstanceReconciler) reconcilePgBackRestStanza(
ctx context.Context, cluster *apiv1.Cluster, configChanged bool,
) error {
if configChanged {
r.pgBackRestStanzaCreated.Store(nil)
}
if r.instance.GetPodName() != cluster.Status.CurrentPrimary || cluster.IsPgBackRestSuspended() {
return nil
}

stanza := cluster.GetPgBackRestStanzaName()
if !stanzaCreateNeeded(r.pgBackRestStanzaCreated.Load(), stanza) {
return nil
}
if err := pgbackrest.StanzaCreate(ctx, stanza); err != nil {
return err
}
r.pgBackRestStanzaCreated.Store(&stanza)
return nil
}

// reconcilePgBackRestTLSServer ensures that the server has loaded the current
// configuration. A failed reload remains pending because the file is already
// current and will not report another content change on the next reconcile.
Expand Down
6 changes: 2 additions & 4 deletions internal/management/controller/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@ type InstanceReconciler struct {
certificateReconciler certificateRefresher
pluginRepository repository.Interface

// pgBackRestStanzaCreated holds the name of the pgbackrest stanza that was
// last created by this manager. It is a pointer (not a bool) so that a
// change of stanza name triggers stanza-create for the new stanza instead
// of being skipped.
// pgBackRestStanzaCreated records successful initialization. Configuration
// changes clear it so a new repository is initialized under the same stanza.
pgBackRestStanzaCreated atomic.Pointer[string]
pgBackRestReloadPending atomic.Bool
pgBackRestTLSServer pgBackRestTLSServer
Expand Down
129 changes: 129 additions & 0 deletions internal/management/controller/pgbackrest_stanza_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,20 @@ SPDX-License-Identifier: Apache-2.0
package controller

import (
"context"
"os"
"path/filepath"
"strings"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"

apiv1 "github.com/xataio/xata-cnpg/api/v1"
"github.com/xataio/xata-cnpg/pkg/management/postgres"
"github.com/xataio/xata-cnpg/pkg/pgbackrest"
"github.com/xataio/xata-cnpg/pkg/utils"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
Expand All @@ -41,3 +53,120 @@ var _ = Describe("pgbackrest stanzaCreateNeeded", func() {
Expect(stanzaCreateNeeded(ptr.To("pool-cluster-xyz"), "branch-abc")).To(BeTrue())
})
})

func TestStanzaInitializationAfterConfigChange(t *testing.T) {
dir := t.TempDir()
calls := filepath.Join(dir, "calls")
fail := filepath.Join(dir, "fail")
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
t.Setenv("STANZA_CALLS", calls)
t.Setenv("STANZA_FAIL", fail)
binary := `#!/bin/sh
printf '%s\n' "$*" >> "$STANZA_CALLS"
if [ -f "$STANZA_FAIL" ]; then exit 1; fi
`
// #nosec G306 -- The fake executable needs owner execute permission inside t.TempDir.
if err := os.WriteFile(filepath.Join(dir, "pgbackrest"), []byte(binary), 0o700); err != nil {
t.Fatal(err)
}
cluster := &apiv1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "branch"},
Status: apiv1.ClusterStatus{CurrentPrimary: "branch-1"},
}
r := &InstanceReconciler{instance: postgres.NewInstance().WithPodName("branch-1")}
r.pgBackRestStanzaCreated.Store(ptr.To("branch"))

// An unchanged configuration does not initialize an existing stanza again.
if err := r.reconcilePgBackRestStanza(context.Background(), cluster, false); err != nil {
t.Fatal(err)
}
if _, err := os.Stat(calls); !os.IsNotExist(err) {
t.Fatal("unexpected stanza-create for unchanged configuration")
}

// Simulate a repository location change with the same stanza and a transient error.
if err := os.WriteFile(fail, nil, 0o600); err != nil {
t.Fatal(err)
}
if err := r.reconcilePgBackRestStanza(context.Background(), cluster, true); err == nil {
t.Fatal("expected initialization failure")
}
if r.pgBackRestStanzaCreated.Load() != nil {
t.Fatal("failed initialization must remain pending")
}
if err := os.Remove(fail); err != nil {
t.Fatal(err)
}

// The file is already updated on retry, but initialization must still run.
if err := r.reconcilePgBackRestStanza(context.Background(), cluster, false); err != nil {
t.Fatal(err)
}
if err := r.reconcilePgBackRestStanza(context.Background(), cluster, false); err != nil {
t.Fatal(err)
}
content, err := os.ReadFile(calls) // #nosec G304 -- Test-owned path inside t.TempDir.
if err != nil {
t.Fatal(err)
}
if strings.Count(string(content), "stanza-create") != 2 {
t.Fatalf("expected one failed attempt and one successful retry, got %s", content)
}
}

func TestStanzaInitializationDeferredAfterConfigChange(t *testing.T) {
for _, reason := range []string{"replica", "suspended"} {
t.Run(reason, func(t *testing.T) {
dir := t.TempDir()
calls := filepath.Join(dir, "calls")
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
t.Setenv("STANZA_CALLS", calls)
// #nosec G306 -- The fake executable needs owner execute permission inside t.TempDir.
if err := os.WriteFile(filepath.Join(dir, "pgbackrest"),
[]byte("#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"$STANZA_CALLS\"\n"), 0o700); err != nil {
t.Fatal(err)
}
cluster := &apiv1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "branch"},
Status: apiv1.ClusterStatus{CurrentPrimary: "branch-1"},
}
if reason == "replica" {
cluster.Status.CurrentPrimary = "branch-2"
} else {
cluster.Annotations = map[string]string{utils.PgBackRestSuspended: "enabled"}
}
r := &InstanceReconciler{instance: postgres.NewInstance().WithPodName("branch-1")}
r.pgBackRestStanzaCreated.Store(ptr.To("branch"))

// A config change must invalidate initialization without writing to the
// repository from a replica or a suspended cluster.
if err := r.reconcilePgBackRestStanza(context.Background(), cluster, true); err != nil {
t.Fatal(err)
}
if _, err := os.Stat(calls); !os.IsNotExist(err) {
t.Fatal("unexpected stanza-create while initialization is deferred")
}
if r.pgBackRestStanzaCreated.Load() != nil {
t.Fatal("initialization must remain pending")
}

// Promotion or resumption must initialize the repository even though
// the configuration file is already current.
cluster.Status.CurrentPrimary = "branch-1"
cluster.Annotations = nil
if err := r.reconcilePgBackRestStanza(context.Background(), cluster, false); err != nil {
t.Fatal(err)
}
content, err := os.ReadFile(calls) // #nosec G304 -- Test-owned path inside t.TempDir.
if err != nil {
t.Fatal(err)
}
if string(content) != "--config="+pgbackrest.ConfigFilePath+" --stanza=branch stanza-create --no-online\n" {
t.Fatalf("unexpected stanza-create invocation: %s", content)
}
if created := r.pgBackRestStanzaCreated.Load(); created == nil || *created != "branch" {
t.Fatal("successful initialization must be recorded")
}
})
}
}
Loading