Skip to content

Commit 99ee6b5

Browse files
dsessler7dsessler7
committed
Allow pgbackrest in cloud backup jobs to log to an additional volume.
1 parent d80f1ec commit 99ee6b5

File tree

6 files changed

+156
-34
lines changed

6 files changed

+156
-34
lines changed

‎config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,13 @@ spec:
14301430
x-kubernetes-list-type: atomic
14311431
type: object
14321432
type: object
1433+
log:
1434+
description: Logging configuration for pgbackrest processes
1435+
running in Backup Job Pods.
1436+
properties:
1437+
path:
1438+
type: string
1439+
type: object
14331440
priorityClassName:
14341441
description: |-
14351442
Priority class name for the pgBackRest backup Job pods.
@@ -4606,6 +4613,9 @@ spec:
46064613
- message: log path is restricted to an existing additional volume
46074614
rule: '!has(self.repoHost) || !has(self.repoHost.log) || !has(self.repoHost.log.path)
46084615
|| self.repoHost.volumes.additional.exists(x, self.repoHost.log.path.startsWith("/volumes/"+x.name))'
4616+
- message: log path is restricted to an existing additional volume
4617+
rule: '!has(self.jobs) || !has(self.jobs.log) || !has(self.jobs.log.path)
4618+
|| self.jobs.volumes.additional.exists(x, self.jobs.log.path.startsWith("/volumes/"+x.name))'
46094619
snapshots:
46104620
description: VolumeSnapshot configuration
46114621
properties:
@@ -20381,6 +20391,13 @@ spec:
2038120391
x-kubernetes-list-type: atomic
2038220392
type: object
2038320393
type: object
20394+
log:
20395+
description: Logging configuration for pgbackrest processes
20396+
running in Backup Job Pods.
20397+
properties:
20398+
path:
20399+
type: string
20400+
type: object
2038420401
priorityClassName:
2038520402
description: |-
2038620403
Priority class name for the pgBackRest backup Job pods.

‎internal/controller/postgrescluster/pgbackrest.go‎

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"github.com/crunchydata/postgres-operator/internal/pgbackrest"
3939
"github.com/crunchydata/postgres-operator/internal/pki"
4040
"github.com/crunchydata/postgres-operator/internal/postgres"
41+
"github.com/crunchydata/postgres-operator/internal/shell"
4142
"github.com/crunchydata/postgres-operator/internal/util"
4243
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
4344
)
@@ -821,7 +822,13 @@ func (r *Reconciler) generateBackupJobSpecIntent(ctx context.Context, postgresCl
821822
{Name: "SELECTOR", Value: naming.PGBackRestDedicatedSelector(postgresCluster.GetName()).String()},
822823
}
823824
} else {
824-
container.Command = []string{"/bin/pgbackrest", "backup"}
825+
mkdirCommand := ""
826+
cloudLogPath := r.reconcileCloudLogPath(ctx, postgresCluster)
827+
if cloudLogPath != "" {
828+
mkdirCommand += shell.MakeDirectories(cloudLogPath, cloudLogPath) + "; "
829+
}
830+
831+
container.Command = []string{"sh", "-c", "--", mkdirCommand + `exec "$@"`, "--", "/bin/pgbackrest", "backup"}
825832
container.Command = append(container.Command, cmdOpts...)
826833
}
827834

