Skip to content

Commit 5eaf862

Browse files
dsessler7dsessler7
committed
Allow pgbackrest in cloud backup jobs to log to an additional volume.
1 parent bf21f0a commit 5eaf862

File tree

6 files changed

+150
-33
lines changed

6 files changed

+150
-33
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

@@ -2075,28 +2082,7 @@ func (r *Reconciler) reconcilePGBackRestConfig(ctx context.Context,
20752082
repoHostName, configHash, serviceName, serviceNamespace string,
20762083
instanceNames []string) error {
20772084

2078-
// If the user has specified a PVC to use as a log volume for cloud backups via the
2079-
// PGBackRestCloudLogVolume annotation, check for the PVC. If we find it, set the cloud
2080-
// log path. If the user has specified a PVC, but we can't find it, create a warning event.
2081-
cloudLogPath := ""
2082-
if logVolumeName := postgresCluster.Annotations[naming.PGBackRestCloudLogVolume]; logVolumeName != "" {
2083-
logVolume := &corev1.PersistentVolumeClaim{
2084-
ObjectMeta: metav1.ObjectMeta{
2085-
Name: logVolumeName,
2086-
Namespace: postgresCluster.GetNamespace(),
2087-
},
2088-
}
2089-
err := errors.WithStack(r.Client.Get(ctx,
2090-
client.ObjectKeyFromObject(logVolume), logVolume))
2091-
if err != nil {
2092-
// PVC not retrieved, create warning event
2093-
r.Recorder.Event(postgresCluster, corev1.EventTypeWarning,
2094-
"PGBackRestCloudLogVolumeNotFound", err.Error())
2095-
} else {
2096-
// We successfully found the specified PVC, so we will set the log path
2097-
cloudLogPath = "/volumes/" + logVolumeName
2098-
}
2099-
}
2085+
cloudLogPath := r.reconcileCloudLogPath(ctx, postgresCluster)
21002086

21012087
backrestConfig, err := pgbackrest.CreatePGBackRestConfigMapIntent(ctx, postgresCluster, repoHostName,
21022088
configHash, serviceName, serviceNamespace, cloudLogPath, instanceNames)
@@ -3351,3 +3337,40 @@ func authorizeBackupRemovalAnnotationPresent(postgresCluster *v1beta1.PostgresCl
33513337
}
33523338
return false
33533339
}
3340+
3341+
// reconcileCloudLogPath is responsible for determining the appropriate log path
3342+
// for pgbackrest in cloud backup jobs.
3343+
func (r *Reconciler) reconcileCloudLogPath(ctx context.Context,
3344+
postgresCluster *v1beta1.PostgresCluster) string {
3345+
// If the user has specified a PVC to use as a log volume for cloud backups via the
3346+
// PGBackRestCloudLogVolume annotation, check for the PVC. If we find it, set the cloud
3347+
// log path. If the user has specified a PVC, but we can't find it, create a warning event.
3348+
// If the user has not set the PGBackRestCloudLogVolume annotation, but has set a log
3349+
// path via the spec, use that.
3350+
// TODO: Make sure this is what we want (i.e. annotation to take precedence over spec)
3351+
cloudLogPath := ""
3352+
if logVolumeName := postgresCluster.Annotations[naming.PGBackRestCloudLogVolume]; logVolumeName != "" {
3353+
logVolume := &corev1.PersistentVolumeClaim{
3354+
ObjectMeta: metav1.ObjectMeta{
3355+
Name: logVolumeName,
3356+
Namespace: postgresCluster.GetNamespace(),
3357+
},
3358+
}
3359+
err := errors.WithStack(r.Client.Get(ctx,
3360+
client.ObjectKeyFromObject(logVolume), logVolume))
3361+
if err != nil {
3362+
// PVC not retrieved, create warning event
3363+
r.Recorder.Event(postgresCluster, corev1.EventTypeWarning,
3364+
"PGBackRestCloudLogVolumeNotFound", err.Error())
3365+
} else {
3366+
// We successfully found the specified PVC, so we will set the log path
3367+
cloudLogPath = "/volumes/" + logVolumeName
3368+
}
3369+
// TODO: Can we safely assume that backups are enabled?
3370+
} else if postgresCluster.Spec.Backups.PGBackRest.Jobs != nil &&
3371+
postgresCluster.Spec.Backups.PGBackRest.Jobs.Log != nil &&
3372+
postgresCluster.Spec.Backups.PGBackRest.Jobs.Log.Path != "" {
3373+
cloudLogPath = postgresCluster.Spec.Backups.PGBackRest.Jobs.Log.Path
3374+
}
3375+
return cloudLogPath
3376+
}

