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

Configure Patroni Logs to be stored in a file #4047

Merged
merged 1 commit into from
Dec 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -11630,6 +11630,35 @@ spec:
format: int32
minimum: 3
type: integer
logging:
description: Patroni log configuration settings.
properties:
level:
default: INFO
description: |-
The Patroni log level.
https://docs.python.org/3.6/library/logging.html#levels
enum:
- CRITICAL
- ERROR
- WARNING
- INFO
- DEBUG
- NOTSET
type: string
storageLimit:
anyOf:
- type: integer
- type: string
description: |-
Limits the total amount of space taken by Patroni Log files.
Minimum value is 25MB.
https://pkg.go.dev/k8s.io/apimachinery/pkg/api/resource#Quantity
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
required:
- storageLimit
type: object
port:
default: 8008
description: |-
Expand Down
29 changes: 28 additions & 1 deletion internal/controller/postgrescluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (r *Reconciler) reconcileClusterConfigMap(

if err == nil {
err = patroni.ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters,
clusterConfigMap)
clusterConfigMap, r.patroniLogSize(cluster))
}
if err == nil {
err = errors.WithStack(r.apply(ctx, clusterConfigMap))
Expand All @@ -53,6 +53,33 @@ func (r *Reconciler) reconcileClusterConfigMap(
return clusterConfigMap, err
}

// patroniLogSize attempts to parse the defined log file storage limit, if configured.
// If a value is set, this enables volume based log storage and triggers the
// relevant Patroni configuration. If the value given is less than 25M, the log
// file size storage limit defaults to 25M and an event is triggered.
func (r *Reconciler) patroniLogSize(cluster *v1beta1.PostgresCluster) int64 {

if cluster.Spec.Patroni != nil {
if cluster.Spec.Patroni.Logging != nil {
if cluster.Spec.Patroni.Logging.StorageLimit != nil {

sizeInBytes := cluster.Spec.Patroni.Logging.StorageLimit.Value()

if sizeInBytes < 25000000 {
// TODO(validation): Eventually we should be able to remove this in favor of CEL validation.
// - https://kubernetes.io/docs/reference/using-api/cel/
r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "PatroniLogStorageLimitTooSmall",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 I updated this from a normal event to a warning event. This is more in line with our existing patterns and what seems to me to be typical usage.

"Configured Patroni log storage limit is too small. File size will default to 25M.")

sizeInBytes = 25000000
}
return sizeInBytes
}
}
}
return 0
}

// +kubebuilder:rbac:groups="",resources="services",verbs={create,patch}

