diff --git a/README.md b/README.md index 1a7898a..39bd288 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,7 @@ for a fuller example including the S3 backend. | `policy.downsample[]` | Merge-time age-tiered rollup: `{after, interval, agg}`. Samples past `after` collapse to one per `interval` bucket. **Lossy and irreversible.** | | `policy.precision[]` | Age-tiered lossy float precision: `{after, bits}`. Parts past `after` keep only `bits` mantissa bits. **Lossy and irreversible.** | | `policy.recompress` | `{after, level}`. Rewrites fully-cold parts with a higher-ratio Zstandard profile. Decode-transparent and lossless. | +| `policy.ec` | `{data, parity, after}`. Erasure-codes fully-cold parts across `data+parity` nodes instead of RF full copies — `{4,2}` is 1.5x the logical bytes for two tolerated node losses, against 3x for RF=3. See [Erasure coding](#erasure-coding). | | `service.type` / `annotations` | Client Service exposing the query/ingest APIs. | | `resources`, `nodeSelector`, `affinity`, `tolerations`, `topologySpreadConstraints`, `podSecurityContext`, `securityContext`, `podAnnotations`, `podLabels`, `serviceAccountName` | Standard pod scheduling/security knobs. | | `extraConfig` | Arbitrary raw oteldb config **deep-merged** over the generated config — for fields the CRD does not model (auth, prometheus tuning, …). Nested objects merge key by key (`storage.policy` does not wipe `storage.backend`); operator-owned paths are [reserved](#reserved-extraconfig-paths). | @@ -136,6 +137,40 @@ for a fuller example including the S3 backend. > so on those the two newer blocks are accepted by the API server and have no effect. Pin a newer > `spec.image` before relying on them. +### Erasure coding + +`spec.policy.ec` is the one storage policy that is only reachable in this operator's topology. The +engine gates erasure coding on a **shared-nothing cluster** — cluster mode *and* a private per-node +backend — which is precisely what an `OtelDBCluster` deploys: etcd is required, and the default +`file` backend is one PVC per pod. A single-node oteldb cannot use it, and neither can a cluster on +a shared bucket, where the store owns durability and every part stays full-copy. + +```yaml +spec: + replicas: 6 + policy: + ec: + data: 4 + parity: 2 + after: 72h # omit to erasure-code every part +``` + +Two consequences are worth knowing before turning it on. + +**`data+parity` replaces the replication factor.** Under an EC policy the engine's replication-factor +lookup returns `data+parity` and ignores the configured RF — for the unflushed head as well as the +converted parts. Setting both would mean one of them is silently dropped, so the operator +**rejects** `spec.policy.ec` together with `spec.cluster.replicationFactor` rather than pick a +winner. It also rejects a scheme with more shards than `spec.replicas`, since the ring would +otherwise clamp the placement to the members it has. + +**Rack safety needs zones.** A placement survives losing a whole failure domain only with at least +`ceil((data+parity)/parity)` distinct zones — 3 for `{4,2}`. Below that the engine converts anyway +and warns; spread the nodes with `spec.topologySpreadConstraints` to earn it. + +Erasure coding also needs an oteldb carrying [oteldb/oteldb#1279](https://github.com/oteldb/oteldb/pull/1279); +older builds ignore the key silently. + ### Reserved `extraConfig` paths `extraConfig` is merged recursively, so it can add keys the CRD does not model: @@ -159,7 +194,7 @@ spec field to use instead. | `storage.s3` | `spec.storage.s3` | | `storage.cluster` (whole subtree) | `spec.cluster`, `spec.etcd.endpoints` | | `storage.flush_interval`, `storage.read_cache_bytes`, `storage.decode_cache_bytes`, `storage.decode_memory_bytes`, `storage.aggregate_stats` | `spec.engine` | -| `storage.policy.retention`, `storage.policy.limits`, `storage.policy.downsample`, `storage.policy.precision`, `storage.policy.recompress` | `spec.policy` | +| `storage.policy.retention`, `storage.policy.limits`, `storage.policy.downsample`, `storage.policy.precision`, `storage.policy.recompress`, `storage.policy.ec` | `spec.policy` | `storage.policy` is now modelled in full, so the whole block is reserved. diff --git a/api/v1alpha1/oteldbcluster_types.go b/api/v1alpha1/oteldbcluster_types.go index f58a50d..835cfa5 100644 --- a/api/v1alpha1/oteldbcluster_types.go +++ b/api/v1alpha1/oteldbcluster_types.go @@ -103,8 +103,9 @@ type OtelDBClusterSpec struct { Engine EngineSpec `json:"engine,omitempty"` // Policy is the per-tenant storage policy: retention, admission-control limits, and the - // merge-time downsample/precision/recompress tiers. It maps onto oteldb's storage.policy - // block. Empty leaves the engine at its defaults (retain forever, no limits, lossless, raw). + // merge-time downsample/precision/recompress/erasure-coding tiers. It maps onto oteldb's + // storage.policy block. Empty leaves the engine at its defaults (retain forever, no limits, + // lossless, raw, full-copy). // +optional Policy PolicySpec `json:"policy,omitempty"` @@ -165,8 +166,8 @@ type OtelDBClusterSpec struct { // instead of being merged: metrics_backend, traces_backend, logs_backend, profiles_backend, // storage.backend, storage.dir, storage.wal_dir, storage.s3, storage.cluster (and everything // below it), storage.flush_interval, storage.read_cache_bytes, storage.decode_cache_bytes, - // storage.decode_memory_bytes, storage.aggregate_stats and storage.policy (modelled in full, so - // the whole block is reserved). Configure those through spec.storage, spec.cluster, spec.etcd, + // storage.decode_memory_bytes, storage.aggregate_stats and storage.policy (modelled in full, + // erasure coding included, so the whole block is reserved). Configure those through spec.storage, spec.cluster, spec.etcd, // spec.signals, spec.engine and spec.policy. // +optional // +kubebuilder:pruning:PreserveUnknownFields @@ -573,6 +574,17 @@ type PolicySpec struct { // merge CPU for storage. It is decode-transparent and lossless. Nil disables it. // +optional Recompress *RecompressSpec `json:"recompress,omitempty"` + + // EC erasure-codes fully-cold parts across Data+Parity nodes instead of holding RF full copies. + // Nil keeps full-copy replication. + // + // It applies only to a shared-nothing cluster — cluster mode with a private per-node backend — + // which is exactly what this operator deploys by default, and is the only deployment shape in + // which erasure coding is reachable at all. On a shared object store the store owns durability + // and the engine leaves every part full-copy, so the operator rejects the block there instead + // of rendering a policy that costs and saves nothing. + // +optional + EC *ECSpec `json:"ec,omitempty"` } // DownsampleTierSpec is one age band of the downsampling policy. Tiers are order-independent: a @@ -626,6 +638,42 @@ type RecompressSpec struct { Level *int32 `json:"level,omitempty"` } +// ECSpec configures the cold-data erasure-coding tier: a flushed part older than After is +// re-encoded at merge into Data + Parity Reed-Solomon shards, one per cluster node, in place of the +// RF full copies. It stores (Data+Parity)/Data of the logical bytes and survives Parity node +// losses — {4,2} is 1.5x for two tolerated losses, against 3x for RF=3 — paying a +// reconstruct-on-read cost only for the parts that have converted. +// +// Data+Parity becomes the tenant's owner count, so it must not exceed Replicas, and it replaces +// spec.cluster.replicationFactor outright rather than combining with it (see ECSpec.Parity). +type ECSpec struct { + // Data is the number of data shards (k): any Data shards reconstruct the object. + // +kubebuilder:validation:Minimum=1 + // +required + Data int32 `json:"data"` + + // Parity is the number of parity shards (m): the scheme tolerates Parity node losses. + // + // Data+Parity is also the tenant's owner count under erasure coding, and it applies to the + // unflushed head too, not only to the converted parts: the storage engine's replication-factor + // lookup returns Data+Parity and ignores the configured replication factor entirely. An EC + // policy therefore silently overrides spec.cluster.replicationFactor, so the operator rejects + // setting both instead of honouring one and dropping the other. + // + // Shard placement is rack-safe — one zone failure costs at most Parity shards — only with at + // least ceil((Data+Parity)/Parity) distinct zones; below that the engine still converts, and + // warns. Spread the nodes with spec.topologySpreadConstraints to earn that. + // +kubebuilder:validation:Minimum=1 + // +required + Parity int32 `json:"parity"` + + // After is the age past which a fully-cold part is erasure-coded at merge, mirroring + // RecompressSpec.After and riding the same background merge. Empty erasure-codes every part, + // accepting the reconstruct-on-read cost everywhere for the cheapest storage. + // +optional + After *metav1.Duration `json:"after,omitempty"` +} + // RetentionSpec bounds how long data is kept. Enforcement happens at merge time and drops whole // partitions — never individual rows — so data can outlive the window until the partition holding // it has fully expired. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index ff08843..5aca5d6 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -73,6 +73,26 @@ func (in *DownsampleTierSpec) DeepCopy() *DownsampleTierSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ECSpec) DeepCopyInto(out *ECSpec) { + *out = *in + if in.After != nil { + in, out := &in.After, &out.After + *out = new(metav1.Duration) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ECSpec. +func (in *ECSpec) DeepCopy() *ECSpec { + if in == nil { + return nil + } + out := new(ECSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EngineSpec) DeepCopyInto(out *EngineSpec) { *out = *in @@ -454,6 +474,11 @@ func (in *PolicySpec) DeepCopyInto(out *PolicySpec) { *out = new(RecompressSpec) (*in).DeepCopyInto(*out) } + if in.EC != nil { + in, out := &in.EC, &out.EC + *out = new(ECSpec) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PolicySpec. diff --git a/config/crd/bases/db.oteldb.io_oteldbclusters.yaml b/config/crd/bases/db.oteldb.io_oteldbclusters.yaml index 62685aa..e01f454 100644 --- a/config/crd/bases/db.oteldb.io_oteldbclusters.yaml +++ b/config/crd/bases/db.oteldb.io_oteldbclusters.yaml @@ -1089,8 +1089,8 @@ spec: instead of being merged: metrics_backend, traces_backend, logs_backend, profiles_backend, storage.backend, storage.dir, storage.wal_dir, storage.s3, storage.cluster (and everything below it), storage.flush_interval, storage.read_cache_bytes, storage.decode_cache_bytes, - storage.decode_memory_bytes, storage.aggregate_stats and storage.policy (modelled in full, so - the whole block is reserved). Configure those through spec.storage, spec.cluster, spec.etcd, + storage.decode_memory_bytes, storage.aggregate_stats and storage.policy (modelled in full, + erasure coding included, so the whole block is reserved). Configure those through spec.storage, spec.cluster, spec.etcd, spec.signals, spec.engine and spec.policy. type: object x-kubernetes-preserve-unknown-fields: true @@ -3071,8 +3071,9 @@ spec: policy: description: |- Policy is the per-tenant storage policy: retention, admission-control limits, and the - merge-time downsample/precision/recompress tiers. It maps onto oteldb's storage.policy - block. Empty leaves the engine at its defaults (retain forever, no limits, lossless, raw). + merge-time downsample/precision/recompress/erasure-coding tiers. It maps onto oteldb's + storage.policy block. Empty leaves the engine at its defaults (retain forever, no limits, + lossless, raw, full-copy). properties: downsample: description: |- @@ -3114,6 +3115,49 @@ spec: type: object type: array x-kubernetes-list-type: atomic + ec: + description: |- + EC erasure-codes fully-cold parts across Data+Parity nodes instead of holding RF full copies. + Nil keeps full-copy replication. + + It applies only to a shared-nothing cluster — cluster mode with a private per-node backend — + which is exactly what this operator deploys by default, and is the only deployment shape in + which erasure coding is reachable at all. On a shared object store the store owns durability + and the engine leaves every part full-copy, so the operator rejects the block there instead + of rendering a policy that costs and saves nothing. + properties: + after: + description: |- + After is the age past which a fully-cold part is erasure-coded at merge, mirroring + RecompressSpec.After and riding the same background merge. Empty erasure-codes every part, + accepting the reconstruct-on-read cost everywhere for the cheapest storage. + type: string + data: + description: 'Data is the number of data shards (k): any Data + shards reconstruct the object.' + format: int32 + minimum: 1 + type: integer + parity: + description: |- + Parity is the number of parity shards (m): the scheme tolerates Parity node losses. + + Data+Parity is also the tenant's owner count under erasure coding, and it applies to the + unflushed head too, not only to the converted parts: the storage engine's replication-factor + lookup returns Data+Parity and ignores the configured replication factor entirely. An EC + policy therefore silently overrides spec.cluster.replicationFactor, so the operator rejects + setting both instead of honouring one and dropping the other. + + Shard placement is rack-safe — one zone failure costs at most Parity shards — only with at + least ceil((Data+Parity)/Parity) distinct zones; below that the engine still converts, and + warns. Spread the nodes with spec.topologySpreadConstraints to earn that. + format: int32 + minimum: 1 + type: integer + required: + - data + - parity + type: object limits: description: Limits are the per-node admission-control limits. Empty means unlimited. diff --git a/internal/controller/extraconfig.go b/internal/controller/extraconfig.go index 19eae57..1cf487e 100644 --- a/internal/controller/extraconfig.go +++ b/internal/controller/extraconfig.go @@ -52,6 +52,7 @@ var reservedConfigPaths = map[string]string{ "storage.policy.downsample": "use spec.policy.downsample", "storage.policy.precision": "use spec.policy.precision", "storage.policy.recompress": "use spec.policy.recompress", + "storage.policy.ec": "use spec.policy.ec", } // validationError marks a spec problem that no amount of retrying can fix: the reconcile is diff --git a/internal/controller/policy.go b/internal/controller/policy.go index 1123f9b..8937fbb 100644 --- a/internal/controller/policy.go +++ b/internal/controller/policy.go @@ -24,6 +24,11 @@ import ( dbv1alpha1 "github.com/oteldb/operator/api/v1alpha1" ) +// maxECShards is the total shard ceiling the storage library enforces on an erasure-coding scheme +// (github.com/oteldb/storage/cluster/ec, Scheme.Validate). Past it the scheme is rejected +// downstream and the tenant silently falls back to full-copy. +const maxECShards = 256 + // oteldb storage.policy config keys. const ( keyPolicy = "policy" @@ -32,8 +37,11 @@ const ( keyDownsample = "downsample" keyPrecision = "precision" keyRecompress = "recompress" + keyEC = "ec" keyAfter = "after" keyInterval = "interval" + keyECData = "data" + keyECParity = "parity" ) // renderPolicy builds the storage.policy block from spec.policy, or returns nil when nothing is @@ -62,6 +70,18 @@ func renderPolicy(cr *dbv1alpha1.OtelDBCluster) map[string]any { } policy[keyRecompress] = m } + if e := spec.EC; e != nil { + m := map[string]any{ + keyECData: e.Data, + keyECParity: e.Parity, + } + // An absent after erasure-codes every part; oteldb spells that as the zero duration, which + // is what an omitted key decodes to. + if e.After != nil { + m[keyAfter] = e.After.Duration.String() + } + policy[keyEC] = m + } if len(policy) == 0 { return nil @@ -161,7 +181,68 @@ func validatePolicy(cr *dbv1alpha1.OtelDBCluster) error { } } - return validateMergeTiers(cr.Spec.Policy) + if err := validateMergeTiers(cr.Spec.Policy); err != nil { + return err + } + return validateEC(cr) +} + +// validateEC rejects an erasure-coding policy the engine would ignore or could not place. EC is +// the one policy whose preconditions are topology, not values, and every one of them fails +// silently: oteldb warns and carries on, the ring clamps an oversized scheme to the members it +// has, and a configured replication factor is dropped without a word. The operator owns the +// topology, so it can turn each of those into a spec error instead. +func validateEC(cr *dbv1alpha1.OtelDBCluster) error { + e := cr.Spec.Policy.EC + if e == nil { + return nil + } + + // Data and Parity are already bounded below by the CRD; only the joint bound is left, and it + // is the storage library's ([ec.Scheme].Validate). + if shards := int64(e.Data) + int64(e.Parity); shards > maxECShards { + return invalidSpec("spec.policy.ec has %d shards (data %d + parity %d), at most %d are allowed", + shards, e.Data, e.Parity, maxECShards) + } + if e.After != nil && e.After.Duration < 0 { + return invalidSpec("spec.policy.ec.after must not be negative, got %s", e.After.Duration) + } + + // Erasure coding needs a shared-nothing cluster. Cluster mode is a given here (spec.etcd is + // required), so a private backend is the only half that can be missing. + if !privateBackendOf(cr) { + return invalidSpec("spec.policy.ec needs a private per-node backend, but spec.storage.backend " + + "is s3 (or spec.cluster.privateBackend is false): a shared object store owns durability " + + "itself, so every part would stay full-copy and the policy would do nothing") + } + + // Data+Parity is the owner count, so the shards need that many nodes to land on. The ring + // clamps to the members it has instead of failing, which converts parts into a scheme the + // cluster cannot actually spread. + if shards := e.Data + e.Parity; shards > replicasOf(cr) { + return invalidSpec("spec.policy.ec spreads %d shards (data %d + parity %d) one per node, "+ + "but spec.replicas is %d", shards, e.Data, e.Parity, replicasOf(cr)) + } + + // Under EC the owner count is Data+Parity and the tenant's replication factor is ignored — + // for the flushed parts and for the unflushed head alike. Honouring one and dropping the other + // is exactly the surprise worth refusing: the CR would document a replication factor the + // cluster does not use. + if rf := cr.Spec.Cluster.ReplicationFactor; rf != nil { + return invalidSpec("spec.policy.ec and spec.cluster.replicationFactor are mutually exclusive: "+ + "erasure coding fixes the owner count at data+parity (%d), and the replication factor "+ + "(%d) is ignored; drop spec.cluster.replicationFactor", e.Data+e.Parity, *rf) + } + + // A tier that only applies after the data is gone is merge work whose output is dropped + // unread, the same rule the other cold tiers get. + if maxAge := cr.Spec.Policy.Retention.MaxAge; maxAge != nil && maxAge.Duration > 0 && + e.After != nil && e.After.Duration >= maxAge.Duration { + return invalidSpec("spec.policy.ec.after (%s) is at or past spec.policy.retention.maxAge (%s): "+ + "parts are dropped before they are erasure-coded", e.After.Duration, maxAge.Duration) + } + + return nil } // validateMergeTiers rejects downsample/precision/recompress settings the engine would ignore. The diff --git a/internal/controller/policy_ec_test.go b/internal/controller/policy_ec_test.go new file mode 100644 index 0000000..63ed27a --- /dev/null +++ b/internal/controller/policy_ec_test.go @@ -0,0 +1,193 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + dbv1alpha1 "github.com/oteldb/operator/api/v1alpha1" +) + +func TestRenderPolicyECAbsentByDefault(t *testing.T) { + storage := renderStorage(t, testCluster()) + require.NotContains(t, storage, "policy", "an unchanged cluster must render no policy at all") +} + +func TestRenderPolicyEC(t *testing.T) { + cr := testCluster() + cr.Spec.Policy.EC = &dbv1alpha1.ECSpec{ + Data: 3, + Parity: 2, + After: &metav1.Duration{Duration: 72 * time.Hour}, + } + + policy, ok := renderStorage(t, cr)["policy"].(map[string]any) + require.True(t, ok, "policy block missing") + require.Equal(t, map[string]any{ + "data": float64(3), + "parity": float64(2), + "after": "72h0m0s", + }, policy["ec"]) +} + +func TestRenderPolicyECWithoutAfter(t *testing.T) { + cr := testCluster() + cr.Spec.Policy.EC = &dbv1alpha1.ECSpec{Data: 4, Parity: 1} + + policy := renderStorage(t, cr)["policy"].(map[string]any) + require.Equal(t, map[string]any{"data": float64(4), "parity": float64(1)}, policy["ec"], + "an unset after must be omitted so oteldb reads it as zero: erasure-code every part") +} + +// storage.policy.ec joins the rest of the policy block as a reserved extraConfig path. +func TestValidateExtraConfigECReserved(t *testing.T) { + err := validateExtraConfig(map[string]any{ + "storage": map[string]any{"policy": map[string]any{"ec": map[string]any{"data": 4}}}, + }) + require.ErrorContains(t, err, "storage.policy.ec (use spec.policy.ec)") +} + +func TestValidateEC(t *testing.T) { + ec := func(data, parity int32) *dbv1alpha1.ECSpec { + return &dbv1alpha1.ECSpec{Data: data, Parity: parity} + } + + tests := []struct { + name string + mutate func(*dbv1alpha1.OtelDBCluster) + wantErr string + }{ + { + name: "absent is valid", + mutate: func(*dbv1alpha1.OtelDBCluster) {}, + }, + { + name: "a scheme that fits the cluster", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.EC = ec(3, 2) + }, + }, + { + name: "too many shards", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.EC = ec(200, 100) + }, + wantErr: "at most 256 are allowed", + }, + { + name: "negative after", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.EC = ec(3, 2) + cr.Spec.Policy.EC.After = &metav1.Duration{Duration: -time.Hour} + }, + wantErr: "spec.policy.ec.after must not be negative", + }, + { + name: "a shared s3 backend cannot erasure-code", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Storage.Backend = dbv1alpha1.StorageBackendS3 + cr.Spec.Storage.S3 = &dbv1alpha1.S3Spec{Bucket: "b"} + cr.Spec.Policy.EC = ec(3, 2) + }, + wantErr: "needs a private per-node backend", + }, + { + name: "an explicitly private s3 backend can", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Storage.Backend = dbv1alpha1.StorageBackendS3 + cr.Spec.Storage.S3 = &dbv1alpha1.S3Spec{Bucket: "b"} + cr.Spec.Cluster.PrivateBackend = ptr.To(true) + cr.Spec.Policy.EC = ec(3, 2) + }, + }, + { + name: "a file backend declared shared cannot", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Cluster.PrivateBackend = ptr.To(false) + cr.Spec.Policy.EC = ec(3, 2) + }, + wantErr: "needs a private per-node backend", + }, + { + name: "more shards than nodes", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.EC = ec(4, 2) + }, + wantErr: "spreads 6 shards (data 4 + parity 2) one per node, but spec.replicas is 5", + }, + { + name: "exactly as many shards as nodes", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.EC = ec(4, 1) + }, + }, + { + name: "a replication factor alongside is ignored, so it is refused", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Cluster.ReplicationFactor = ptr.To[int32](3) + cr.Spec.Policy.EC = ec(3, 2) + }, + wantErr: "spec.policy.ec and spec.cluster.replicationFactor are mutually exclusive", + }, + { + name: "a replication factor without ec stays fine", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Cluster.ReplicationFactor = ptr.To[int32](3) + }, + }, + { + name: "a tier past the retention window never runs", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.Retention.MaxAge = &metav1.Duration{Duration: 24 * time.Hour} + cr.Spec.Policy.EC = ec(3, 2) + cr.Spec.Policy.EC.After = &metav1.Duration{Duration: 48 * time.Hour} + }, + wantErr: "parts are dropped before they are erasure-coded", + }, + { + name: "a tier inside the retention window is fine", + mutate: func(cr *dbv1alpha1.OtelDBCluster) { + cr.Spec.Policy.Retention.MaxAge = &metav1.Duration{Duration: 720 * time.Hour} + cr.Spec.Policy.EC = ec(3, 2) + cr.Spec.Policy.EC.After = &metav1.Duration{Duration: 48 * time.Hour} + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cr := testCluster() + tt.mutate(cr) + + err := validatePolicy(cr) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tt.wantErr) + + var invalid validationError + require.True(t, errors.As(err, &invalid), "must be reported as a spec validation error") + }) + } +} diff --git a/internal/controller/policy_test.go b/internal/controller/policy_test.go index c2805ca..437795d 100644 --- a/internal/controller/policy_test.go +++ b/internal/controller/policy_test.go @@ -243,7 +243,7 @@ func TestValidateExtraConfigReservedPolicyPaths(t *testing.T) { // does not lock users out of the rest of storage.policy. // Every storage.policy key the CRD models is reserved, so extraConfig cannot fight spec.policy. func TestValidateExtraConfigPolicyFullyReserved(t *testing.T) { - for _, key := range []string{"retention", "limits", "downsample", "precision", "recompress"} { + for _, key := range []string{"retention", "limits", "downsample", "precision", "recompress", "ec"} { t.Run(key, func(t *testing.T) { err := validateExtraConfig(map[string]any{ "storage": map[string]any{"policy": map[string]any{key: "whatever"}},