‎internal/controller/postgrescluster/pgbackrest_test.go‎

Lines changed: 77 additions & 10 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
@@ -2963,6 +2968,12 @@ volumes:
29632968
assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
29642969
containers:
29652970
- command:
2971+
- sh
2972+
- -c
2973+
- --
2974+
- mkdir -p '/volumes/another-pvc' && { chmod 0775 '/volumes/another-pvc' || :; };
2975+
exec "$@"
2976+
- --
29662977
- /bin/pgbackrest
29672978
- backup
29682979
- --stanza=db
@@ -3029,7 +3040,11 @@ volumes:
30293040

30303041
cluster := cluster.DeepCopy()
30313042
cluster.Namespace = ns.Name
3043+
cluster.Annotations = map[string]string{}
30323044
cluster.Spec.Backups.PGBackRest.Jobs = &v1beta1.BackupJobs{
3045+
Log: &v1beta1.LoggingConfiguration{
3046+
Path: "/volumes/stuff/log",
3047+
},
30333048
Volumes: &v1beta1.PGBackRestVolumesSpec{
30343049
Additional: []v1beta1.AdditionalVolume{
30353050
{
@@ -3046,18 +3061,70 @@ volumes:
30463061
nil, nil,
30473062
)
30483063

3049-
for _, container := range spec.Template.Spec.Containers {
3050-
assert.Assert(t, cmp.MarshalContains(container.VolumeMounts,
3051-
`
3052-
- mountPath: /volumes/stuff
3053-
name: volumes-stuff`))
3054-
}
3055-
3056-
assert.Assert(t, cmp.MarshalContains(spec.Template.Spec.Volumes,
3057-
`
3064+
assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
3065+
containers:
3066+
- command:
3067+
- sh
3068+
- -c
3069+
- --
3070+
- mkdir -p '/volumes/stuff/log' && { chmod 0775 '/volumes/stuff/log' || :; }; exec
3071+
"$@"
3072+
- --
3073+
- /bin/pgbackrest
3074+
- backup
3075+
- --stanza=db
3076+
- --repo=
3077+
name: pgbackrest
3078+
resources: {}
3079+
securityContext:
3080+
allowPrivilegeEscalation: false
3081+
capabilities:
3082+
drop:
3083+
- ALL
3084+
privileged: false
3085+
readOnlyRootFilesystem: true
3086+
runAsNonRoot: true
3087+
seccompProfile:
3088+
type: RuntimeDefault
3089+
volumeMounts:
3090+
- mountPath: /etc/pgbackrest/conf.d
3091+
name: pgbackrest-config
3092+
readOnly: true
3093+
- mountPath: /tmp
3094+
name: tmp
3095+
- mountPath: /volumes/stuff
3096+
name: volumes-stuff
3097+
enableServiceLinks: false
3098+
restartPolicy: Never
3099+
securityContext:
3100+
fsGroup: 26
3101+
fsGroupChangePolicy: OnRootMismatch
3102+
volumes:
3103+
- name: pgbackrest-config
3104+
projected:
3105+
sources:
3106+
- configMap:
3107+
items:
3108+
- key: pgbackrest_cloud.conf
3109+
path: pgbackrest_cloud.conf
3110+
name: hippo-test-pgbackrest-config
3111+
- secret:
3112+
items:
3113+
- key: pgbackrest.ca-roots
3114+
path: ~postgres-operator/tls-ca.crt
3115+
- key: pgbackrest-client.crt
3116+
path: ~postgres-operator/client-tls.crt
3117+
- key: pgbackrest-client.key
3118+
mode: 384
3119+
path: ~postgres-operator/client-tls.key
3120+
name: hippo-test-pgbackrest
3121+
- emptyDir:
3122+
sizeLimit: 16Mi
3123+
name: tmp
30583124
- name: volumes-stuff
30593125
persistentVolumeClaim:
3060-
claimName: additional-pvc`))
3126+
claimName: additional-pvc
3127+
`))
30613128

30623129
// No events created
30633130
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)