// reconcileClusterPodService writes the Service that can provide stable DNS
Expand Down
74 changes: 74 additions & 0 deletions internal/controller/postgrescluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/tools/record"
Expand All @@ -23,6 +24,7 @@ import (
"github.com/crunchydata/postgres-operator/internal/initialize"
"github.com/crunchydata/postgres-operator/internal/naming"
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
"github.com/crunchydata/postgres-operator/internal/testing/events"
"github.com/crunchydata/postgres-operator/internal/testing/require"
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
)
Expand Down Expand Up @@ -783,3 +785,75 @@ postgres-operator.crunchydata.com/role: replica
`))
})
}

func TestPatroniLogSize(t *testing.T) {

oneHundredMeg, err := resource.ParseQuantity("100M")
assert.NilError(t, err)

tooSmall, err := resource.ParseQuantity("1k")
assert.NilError(t, err)

cluster := v1beta1.PostgresCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "sometest",
Namespace: "test-namespace",
},
Spec: v1beta1.PostgresClusterSpec{}}

t.Run("Default", func(t *testing.T) {
recorder := events.NewRecorder(t, runtime.Scheme)
reconciler := &Reconciler{Recorder: recorder}

size := reconciler.patroniLogSize(&cluster)

assert.Equal(t, size, int64(0))
assert.Equal(t, len(recorder.Events), 0)
})

t.Run("NoSize", func(t *testing.T) {
recorder := events.NewRecorder(t, runtime.Scheme)
reconciler := &Reconciler{Recorder: recorder}

cluster.Spec.Patroni = &v1beta1.PatroniSpec{
Logging: &v1beta1.PatroniLogConfig{}}

size := reconciler.patroniLogSize(&cluster)

assert.Equal(t, size, int64(0))
assert.Equal(t, len(recorder.Events), 0)
})

t.Run("ValidSize", func(t *testing.T) {
recorder := events.NewRecorder(t, runtime.Scheme)
reconciler := &Reconciler{Recorder: recorder}

cluster.Spec.Patroni = &v1beta1.PatroniSpec{
Logging: &v1beta1.PatroniLogConfig{
StorageLimit: &oneHundredMeg,
}}

size := reconciler.patroniLogSize(&cluster)

assert.Equal(t, size, int64(100000000))
assert.Equal(t, len(recorder.Events), 0)
})

t.Run("BadSize", func(t *testing.T) {
recorder := events.NewRecorder(t, runtime.Scheme)
reconciler := &Reconciler{Recorder: recorder}

cluster.Spec.Patroni = &v1beta1.PatroniSpec{
Logging: &v1beta1.PatroniLogConfig{
StorageLimit: &tooSmall,
}}

size := reconciler.patroniLogSize(&cluster)

assert.Equal(t, size, int64(25000000))
assert.Equal(t, len(recorder.Events), 1)
assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name)
assert.Equal(t, recorder.Events[0].Reason, "PatroniLogStorageLimitTooSmall")
assert.Equal(t, recorder.Events[0].Note, "Configured Patroni log storage limit is too small. File size will default to 25M.")
})
}
4 changes: 4 additions & 0 deletions internal/naming/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ const (
)

const (
// PatroniPGDataLogPath is the Patroni default log path configuration used by the
// PostgreSQL instance.
PatroniPGDataLogPath = "/pgdata/patroni/log"

// PGBackRestRepoContainerName is the name assigned to the container used to run pgBackRest
PGBackRestRepoContainerName = "pgbackrest"

Expand Down
25 changes: 24 additions & 1 deletion internal/patroni/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func quoteShellWord(s string) string {
// clusterYAML returns Patroni settings that apply to the entire cluster.
func clusterYAML(
cluster *v1beta1.PostgresCluster,
pgHBAs postgres.HBAs, pgParameters postgres.Parameters,
pgHBAs postgres.HBAs, pgParameters postgres.Parameters, patroniLogStorageLimit int64,
) (string, error) {
root := map[string]any{
// The cluster identifier. This value cannot change during the cluster's
Expand Down Expand Up @@ -152,6 +152,29 @@ func clusterYAML(
},
}

// if a Patroni log file size is configured, configure volume file storage
if patroniLogStorageLimit != 0 {

// Configure the Patroni log settings
// - https://patroni.readthedocs.io/en/latest/yaml_configuration.html#log
root["log"] = map[string]any{

"dir": naming.PatroniPGDataLogPath,
"type": "json",

// defaults to "INFO"
"level": cluster.Spec.Patroni.Logging.Level,

// There will only be two log files. Cannot set to 1 or the logs won't rotate.
// - https://github.com/python/cpython/blob/3.11/Lib/logging/handlers.py#L134
"file_num": 1,

// Since there are two log files, ensure the total space used is under
// the configured limit.
"file_size": patroniLogStorageLimit / 2,
}
}

if !ClusterBootstrapped(cluster) {
// Patroni has not yet bootstrapped. Populate the "bootstrap.dcs" field to
// facilitate it. When Patroni is already bootstrapped, this field is ignored.
Expand Down
78 changes: 76 additions & 2 deletions internal/patroni/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"gotest.tools/v3/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/yaml"

Expand All @@ -32,7 +33,7 @@ func TestClusterYAML(t *testing.T) {
cluster.Namespace = "some-namespace"
cluster.Name = "cluster-name"

data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 0)
assert.NilError(t, err)
assert.Equal(t, data, strings.TrimSpace(`
# Generated by postgres-operator. DO NOT EDIT.
Expand Down Expand Up @@ -90,7 +91,7 @@ watchdog:
cluster.Name = "cluster-name"
cluster.Spec.PostgresVersion = 14

data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 0)
assert.NilError(t, err)
assert.Equal(t, data, strings.TrimSpace(`
# Generated by postgres-operator. DO NOT EDIT.
Expand Down Expand Up @@ -136,6 +137,79 @@ restapi:
keyfile: null
verify_client: optional
scope: cluster-name-ha
watchdog:
mode: "off"
`)+"\n")
})