@@ -2091,28 +2098,7 @@ func (r *Reconciler) reconcilePGBackRestConfig(ctx context.Context,
20912098
repoHostName, configHash, serviceName, serviceNamespace string,
20922099
instanceNames []string) error {
20932100

2094-
// If the user has specified a PVC to use as a log volume for cloud backups via the
2095-
// PGBackRestCloudLogVolume annotation, check for the PVC. If we find it, set the cloud
2096-
// log path. If the user has specified a PVC, but we can't find it, create a warning event.
2097-
cloudLogPath := ""
2098-
if logVolumeName := postgresCluster.Annotations[naming.PGBackRestCloudLogVolume]; logVolumeName != "" {
2099-
logVolume := &corev1.PersistentVolumeClaim{
2100-
ObjectMeta: metav1.ObjectMeta{
2101-
Name: logVolumeName,
2102-
Namespace: postgresCluster.GetNamespace(),
2103-
},
2104-
}
2105-
err := errors.WithStack(r.Client.Get(ctx,
2106-
client.ObjectKeyFromObject(logVolume), logVolume))
2107-
if err != nil {
2108-
// PVC not retrieved, create warning event
2109-
r.Recorder.Event(postgresCluster, corev1.EventTypeWarning,
2110-
"PGBackRestCloudLogVolumeNotFound", err.Error())
2111-
} else {
2112-
// We successfully found the specified PVC, so we will set the log path
2113-
cloudLogPath = "/volumes/" + logVolumeName
2114-
}
2115-
}
2101+
cloudLogPath := r.reconcileCloudLogPath(ctx, postgresCluster)
21162102

21172103
backrestConfig, err := pgbackrest.CreatePGBackRestConfigMapIntent(ctx, postgresCluster, repoHostName,
21182104
configHash, serviceName, serviceNamespace, cloudLogPath, instanceNames)
@@ -3367,3 +3353,40 @@ func authorizeBackupRemovalAnnotationPresent(postgresCluster *v1beta1.PostgresCl
33673353
}
33683354
return false
33693355
}
3356+
3357+
// reconcileCloudLogPath is responsible for determining the appropriate log path
3358+
// for pgbackrest in cloud backup jobs.
3359+
func (r *Reconciler) reconcileCloudLogPath(ctx context.Context,
3360+
postgresCluster *v1beta1.PostgresCluster) string {
3361+
// If the user has specified a PVC to use as a log volume for cloud backups via the
3362+
// PGBackRestCloudLogVolume annotation, check for the PVC. If we find it, set the cloud
3363+
// log path. If the user has specified a PVC, but we can't find it, create a warning event.
3364+
// If the user has not set the PGBackRestCloudLogVolume annotation, but has set a log
3365+
// path via the spec, use that.
3366+
// TODO: Make sure this is what we want (i.e. annotation to take precedence over spec)
3367+
cloudLogPath := ""
3368+
if logVolumeName := postgresCluster.Annotations[naming.PGBackRestCloudLogVolume]; logVolumeName != "" {
3369+
logVolume := &corev1.PersistentVolumeClaim{
3370+
ObjectMeta: metav1.ObjectMeta{
3371+
Name: logVolumeName,
3372+
Namespace: postgresCluster.GetNamespace(),
3373+
},
3374+
}
3375+
err := errors.WithStack(r.Client.Get(ctx,
3376+
client.ObjectKeyFromObject(logVolume), logVolume))
3377+
if err != nil {
3378+
// PVC not retrieved, create warning event
3379+
r.Recorder.Event(postgresCluster, corev1.EventTypeWarning,
3380+
"PGBackRestCloudLogVolumeNotFound", err.Error())
3381+
} else {
3382+
// We successfully found the specified PVC, so we will set the log path
3383+
cloudLogPath = "/volumes/" + logVolumeName
3384+
}
3385+
// TODO: Can we safely assume that backups are enabled?
3386+
} else if postgresCluster.Spec.Backups.PGBackRest.Jobs != nil &&
3387+
postgresCluster.Spec.Backups.PGBackRest.Jobs.Log != nil &&
3388+
postgresCluster.Spec.Backups.PGBackRest.Jobs.Log.Path != "" {
3389+
cloudLogPath = postgresCluster.Spec.Backups.PGBackRest.Jobs.Log.Path
3390+
}
3391+
return cloudLogPath
3392+
}