t.Run("PatroniLogSizeConfigured", func(t *testing.T) {
cluster := new(v1beta1.PostgresCluster)
cluster.Default()
cluster.Namespace = "some-namespace"
cluster.Name = "cluster-name"
cluster.Spec.PostgresVersion = 14

fileSize, err := resource.ParseQuantity("1k")
assert.NilError(t, err)

logLevel := "DEBUG"
cluster.Spec.Patroni.Logging = &v1beta1.PatroniLogConfig{
StorageLimit: &fileSize,
Level: &logLevel,
}

data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 1000)
assert.NilError(t, err)
assert.Equal(t, data, strings.TrimSpace(`
# Generated by postgres-operator. DO NOT EDIT.
# Your changes will not be saved.
bootstrap:
dcs:
loop_wait: 10
postgresql:
parameters: {}
pg_hba: []
use_pg_rewind: true
use_slots: false
ttl: 30
ctl:
cacert: /etc/patroni/~postgres-operator/patroni.ca-roots
certfile: /etc/patroni/~postgres-operator/patroni.crt+key
insecure: false
keyfile: null
kubernetes:
labels:
postgres-operator.crunchydata.com/cluster: cluster-name
namespace: some-namespace
role_label: postgres-operator.crunchydata.com/role
scope_label: postgres-operator.crunchydata.com/patroni
use_endpoints: true
log:
dir: /pgdata/patroni/log
file_num: 1
file_size: 500
level: DEBUG
type: json
postgresql:
authentication:
replication:
sslcert: /tmp/replication/tls.crt
sslkey: /tmp/replication/tls.key
sslmode: verify-ca
sslrootcert: /tmp/replication/ca.crt
username: _crunchyrepl
rewind:
sslcert: /tmp/replication/tls.crt
sslkey: /tmp/replication/tls.key
sslmode: verify-ca
sslrootcert: /tmp/replication/ca.crt
username: _crunchyrepl
restapi:
cafile: /etc/patroni/~postgres-operator/patroni.ca-roots
certfile: /etc/patroni/~postgres-operator/patroni.crt+key
keyfile: null
verify_client: optional
scope: cluster-name-ha
watchdog:
mode: "off"
`)+"\n")
Expand Down
3 changes: 2 additions & 1 deletion internal/patroni/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ func ClusterConfigMap(ctx context.Context,
inHBAs postgres.HBAs,
inParameters postgres.Parameters,
outClusterConfigMap *corev1.ConfigMap,
patroniLogStorageLimit int64,
) error {
var err error

initialize.Map(&outClusterConfigMap.Data)

outClusterConfigMap.Data[configMapFileKey], err = clusterYAML(inCluster, inHBAs,
inParameters)
inParameters, patroniLogStorageLimit)

return err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/patroni/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ func TestClusterConfigMap(t *testing.T) {

cluster.Default()
config := new(corev1.ConfigMap)
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config))
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0))

// The output of clusterYAML should go into config.
data, _ := clusterYAML(cluster, pgHBAs, pgParameters)
data, _ := clusterYAML(cluster, pgHBAs, pgParameters, 0)
assert.DeepEqual(t, config.Data["patroni.yaml"], data)

// No change when called again.
before := config.DeepCopy()
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config))
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0))
assert.DeepEqual(t, config, before)
}

Expand Down
9 changes: 7 additions & 2 deletions internal/postgres/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,9 @@ chmod +x /tmp/pg_rewind_tde.sh
`
}

args := []string{version, walDir, naming.PGBackRestPGDataLogPath}
args := []string{version, walDir, naming.PGBackRestPGDataLogPath, naming.PatroniPGDataLogPath}
script := strings.Join([]string{
`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3"`,
`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3" patroniLog_directory="$4"`,

// Function to print the permissions of a file or directory and its parents.
bashPermissions,
Expand Down Expand Up @@ -369,6 +369,11 @@ chmod +x /tmp/pg_rewind_tde.sh
`install --directory --mode=0775 "${pgbrLog_directory}" ||`,
`halt "$(permissions "${pgbrLog_directory}" ||:)"`,

// Create the Patroni log directory.
`results 'Patroni log directory' "${patroniLog_directory}"`,
`install --directory --mode=0775 "${patroniLog_directory}" ||`,
`halt "$(permissions "${patroniLog_directory}" ||:)"`,

// Copy replication client certificate files
// from the /pgconf/tls/replication directory to the /tmp/replication directory in order
// to set proper file permissions. This is required because the group permission settings
Expand Down
Loading