‎internal/controller/postgrescluster/pgbackrest_test.go‎

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2661,6 +2661,11 @@ func TestGenerateBackupJobIntent(t *testing.T) {
26612661
assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
26622662
containers:
26632663
- command:
2664+
- sh
2665+
- -c
2666+
- --
2667+
- exec "$@"
2668+
- --
26642669
- /bin/pgbackrest
26652670
- backup
26662671
- --stanza=db
@@ -2962,6 +2967,11 @@ volumes:
29622967
assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
29632968
containers:
29642969
- command:
2970+
- sh
2971+
- -c
2972+
- --
2973+
- exec "$@"
2974+
- --
29652975
- /bin/pgbackrest
29662976
- backup
29672977
- --stanza=db
@@ -3013,7 +3023,7 @@ volumes:
30133023
name: tmp
30143024
`))
30153025

3016-
assert.Equal(t, len(recorder.Events), 1)
3026+
assert.Equal(t, len(recorder.Events), 2)
30173027
assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name)
30183028
assert.Equal(t, recorder.Events[0].Reason, "PGBackRestCloudLogVolumeNotFound")
30193029
assert.Equal(t, recorder.Events[0].Note, "persistentvolumeclaims \"some-pvc\" not found")
@@ -3045,6 +3055,12 @@ volumes:
30453055
assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
30463056
containers:
30473057
- command:
3058+
- sh
3059+
- -c
3060+
- --
3061+
- mkdir -p '/volumes/another-pvc' && { chmod 0775 '/volumes/another-pvc' || :; };
3062+
exec "$@"
3063+
- --
30483064
- /bin/pgbackrest
30493065
- backup
30503066
- --stanza=db
@@ -3111,7 +3127,11 @@ volumes:
31113127

31123128
cluster := cluster.DeepCopy()
31133129
cluster.Namespace = ns.Name
3130+
cluster.Annotations = map[string]string{}
31143131
cluster.Spec.Backups.PGBackRest.Jobs = &v1beta1.BackupJobs{
3132+
Log: &v1beta1.LoggingConfiguration{
3133+
Path: "/volumes/stuff/log",
3134+
},
31153135
Volumes: &v1beta1.PGBackRestVolumesSpec{
31163136
Additional: []v1beta1.AdditionalVolume{
31173137
{
@@ -3128,18 +3148,70 @@ volumes:
31283148
nil, nil,
31293149
)
31303150

3131-
for _, container := range spec.Template.Spec.Containers {
3132-
assert.Assert(t, cmp.MarshalContains(container.VolumeMounts,
3133-
`
3134-
- mountPath: /volumes/stuff
3135-
name: volumes-stuff`))
3136-
}
3137-
3138-
assert.Assert(t, cmp.MarshalContains(spec.Template.Spec.Volumes,
3139-
`
3151+
assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
3152+
containers:
3153+
- command:
3154+
- sh
3155+
- -c
3156+
- --
3157+
- mkdir -p '/volumes/stuff/log' && { chmod 0775 '/volumes/stuff/log' || :; }; exec
3158+
"$@"
3159+
- --
3160+
- /bin/pgbackrest
3161+
- backup
3162+
- --stanza=db
3163+
- --repo=
3164+
name: pgbackrest
3165+
resources: {}
3166+
securityContext:
3167+
allowPrivilegeEscalation: false
3168+
capabilities:
3169+
drop:
3170+
- ALL
3171+
privileged: false
3172+
readOnlyRootFilesystem: true
3173+
runAsNonRoot: true
3174+
seccompProfile:
3175+
type: RuntimeDefault
3176+
volumeMounts:
3177+
- mountPath: /etc/pgbackrest/conf.d
3178+
name: pgbackrest-config
3179+
readOnly: true
3180+
- mountPath: /tmp
3181+
name: tmp
3182+
- mountPath: /volumes/stuff
3183+
name: volumes-stuff
3184+
enableServiceLinks: false
3185+
restartPolicy: Never
3186+
securityContext:
3187+
fsGroup: 26
3188+
fsGroupChangePolicy: OnRootMismatch
3189+
volumes:
3190+
- name: pgbackrest-config
3191+
projected:
3192+
sources:
3193+
- configMap:
3194+
items:
3195+
- key: pgbackrest_cloud.conf
3196+
path: pgbackrest_cloud.conf
3197+
name: hippo-test-pgbackrest-config
3198+
- secret:
3199+
items:
3200+
- key: pgbackrest.ca-roots
3201+
path: ~postgres-operator/tls-ca.crt
3202+
- key: pgbackrest-client.crt
3203+
path: ~postgres-operator/client-tls.crt
3204+
- key: pgbackrest-client.key
3205+
mode: 384
3206+
path: ~postgres-operator/client-tls.key
3207+
name: hippo-test-pgbackrest
3208+
- emptyDir:
3209+
sizeLimit: 16Mi
3210+
name: tmp
31403211
- name: volumes-stuff
31413212
persistentVolumeClaim:
3142-
claimName: additional-pvc`))
3213+
claimName: additional-pvc
3214+
`))
31433215

31443216
// No events created
31453217
assert.Equal(t, len(recorder.Events), 0)

‎pkg/apis/postgres-operator.crunchydata.com/v1/pgbackrest_types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
// PGBackRestArchive defines a pgBackRest archive configuration
1212
// +kubebuilder:validation:XValidation:rule=`!has(self.repoHost) || !has(self.repoHost.log) || !has(self.repoHost.log.path) || self.repoHost.volumes.additional.exists(x, self.repoHost.log.path.startsWith("/volumes/"+x.name))`,message=`log path is restricted to an existing additional volume`
13+
// +kubebuilder:validation:XValidation:rule=`!has(self.jobs) || !has(self.jobs.log) || !has(self.jobs.log.path) || self.jobs.volumes.additional.exists(x, self.jobs.log.path.startsWith("/volumes/"+x.name))`,message=`log path is restricted to an existing additional volume`
1314
type PGBackRestArchive struct {
1415
v1beta1.PGBackRestArchive `json:",inline"`
1516
}

‎pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbackrest_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ type BackupJobs struct {
159159
// +optional
160160
Resources corev1.ResourceRequirements `json:"resources,omitzero"`
161161

162+
// Logging configuration for pgbackrest processes running in Backup Job Pods.
163+
// +optional
164+
Log *LoggingConfiguration `json:"log,omitempty"`
165+
162166
// Priority class name for the pgBackRest backup Job pods.
163167
// More info: https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/
164168
// +optional

‎pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)