From 60d94b7e92f09fe30d217bcc05b7adb0cc37d401 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Mon, 31 Aug 2026 16:55:33 +0200 Subject: [PATCH 1/6] feat(core): reclaim a stale Kopia cache directory at startup When a tier's cache moves to a different volume, the cache it leaves behind keeps consuming space on the old one, and nothing ever removes it: the cache directory is only wiped when the repository is first initialized. Add an optional `stale_cache` path per tier. When set, and different from the cache in use, the server removes it before initializing the repository. Kopia rebuilds the cache on demand, so there is nothing to migrate. Signed-off-by: Armando Ruocco --- core/cmd/server/initialize.go | 27 +++++++++++ core/cmd/server/initialize_test.go | 73 ++++++++++++++++++++++++++++++ core/pkg/config/server.go | 10 ++++ 3 files changed, 110 insertions(+) create mode 100644 core/cmd/server/initialize_test.go diff --git a/core/cmd/server/initialize.go b/core/cmd/server/initialize.go index 6cf00691..eaabd419 100644 --- a/core/cmd/server/initialize.go +++ b/core/cmd/server/initialize.go @@ -22,10 +22,12 @@ package server import ( "context" "fmt" + "path/filepath" "github.com/cloudnative-pg/machinery/pkg/log" "github.com/cloudnative-pg/klio/core/cmd/initialize" + "github.com/cloudnative-pg/klio/core/internal/kopia" "github.com/cloudnative-pg/klio/core/internal/tier2" "github.com/cloudnative-pg/klio/core/pkg/config" ) @@ -46,7 +48,26 @@ func initializeRepository(ctx context.Context, opts serverOpts) error { return nil } +// reclaimStaleCache removes a Kopia cache directory left behind on another +// volume by a cache that moved. Kopia rebuilds the cache on demand, so the +// leftover is only wasted space. +func reclaimStaleCache(ctx context.Context, stale, inUse string) error { + if stale == "" || filepath.Clean(stale) == filepath.Clean(inUse) { + return nil + } + + log.FromContext(ctx).Info("Reclaiming stale Kopia cache directory", "directory", stale) + + return kopia.CleanupCacheDirectory(stale) +} + func initializeTier1(ctx context.Context, cfg *config.ServerConfig) error { + if err := reclaimStaleCache( + ctx, cfg.Tier1.Base.StaleCacheDirectory, cfg.Tier1.Base.CacheDirectory, + ); err != nil { + return err + } + walDirectory := cfg.Tier1.Wal.WALPath kopiaDirectory := cfg.Tier1.Base.RepositoryDirectory @@ -60,6 +81,12 @@ func initializeTier1(ctx context.Context, cfg *config.ServerConfig) error { } func initializeTier2(ctx context.Context, cfg *config.ServerConfig) error { + if err := reclaimStaleCache( + ctx, cfg.Tier2.StaleCacheDirectory, cfg.Tier2.CacheDirectory, + ); err != nil { + return err + } + tier2BaseFS, err := tier2.ConnectBase(ctx, &cfg.Tier2) if err != nil { return fmt.Errorf("error while connecting to tier2 (base): %w", err) diff --git a/core/cmd/server/initialize_test.go b/core/cmd/server/initialize_test.go new file mode 100644 index 00000000..f0924326 --- /dev/null +++ b/core/cmd/server/initialize_test.go @@ -0,0 +1,73 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +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. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package server + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newCacheDirectory(t *testing.T, parent, name string) string { + t.Helper() + + dir := filepath.Join(parent, name) + require.NoError(t, os.MkdirAll(dir, 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "blob"), []byte("cached"), 0o600)) + + return dir +} + +func TestReclaimStaleCacheRemovesTheUnusedDirectory(t *testing.T) { + root := t.TempDir() + stale := newCacheDirectory(t, root, "stale") + inUse := newCacheDirectory(t, root, "in-use") + + require.NoError(t, reclaimStaleCache(context.Background(), stale, inUse)) + + assert.NoDirExists(t, stale) + assert.DirExists(t, inUse) +} + +func TestReclaimStaleCacheKeepsTheDirectoryInUse(t *testing.T) { + root := t.TempDir() + inUse := newCacheDirectory(t, root, "in-use") + + // The two paths point at the same directory, spelled differently. + require.NoError(t, reclaimStaleCache(context.Background(), inUse+"/", inUse)) + + assert.DirExists(t, inUse) +} + +func TestReclaimStaleCacheIsANoOpWhenUnset(t *testing.T) { + inUse := newCacheDirectory(t, t.TempDir(), "in-use") + + require.NoError(t, reclaimStaleCache(context.Background(), "", inUse)) + + assert.DirExists(t, inUse) +} + +func TestReclaimStaleCacheRefusesRelativePaths(t *testing.T) { + require.Error(t, reclaimStaleCache(context.Background(), "cache", "/data/cache")) +} diff --git a/core/pkg/config/server.go b/core/pkg/config/server.go index 09b77188..5ee045bc 100644 --- a/core/pkg/config/server.go +++ b/core/pkg/config/server.go @@ -98,6 +98,11 @@ type Tier2Config struct { // CacheDirectory is the directory of the Kopia cache CacheDirectory string `mapstructure:"cache"` + // StaleCacheDirectory is a cache directory that is no longer in use and + // whose space must be reclaimed at startup. It is set when the cache moved + // to a different volume. Empty when there is nothing to reclaim. + StaleCacheDirectory string `mapstructure:"stale_cache"` + // S3 contains the configuration parameters for an S3-based tier 2 S3 S3Configuration `json:"s3" mapstructure:"s3"` } @@ -108,6 +113,11 @@ type BaseServerConfig struct { // CacheDirectory is the directory of the Kopia cache CacheDirectory string `mapstructure:"cache"` + // StaleCacheDirectory is a cache directory that is no longer in use and + // whose space must be reclaimed at startup. It is set when the cache moved + // to a different volume. Empty when there is nothing to reclaim. + StaleCacheDirectory string `mapstructure:"stale_cache"` + // RepositoryDirectory is the directory where the Kopia repository is stored. RepositoryDirectory string `mapstructure:"repository"` From 8e4995e2d9ef8ec4d31efc683876d1216accd48c Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Mon, 31 Aug 2026 16:55:43 +0200 Subject: [PATCH 2/6] feat(operator): make the tier1 and tier2 cache volumes optional A Server had to dedicate a PVC to the Kopia cache of every tier it configured, which is three volumes for the common tier1+tier2 setup and a decision the user has no basis to make before running anything. `tier1.cache` and `tier2.cache` are now optional. When a tier has no cache stanza, its cache lives in the tier1 data volume, under /data/cache_tier1 and /data/cache_tier2. Since the fallback needs the data volume, tier2.cache stays required when tier1 is not configured, which a CEL rule enforces; the cache shrink rules now tolerate a cache that is absent on either side of an update. The stanza can be added or removed on a running server: the VolumeClaimTemplates change recreates the StatefulSet through the existing path, the server reclaims a cache left behind in the data volume, and the operator deletes the PVC of a cache that is no longer dedicated. Cache PVCs are the only ones reclaimed, as they hold no backup data. Signed-off-by: Armando Ruocco --- operator/api/v1alpha1/server_types.go | 18 +- .../api/v1alpha1/zz_generated.deepcopy.go | 12 +- .../crd/bases/klio.cnpg.io_servers.yaml | 25 +-- operator/config/rbac/role.yaml | 1 + operator/dist/chart/crds/server-crd.yaml | 25 +-- .../dist/chart/templates/manager-rbac.yaml | 1 + .../controller/server_cachevalidation_test.go | 169 ++++++++++++++++++ .../internal/controller/server_controller.go | 2 +- .../controller/server_controller_test.go | 2 +- .../internal/controller/server_envbuilder.go | 35 +++- .../controller/server_envbuilder_test.go | 56 ++++++ .../internal/controller/server_pvc_resize.go | 50 +++++- .../controller/server_pvc_resize_test.go | 64 ++++++- .../internal/controller/server_reconciler.go | 46 ++++- .../controller/server_statefulset_test.go | 72 +++++++- operator/pkg/config/server.go | 10 ++ operator/test/e2e/server_reconfig_test.go | 116 ++++++++++++ operator/test/klio/features/pvc_resize.go | 2 +- operator/test/utils/templates/klio/klio.go | 4 +- 19 files changed, 660 insertions(+), 50 deletions(-) create mode 100644 operator/internal/controller/server_cachevalidation_test.go diff --git a/operator/api/v1alpha1/server_types.go b/operator/api/v1alpha1/server_types.go index 4f98cb5f..36156d3c 100644 --- a/operator/api/v1alpha1/server_types.go +++ b/operator/api/v1alpha1/server_types.go @@ -41,6 +41,7 @@ const ( // +kubebuilder:validation:XValidation:rule="!(self.mode == 'read-only' && has(self.tier1))",message="tier1 cannot be set when mode is read-only" // +kubebuilder:validation:XValidation:rule="!(self.mode == 'read-only' && has(self.queue))",message="queue cannot be set when mode is read-only" // +kubebuilder:validation:XValidation:rule="self.mode == 'read-only' || has(self.queue)",message="queue is required when tier1 is configured" +// +kubebuilder:validation:XValidation:rule="!has(self.tier2) || has(self.tier1) || has(self.tier2.cache)",message="tier2.cache is required when tier1 is not configured" type ServerSpec struct { // ImageConfiguration tells how to download the Klio // image. @@ -178,8 +179,10 @@ type FileSource struct { // Tier1Configuration is the tier 1 configuration. type Tier1Configuration struct { // Cache is the configuration of the PVC that should be - // used for the cache. - Cache Cache `json:"cache"` + // used for the cache. When omitted, the Kopia cache is stored in a + // directory inside the tier1 data volume. + // +optional + Cache *Cache `json:"cache,omitempty"` // Data is the configuration of the PVC that should be used // for the base backups. @@ -196,8 +199,11 @@ type Tier1Configuration struct { // Tier2Configuration is the tier 2 configuration. type Tier2Configuration struct { // Cache is the configuration of the PVC that should be - // used for the cache. - Cache Cache `json:"cache"` + // used for the cache. When omitted, the Kopia cache is stored in a + // directory inside the tier1 data volume, and is therefore required + // when tier1 is not configured. + // +optional + Cache *Cache `json:"cache,omitempty"` // S3 contains the configuration parameters for an S3-based tier 2. S3 *S3Configuration `json:"s3"` @@ -254,8 +260,8 @@ type ServerStatus struct { // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !('storage' in oldSelf.spec.tier1.data.pvcTemplate.resources.requests) || !('storage' in self.spec.tier1.data.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.data.pvcTemplate.resources.requests['storage']).isLessThan(quantity(oldSelf.spec.tier1.data.pvcTemplate.resources.requests['storage']))",message="tier1.data PVC size cannot be decreased" -// +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !('storage' in oldSelf.spec.tier1.cache.pvcTemplate.resources.requests) || !('storage' in self.spec.tier1.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.cache.pvcTemplate.resources.requests['storage']).isLessThan(quantity(oldSelf.spec.tier1.cache.pvcTemplate.resources.requests['storage']))",message="tier1.cache PVC size cannot be decreased" -// +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.tier2) || !has(self.spec.tier2) || !('storage' in oldSelf.spec.tier2.cache.pvcTemplate.resources.requests) || !('storage' in self.spec.tier2.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier2.cache.pvcTemplate.resources.requests['storage']).isLessThan(quantity(oldSelf.spec.tier2.cache.pvcTemplate.resources.requests['storage']))",message="tier2.cache PVC size cannot be decreased" +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !has(oldSelf.spec.tier1.cache) || !has(self.spec.tier1.cache) || !('storage' in oldSelf.spec.tier1.cache.pvcTemplate.resources.requests) || !('storage' in self.spec.tier1.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.cache.pvcTemplate.resources.requests['storage']).isLessThan(quantity(oldSelf.spec.tier1.cache.pvcTemplate.resources.requests['storage']))",message="tier1.cache PVC size cannot be decreased" +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.tier2) || !has(self.spec.tier2) || !has(oldSelf.spec.tier2.cache) || !has(self.spec.tier2.cache) || !('storage' in oldSelf.spec.tier2.cache.pvcTemplate.resources.requests) || !('storage' in self.spec.tier2.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier2.cache.pvcTemplate.resources.requests['storage']).isLessThan(quantity(oldSelf.spec.tier2.cache.pvcTemplate.resources.requests['storage']))",message="tier2.cache PVC size cannot be decreased" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.queue) || !has(self.spec.queue) || !('storage' in oldSelf.spec.queue.pvcTemplate.resources.requests) || !('storage' in self.spec.queue.pvcTemplate.resources.requests) || !quantity(self.spec.queue.pvcTemplate.resources.requests['storage']).isLessThan(quantity(oldSelf.spec.queue.pvcTemplate.resources.requests['storage']))",message="queue PVC size cannot be decreased" // Server is the Schema for the servers API. diff --git a/operator/api/v1alpha1/zz_generated.deepcopy.go b/operator/api/v1alpha1/zz_generated.deepcopy.go index d0ac006c..abb69d96 100644 --- a/operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/operator/api/v1alpha1/zz_generated.deepcopy.go @@ -507,7 +507,11 @@ func (in *TLSConfiguration) DeepCopy() *TLSConfiguration { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Tier1Configuration) DeepCopyInto(out *Tier1Configuration) { *out = *in - in.Cache.DeepCopyInto(&out.Cache) + if in.Cache != nil { + in, out := &in.Cache, &out.Cache + *out = new(Cache) + (*in).DeepCopyInto(*out) + } in.Data.DeepCopyInto(&out.Data) in.EncryptionKeyFile.DeepCopyInto(&out.EncryptionKeyFile) in.IdentityFile.DeepCopyInto(&out.IdentityFile) @@ -546,7 +550,11 @@ func (in *Tier1PluginConfiguration) DeepCopy() *Tier1PluginConfiguration { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Tier2Configuration) DeepCopyInto(out *Tier2Configuration) { *out = *in - in.Cache.DeepCopyInto(&out.Cache) + if in.Cache != nil { + in, out := &in.Cache, &out.Cache + *out = new(Cache) + (*in).DeepCopyInto(*out) + } if in.S3 != nil { in, out := &in.S3, &out.S3 *out = new(S3Configuration) diff --git a/operator/config/crd/bases/klio.cnpg.io_servers.yaml b/operator/config/crd/bases/klio.cnpg.io_servers.yaml index 6274b0fd..2c0b71fd 100644 --- a/operator/config/crd/bases/klio.cnpg.io_servers.yaml +++ b/operator/config/crd/bases/klio.cnpg.io_servers.yaml @@ -9107,7 +9107,8 @@ spec: cache: description: |- Cache is the configuration of the PVC that should be - used for the cache. + used for the cache. When omitted, the Kopia cache is stored in a + directory inside the tier1 data volume. properties: pvcTemplate: description: |- @@ -13598,7 +13599,6 @@ spec: rule: '[has(self.fileReference)].filter(x,x==true).size() == 1' required: - - cache - data - encryptionKeyFile - identityFile @@ -13609,7 +13609,9 @@ spec: cache: description: |- Cache is the configuration of the PVC that should be - used for the cache. + used for the cache. When omitted, the Kopia cache is stored in a + directory inside the tier1 data volume, and is therefore required + when tier1 is not configured. properties: pvcTemplate: description: |- @@ -17969,7 +17971,6 @@ spec: - bucketName type: object required: - - cache - encryptionKeyFile - identityFile - s3 @@ -17996,6 +17997,8 @@ spec: rule: '!(self.mode == ''read-only'' && has(self.queue))' - message: queue is required when tier1 is configured rule: self.mode == 'read-only' || has(self.queue) + - message: tier2.cache is required when tier1 is not configured + rule: '!has(self.tier2) || has(self.tier1) || has(self.tier2.cache)' status: description: ServerStatus defines the observed state of Server. type: object @@ -18009,13 +18012,15 @@ spec: in oldSelf.spec.tier1.data.pvcTemplate.resources.requests) || !(''storage'' in self.spec.tier1.data.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.data.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier1.data.pvcTemplate.resources.requests[''storage'']))' - message: tier1.cache PVC size cannot be decreased - rule: '!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !(''storage'' - in oldSelf.spec.tier1.cache.pvcTemplate.resources.requests) || !(''storage'' - in self.spec.tier1.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']))' + rule: '!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !has(oldSelf.spec.tier1.cache) + || !has(self.spec.tier1.cache) || !(''storage'' in oldSelf.spec.tier1.cache.pvcTemplate.resources.requests) + || !(''storage'' in self.spec.tier1.cache.pvcTemplate.resources.requests) + || !quantity(self.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']))' - message: tier2.cache PVC size cannot be decreased - rule: '!has(oldSelf.spec.tier2) || !has(self.spec.tier2) || !(''storage'' - in oldSelf.spec.tier2.cache.pvcTemplate.resources.requests) || !(''storage'' - in self.spec.tier2.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']))' + rule: '!has(oldSelf.spec.tier2) || !has(self.spec.tier2) || !has(oldSelf.spec.tier2.cache) + || !has(self.spec.tier2.cache) || !(''storage'' in oldSelf.spec.tier2.cache.pvcTemplate.resources.requests) + || !(''storage'' in self.spec.tier2.cache.pvcTemplate.resources.requests) + || !quantity(self.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']))' - message: queue PVC size cannot be decreased rule: '!has(oldSelf.spec.queue) || !has(self.spec.queue) || !(''storage'' in oldSelf.spec.queue.pvcTemplate.resources.requests) || !(''storage'' diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index 5237110a..a1dbac67 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -15,6 +15,7 @@ rules: resources: - persistentvolumeclaims verbs: + - delete - get - list - patch diff --git a/operator/dist/chart/crds/server-crd.yaml b/operator/dist/chart/crds/server-crd.yaml index 3aadb6d7..e90f44dc 100644 --- a/operator/dist/chart/crds/server-crd.yaml +++ b/operator/dist/chart/crds/server-crd.yaml @@ -9106,7 +9106,8 @@ spec: cache: description: |- Cache is the configuration of the PVC that should be - used for the cache. + used for the cache. When omitted, the Kopia cache is stored in a + directory inside the tier1 data volume. properties: pvcTemplate: description: |- @@ -13597,7 +13598,6 @@ spec: rule: '[has(self.fileReference)].filter(x,x==true).size() == 1' required: - - cache - data - encryptionKeyFile - identityFile @@ -13608,7 +13608,9 @@ spec: cache: description: |- Cache is the configuration of the PVC that should be - used for the cache. + used for the cache. When omitted, the Kopia cache is stored in a + directory inside the tier1 data volume, and is therefore required + when tier1 is not configured. properties: pvcTemplate: description: |- @@ -17968,7 +17970,6 @@ spec: - bucketName type: object required: - - cache - encryptionKeyFile - identityFile - s3 @@ -17995,6 +17996,8 @@ spec: rule: '!(self.mode == ''read-only'' && has(self.queue))' - message: queue is required when tier1 is configured rule: self.mode == 'read-only' || has(self.queue) + - message: tier2.cache is required when tier1 is not configured + rule: '!has(self.tier2) || has(self.tier1) || has(self.tier2.cache)' status: description: ServerStatus defines the observed state of Server. type: object @@ -18008,13 +18011,15 @@ spec: in oldSelf.spec.tier1.data.pvcTemplate.resources.requests) || !(''storage'' in self.spec.tier1.data.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.data.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier1.data.pvcTemplate.resources.requests[''storage'']))' - message: tier1.cache PVC size cannot be decreased - rule: '!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !(''storage'' - in oldSelf.spec.tier1.cache.pvcTemplate.resources.requests) || !(''storage'' - in self.spec.tier1.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']))' + rule: '!has(oldSelf.spec.tier1) || !has(self.spec.tier1) || !has(oldSelf.spec.tier1.cache) + || !has(self.spec.tier1.cache) || !(''storage'' in oldSelf.spec.tier1.cache.pvcTemplate.resources.requests) + || !(''storage'' in self.spec.tier1.cache.pvcTemplate.resources.requests) + || !quantity(self.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier1.cache.pvcTemplate.resources.requests[''storage'']))' - message: tier2.cache PVC size cannot be decreased - rule: '!has(oldSelf.spec.tier2) || !has(self.spec.tier2) || !(''storage'' - in oldSelf.spec.tier2.cache.pvcTemplate.resources.requests) || !(''storage'' - in self.spec.tier2.cache.pvcTemplate.resources.requests) || !quantity(self.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']))' + rule: '!has(oldSelf.spec.tier2) || !has(self.spec.tier2) || !has(oldSelf.spec.tier2.cache) + || !has(self.spec.tier2.cache) || !(''storage'' in oldSelf.spec.tier2.cache.pvcTemplate.resources.requests) + || !(''storage'' in self.spec.tier2.cache.pvcTemplate.resources.requests) + || !quantity(self.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']).isLessThan(quantity(oldSelf.spec.tier2.cache.pvcTemplate.resources.requests[''storage'']))' - message: queue PVC size cannot be decreased rule: '!has(oldSelf.spec.queue) || !has(self.spec.queue) || !(''storage'' in oldSelf.spec.queue.pvcTemplate.resources.requests) || !(''storage'' diff --git a/operator/dist/chart/templates/manager-rbac.yaml b/operator/dist/chart/templates/manager-rbac.yaml index 74d154cd..2f564d50 100644 --- a/operator/dist/chart/templates/manager-rbac.yaml +++ b/operator/dist/chart/templates/manager-rbac.yaml @@ -16,6 +16,7 @@ rules: resources: - persistentvolumeclaims verbs: + - delete - get - list - patch diff --git a/operator/internal/controller/server_cachevalidation_test.go b/operator/internal/controller/server_cachevalidation_test.go new file mode 100644 index 00000000..df3f8bb1 --- /dev/null +++ b/operator/internal/controller/server_cachevalidation_test.go @@ -0,0 +1,169 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +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. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package controller + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + kliov1alpha1 "github.com/cloudnative-pg/klio/operator/api/v1alpha1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func cacheTestPVCTemplate(size string) corev1.PersistentVolumeClaimSpec { + return corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse(size), + }, + }, + } +} + +func cacheTestFileSource(secretName, path string) kliov1alpha1.FileSource { + return kliov1alpha1.FileSource{ + FileReference: &kliov1alpha1.FileReference{ + Volume: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{SecretName: secretName}, + }, + Path: path, + }, + } +} + +func cacheTestServer(name string) *kliov1alpha1.Server { + return &kliov1alpha1.Server{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: kliov1alpha1.ServerSpec{ + ImageConfiguration: kliov1alpha1.ImageConfiguration{Image: "klio:test"}, + TLSConfiguration: kliov1alpha1.TLSConfiguration{ + TLSSecretName: "tls-secret", + ClientCASecretName: "ca-secret", + }, + Mode: kliov1alpha1.ModeStandard, + Tier1: &kliov1alpha1.Tier1Configuration{ + Data: kliov1alpha1.Data{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("1Gi")}, + EncryptionKeyFile: cacheTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: cacheTestFileSource("id-secret", "identity.txt"), + }, + Queue: &kliov1alpha1.Queue{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("1Gi")}, + }, + } +} + +func cacheTestTier2(cache *kliov1alpha1.Cache) *kliov1alpha1.Tier2Configuration { + return &kliov1alpha1.Tier2Configuration{ + Cache: cache, + S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, + EncryptionKeyFile: cacheTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: cacheTestFileSource("id-secret", "identity.txt"), + } +} + +var _ = Describe("Server cache validation", func() { + ctx := context.Background() + + var created []*kliov1alpha1.Server + + create := func(server *kliov1alpha1.Server) error { + err := k8sClient.Create(ctx, server) + if err == nil { + created = append(created, server) + } + + return err + } + + AfterEach(func() { + for _, server := range created { + Expect(k8sClient.Delete(ctx, server)).To(Succeed()) + } + created = nil + }) + + It("accepts a tier1 without a dedicated cache volume", func() { + Expect(create(cacheTestServer("cache-optional-tier1"))).To(Succeed()) + }) + + It("accepts a tier2 without a dedicated cache volume when tier1 is configured", func() { + server := cacheTestServer("cache-optional-tier2") + server.Spec.Tier2 = cacheTestTier2(nil) + Expect(create(server)).To(Succeed()) + }) + + It("rejects a tier2 without a dedicated cache volume when tier1 is missing", func() { + server := cacheTestServer("cache-required-tier2") + server.Spec.Mode = kliov1alpha1.ModeReadOnly + server.Spec.Tier1 = nil + server.Spec.Queue = nil + server.Spec.Tier2 = cacheTestTier2(nil) + + Expect(create(server)).To(MatchError( + ContainSubstring("tier2.cache is required when tier1 is not configured"))) + }) + + It("accepts a read-only server with a dedicated tier2 cache volume", func() { + server := cacheTestServer("cache-readonly-tier2") + server.Spec.Mode = kliov1alpha1.ModeReadOnly + server.Spec.Tier1 = nil + server.Spec.Queue = nil + server.Spec.Tier2 = cacheTestTier2(&kliov1alpha1.Cache{ + PersistentVolumeClaimTemplate: cacheTestPVCTemplate("1Gi"), + }) + + Expect(create(server)).To(Succeed()) + }) + + // The shrink rules dereference spec.tier{1,2}.cache: they must tolerate a + // cache that is absent on either side of the update. + DescribeTable("allows adding and removing cache volumes", + func(before, after *kliov1alpha1.Cache) { + server := cacheTestServer(fmt.Sprintf("cache-transition-%t-%t", before == nil, after == nil)) + server.Spec.Tier1.Cache = before + server.Spec.Tier2 = cacheTestTier2(before) + Expect(create(server)).To(Succeed()) + + server.Spec.Tier1.Cache = after + server.Spec.Tier2.Cache = after + Expect(k8sClient.Update(ctx, server)).To(Succeed()) + }, + Entry("adding", nil, &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("2Gi")}), + Entry("removing", &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("2Gi")}, nil), + ) + + It("still refuses to shrink a cache volume that stays configured", func() { + server := cacheTestServer("cache-shrink") + server.Spec.Tier1.Cache = &kliov1alpha1.Cache{ + PersistentVolumeClaimTemplate: cacheTestPVCTemplate("2Gi"), + } + Expect(create(server)).To(Succeed()) + + server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate = cacheTestPVCTemplate("1Gi") + Expect(k8sClient.Update(ctx, server)).To(MatchError( + ContainSubstring("tier1.cache PVC size cannot be decreased"))) + }) +}) diff --git a/operator/internal/controller/server_controller.go b/operator/internal/controller/server_controller.go index 46e1e89d..bef4a10c 100644 --- a/operator/internal/controller/server_controller.go +++ b/operator/internal/controller/server_controller.go @@ -56,7 +56,7 @@ type ServerReconciler struct { // +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update // +kubebuilder:rbac:groups="",resources=events,verbs=create -// +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;patch +// +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;patch;delete //nolint:godox // TODO(user): Modify the Reconcile function to compare the state specified by diff --git a/operator/internal/controller/server_controller_test.go b/operator/internal/controller/server_controller_test.go index 67f70d06..4fcb1c10 100644 --- a/operator/internal/controller/server_controller_test.go +++ b/operator/internal/controller/server_controller_test.go @@ -75,7 +75,7 @@ var _ = Describe("Server Controller", func() { }, Mode: kliov1alpha1.ModeStandard, Tier1: &kliov1alpha1.Tier1Configuration{ - Cache: kliov1alpha1.Cache{ + Cache: &kliov1alpha1.Cache{ PersistentVolumeClaimTemplate: pvcTemplate, }, Data: kliov1alpha1.Data{ diff --git a/operator/internal/controller/server_envbuilder.go b/operator/internal/controller/server_envbuilder.go index c9f22758..83037f2d 100644 --- a/operator/internal/controller/server_envbuilder.go +++ b/operator/internal/controller/server_envbuilder.go @@ -30,6 +30,29 @@ import ( const kopiaCacheSubdirectory = "kopia-cache" +// cacheDirectory returns the Kopia cache directory for a tier: the dedicated +// cache volume when one is configured, the fallback location inside the tier1 +// data volume otherwise. The cache always lives one level below the mount +// point so that wiping it never touches the volume root. +func cacheDirectory(cache *kliov1alpha1.Cache, mountPath, fallbackPath string) string { + if cache == nil { + return path.Join(fallbackPath, kopiaCacheSubdirectory) + } + + return path.Join(mountPath, kopiaCacheSubdirectory) +} + +// staleCacheDirectory returns the cache location a tier is configured *not* to +// use, so that the server can reclaim the space of a cache left behind by a +// migration to a dedicated cache volume. +func staleCacheDirectory(cache *kliov1alpha1.Cache, fallbackPath string) string { + if cache == nil { + return "" + } + + return path.Join(fallbackPath, kopiaCacheSubdirectory) +} + type envBuilder struct { builtEnvs []corev1.EnvVar @@ -114,7 +137,11 @@ func (e *envBuilder) getCoreEnvVars() []corev1.EnvVar { tier1Envs = append(tier1Envs, corev1.EnvVar{ Name: "TIER1_BASE_CACHE", - Value: path.Join(kopiaCacheTier1MountPath, kopiaCacheSubdirectory), + Value: cacheDirectory(e.tier1.Cache, kopiaCacheTier1MountPath, fallbackCacheTier1Path), + }, + corev1.EnvVar{ + Name: "TIER1_BASE_STALE_CACHE", + Value: staleCacheDirectory(e.tier1.Cache, fallbackCacheTier1Path), }, corev1.EnvVar{ Name: "TIER1_BASE_REPOSITORY", @@ -177,7 +204,11 @@ func (e *envBuilder) getTier2EnvVars() []corev1.EnvVar { }, { Name: "TIER2_CACHE", - Value: path.Join(kopiaCacheTier2MountPath, kopiaCacheSubdirectory), + Value: cacheDirectory(e.tier2.Cache, kopiaCacheTier2MountPath, fallbackCacheTier2Path), + }, + { + Name: "TIER2_STALE_CACHE", + Value: staleCacheDirectory(e.tier2.Cache, fallbackCacheTier2Path), }, { Name: "TIER2_BASE_LISTEN_ADDRESS", diff --git a/operator/internal/controller/server_envbuilder_test.go b/operator/internal/controller/server_envbuilder_test.go index c41ca69f..3bed53ae 100644 --- a/operator/internal/controller/server_envbuilder_test.go +++ b/operator/internal/controller/server_envbuilder_test.go @@ -333,3 +333,59 @@ func TestBuildIdentityVolMountProjected(t *testing.T) { assert.Equal(t, int32(0o400), *vol.Projected.DefaultMode) assert.True(t, mount.ReadOnly) } + +func TestCacheEnvVarsUseTheDedicatedVolumeWhenConfigured(t *testing.T) { + builder := &envBuilder{ + tier1: &kliov1alpha1.Tier1Configuration{ + Cache: &kliov1alpha1.Cache{}, + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + }, + tier2: &kliov1alpha1.Tier2Configuration{ + Cache: &kliov1alpha1.Cache{}, + S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + }, + } + + envVars := append(builder.getCoreEnvVars(), builder.getTier2EnvVars()...) + + assert.Equal(t, "/cache_tier1/kopia-cache", findEnvVar(envVars, "TIER1_BASE_CACHE").Value) + assert.Equal(t, "/cache_tier2/kopia-cache", findEnvVar(envVars, "TIER2_CACHE").Value) + + // The fallback locations are reported as stale so that a cache left there by + // a previous configuration is reclaimed. + assert.Equal(t, "/data/cache_tier1/kopia-cache", findEnvVar(envVars, "TIER1_BASE_STALE_CACHE").Value) + assert.Equal(t, "/data/cache_tier2/kopia-cache", findEnvVar(envVars, "TIER2_STALE_CACHE").Value) +} + +func TestCacheEnvVarsFallBackToTheDataVolume(t *testing.T) { + builder := &envBuilder{ + tier1: &kliov1alpha1.Tier1Configuration{ + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + }, + tier2: &kliov1alpha1.Tier2Configuration{ + S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + }, + } + + envVars := append(builder.getCoreEnvVars(), builder.getTier2EnvVars()...) + + assert.Equal(t, "/data/cache_tier1/kopia-cache", findEnvVar(envVars, "TIER1_BASE_CACHE").Value) + assert.Equal(t, "/data/cache_tier2/kopia-cache", findEnvVar(envVars, "TIER2_CACHE").Value) + + // Nothing to reclaim: the fallback locations are the ones in use. + assert.Empty(t, findEnvVar(envVars, "TIER1_BASE_STALE_CACHE").Value) + assert.Empty(t, findEnvVar(envVars, "TIER2_STALE_CACHE").Value) +} + +// The two tiers must never share a cache directory: the server refuses to start +// when they do. +func TestFallbackCachePathsDiffer(t *testing.T) { + assert.NotEqual(t, fallbackCacheTier1Path, fallbackCacheTier2Path) + assert.NotEqual(t, fallbackCacheTier1Path, kopiaDataMountPath) +} diff --git a/operator/internal/controller/server_pvc_resize.go b/operator/internal/controller/server_pvc_resize.go index ee68c402..c25e4f99 100644 --- a/operator/internal/controller/server_pvc_resize.go +++ b/operator/internal/controller/server_pvc_resize.go @@ -158,13 +158,17 @@ func (r *ServerReconciler) buildDesiredPVCSizes(server *kliov1alpha1.Server) map if size, ok := server.Spec.Tier1.Data.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { sizes[pvcTypeData] = size } - if size, ok := server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeCacheTier1] = size + if server.Spec.Tier1.Cache != nil { + if size, ok := server.Spec.Tier1.Cache. + PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { + sizes[pvcTypeCacheTier1] = size + } } } - if server.Spec.Tier2 != nil { - if size, ok := server.Spec.Tier2.Cache.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { + if server.Spec.Tier2 != nil && server.Spec.Tier2.Cache != nil { + if size, ok := server.Spec.Tier2.Cache. + PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { sizes[pvcTypeCacheTier2] = size } } @@ -178,6 +182,44 @@ func (r *ServerReconciler) buildDesiredPVCSizes(server *kliov1alpha1.Server) map return sizes } +// deleteOrphanCachePVCs removes the cache PVCs of tiers that no longer request a +// dedicated cache volume. Only cache PVCs are reclaimed: they hold no backup +// data, and Kopia rebuilds the cache on demand. +func (r *ServerReconciler) deleteOrphanCachePVCs(ctx context.Context, server *kliov1alpha1.Server) error { + contextLogger := logf.FromContext(ctx) + + orphaned := map[string]bool{ + pvcTypeCacheTier1: server.Spec.Tier1 == nil || server.Spec.Tier1.Cache == nil, + pvcTypeCacheTier2: server.Spec.Tier2 == nil || server.Spec.Tier2.Cache == nil, + } + + var pvcList corev1.PersistentVolumeClaimList + if err := r.List(ctx, &pvcList, + client.InNamespace(server.Namespace), + client.MatchingLabels{klioServerLabel: server.Name}, + ); err != nil { + return fmt.Errorf("failed to list PVCs: %w", err) + } + + for i := range pvcList.Items { + pvc := &pvcList.Items[i] + if !orphaned[pvc.Labels[pvcTypeLabel]] { + continue + } + + contextLogger.Info("Deleting cache PVC of a tier that no longer requests one", "pvc", pvc.Name) + + if err := r.Delete(ctx, pvc); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete orphan cache PVC %s: %w", pvc.Name, err) + } + + r.Recorder.Eventf(server, nil, corev1.EventTypeNormal, "CachePVCDeleted", + "DeleteOrphanCachePVC", "Cache PVC %s deleted: the tier has no dedicated cache volume", pvc.Name) + } + + return nil +} + // isVolumeExpansionError checks if the error indicates the StorageClass doesn't support volume expansion. func isVolumeExpansionError(err error) bool { if !apierrors.IsInvalid(err) && !apierrors.IsForbidden(err) { diff --git a/operator/internal/controller/server_pvc_resize_test.go b/operator/internal/controller/server_pvc_resize_test.go index e9a5429c..8b0fea28 100644 --- a/operator/internal/controller/server_pvc_resize_test.go +++ b/operator/internal/controller/server_pvc_resize_test.go @@ -94,7 +94,7 @@ func newTestServerTier1(dataSize, cacheSize string) *kliov1alpha1.Server { Spec: kliov1alpha1.ServerSpec{ Tier1: &kliov1alpha1.Tier1Configuration{ Data: kliov1alpha1.Data{PersistentVolumeClaimTemplate: newPVCSpec(dataSize)}, - Cache: kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec(cacheSize)}, + Cache: &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec(cacheSize)}, }, }, } @@ -119,7 +119,7 @@ func TestBuildDesiredPVCSizesTier2Only(t *testing.T) { Spec: kliov1alpha1.ServerSpec{ Mode: kliov1alpha1.ModeReadOnly, Tier2: &kliov1alpha1.Tier2Configuration{ - Cache: kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("20Gi")}, + Cache: &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("20Gi")}, S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, }, }, @@ -134,7 +134,7 @@ func TestBuildDesiredPVCSizesTier2Only(t *testing.T) { func TestBuildDesiredPVCSizesBothTiers(t *testing.T) { server := newTestServerTier1("100Gi", "10Gi") server.Spec.Tier2 = &kliov1alpha1.Tier2Configuration{ - Cache: kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("20Gi")}, + Cache: &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("20Gi")}, S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, } server.Spec.Queue = &kliov1alpha1.Queue{PersistentVolumeClaimTemplate: newPVCSpec("5Gi")} @@ -394,3 +394,61 @@ func TestExpandPVC(t *testing.T) { assert.Equal(t, 0, expectedSize.Cmp(actualSize), "PVC size mismatch: expected %s, got %s", expectedSize.String(), actualSize.String()) } + +// --- deleteOrphanCachePVCs tests --- + +func TestBuildDesiredPVCSizesSkipsTiersWithoutCache(t *testing.T) { + server := newTestServerTier1("100Gi", "10Gi") + server.Spec.Tier1.Cache = nil + server.Spec.Tier2 = &kliov1alpha1.Tier2Configuration{} + + sizes := (&ServerReconciler{}).buildDesiredPVCSizes(server) + + assert.NotContains(t, sizes, pvcTypeCacheTier1) + assert.NotContains(t, sizes, pvcTypeCacheTier2) + assert.Contains(t, sizes, pvcTypeData) +} + +func TestDeleteOrphanCachePVCsRemovesOnlyUnusedCaches(t *testing.T) { + dataPVC := newTestPVC("data-test-server-klio-0", "test-server", pvcTypeData, "100Gi") + queuePVC := newTestPVC("queue-test-server-klio-0", "test-server", pvcTypeQueue, "1Gi") + tier1CachePVC := newTestPVC("cachetier1-test-server-klio-0", "test-server", pvcTypeCacheTier1, "10Gi") + tier2CachePVC := newTestPVC("cachetier2-test-server-klio-0", "test-server", pvcTypeCacheTier2, "10Gi") + + reconciler, fakeClient := newTestReconciler(dataPVC, queuePVC, tier1CachePVC, tier2CachePVC) + + // tier1 keeps its dedicated cache, tier2 does not. + server := newTestServerTier1("100Gi", "10Gi") + server.Spec.Tier2 = &kliov1alpha1.Tier2Configuration{} + + require.NoError(t, reconciler.deleteOrphanCachePVCs(context.Background(), server)) + + var pvcList corev1.PersistentVolumeClaimList + require.NoError(t, fakeClient.List(context.Background(), &pvcList)) + + names := make([]string, 0, len(pvcList.Items)) + for i := range pvcList.Items { + names = append(names, pvcList.Items[i].Name) + } + + assert.ElementsMatch(t, []string{ + "data-test-server-klio-0", + "queue-test-server-klio-0", + "cachetier1-test-server-klio-0", + }, names) +} + +func TestDeleteOrphanCachePVCsIgnoresOtherServers(t *testing.T) { + otherPVC := newTestPVC("cachetier1-other-server-klio-0", "other-server", pvcTypeCacheTier1, "10Gi") + reconciler, fakeClient := newTestReconciler(otherPVC) + + server := newTestServerTier1("100Gi", "10Gi") + server.Spec.Tier1.Cache = nil + + require.NoError(t, reconciler.deleteOrphanCachePVCs(context.Background(), server)) + + var pvc corev1.PersistentVolumeClaim + assert.NoError(t, fakeClient.Get(context.Background(), client.ObjectKey{ + Name: "cachetier1-other-server-klio-0", Namespace: "default", + }, &pvc)) +} diff --git a/operator/internal/controller/server_reconciler.go b/operator/internal/controller/server_reconciler.go index 49d5731f..90a4c68e 100644 --- a/operator/internal/controller/server_reconciler.go +++ b/operator/internal/controller/server_reconciler.go @@ -79,6 +79,12 @@ const ( kopiaCacheTier1MountPath = "/cache_tier1" kopiaCacheTier2MountPath = "/cache_tier2" + // Cache locations inside the tier1 data volume, used by a tier that has no + // dedicated cache PVC. They are siblings of the repository (/data/base) and + // of the WAL archive (/data/wal). + fallbackCacheTier1Path = kopiaDataMountPath + "/cache_tier1" + fallbackCacheTier2Path = kopiaDataMountPath + "/cache_tier2" + fileSourceBasePath = "/files" tier1EncKeyFileVolName = "tier1-enc-key-file" tier1IdentityVolName = "tier1-identity" @@ -102,7 +108,14 @@ func (r *ServerReconciler) reconcile(ctx context.Context, server *kliov1alpha1.S return result, nil } - return r.reconcileStatefulSet(ctx, server) + result, err := r.reconcileStatefulSet(ctx, server) + if err != nil || !result.IsZero() { + return result, err + } + + // The StatefulSet now matches the spec, so a cache volume that is no longer + // requested is not mounted by any pod and its PVC can be reclaimed. + return ctrl.Result{}, r.deleteOrphanCachePVCs(ctx, server) } //nolint:cyclop @@ -367,6 +380,13 @@ func injectTier1VolumeClaimTemplates( }, Spec: server.Spec.Tier1.Data.PersistentVolumeClaimTemplate, }, + ) + + if server.Spec.Tier1.Cache == nil { + return + } + + ss.Spec.VolumeClaimTemplates = append(ss.Spec.VolumeClaimTemplates, corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "cachetier1", @@ -383,6 +403,10 @@ func injectTier2VolumeClaimTemplates( ss *appsv1.StatefulSet, server kliov1alpha1.Server, ) { + if server.Spec.Tier2.Cache == nil { + return + } + ss.Spec.VolumeClaimTemplates = append(ss.Spec.VolumeClaimTemplates, corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -597,11 +621,15 @@ func (r *ServerReconciler) buildVolumeMounts(server *kliov1alpha1.Server) []core Name: "data", MountPath: kopiaDataMountPath, }, - corev1.VolumeMount{ + ) + + if server.Spec.Tier1.Cache != nil { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "cachetier1", MountPath: kopiaCacheTier1MountPath, - }, - ) + }) + } + _, mount := buildFileSourceVolMount(tier1EncKeyFileVolName, server.Spec.Tier1.EncryptionKeyFile) volumeMounts = append(volumeMounts, mount) @@ -626,11 +654,15 @@ func (r *ServerReconciler) buildVolumeMounts(server *kliov1alpha1.Server) []core Name: "tier2", MountPath: "/tier2", }, - corev1.VolumeMount{ + ) + + if server.Spec.Tier2.Cache != nil { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "cachetier2", MountPath: kopiaCacheTier2MountPath, - }, - ) + }) + } + _, mount := buildFileSourceVolMount(tier2EncKeyFileVolName, server.Spec.Tier2.EncryptionKeyFile) volumeMounts = append(volumeMounts, mount) diff --git a/operator/internal/controller/server_statefulset_test.go b/operator/internal/controller/server_statefulset_test.go index 7856d305..a6bc9d2d 100644 --- a/operator/internal/controller/server_statefulset_test.go +++ b/operator/internal/controller/server_statefulset_test.go @@ -59,7 +59,7 @@ func newTestServerForStatefulSet() *kliov1alpha1.Server { Mode: kliov1alpha1.ModeStandard, Tier1: &kliov1alpha1.Tier1Configuration{ Data: kliov1alpha1.Data{PersistentVolumeClaimTemplate: newPVCSpec("10Gi")}, - Cache: kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("5Gi")}, + Cache: &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("5Gi")}, EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), IdentityFile: newTestFileSource("id-secret", "identity.txt"), }, @@ -215,3 +215,73 @@ func TestServerPodSecurityContext(t *testing.T) { assert.Nil(t, sc) }) } + +func volumeClaimTemplateNames(ss *appsv1.StatefulSet) []string { + names := make([]string, 0, len(ss.Spec.VolumeClaimTemplates)) + for i := range ss.Spec.VolumeClaimTemplates { + names = append(names, ss.Spec.VolumeClaimTemplates[i].Name) + } + + return names +} + +func volumeMountNames(mounts []corev1.VolumeMount) []string { + names := make([]string, 0, len(mounts)) + for i := range mounts { + names = append(names, mounts[i].Name) + } + + return names +} + +func newTestServerWithBothTiers(t *testing.T) *kliov1alpha1.Server { + t.Helper() + + server := newTestServerForStatefulSet() + server.Spec.Tier2 = &kliov1alpha1.Tier2Configuration{ + Cache: &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("5Gi")}, + S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + } + + return server +} + +func TestVolumeClaimTemplatesIncludeTheRequestedCaches(t *testing.T) { + server := newTestServerWithBothTiers(t) + + ss := &appsv1.StatefulSet{} + injectTier1VolumeClaimTemplates(ss, *server) + injectTier2VolumeClaimTemplates(ss, *server) + + assert.Equal(t, []string{"data", "cachetier1", "cachetier2"}, volumeClaimTemplateNames(ss)) +} + +func TestVolumeClaimTemplatesOmitTheCachesThatFallBackToData(t *testing.T) { + server := newTestServerWithBothTiers(t) + server.Spec.Tier1.Cache = nil + server.Spec.Tier2.Cache = nil + + ss := &appsv1.StatefulSet{} + injectTier1VolumeClaimTemplates(ss, *server) + injectTier2VolumeClaimTemplates(ss, *server) + + assert.Equal(t, []string{"data"}, volumeClaimTemplateNames(ss)) +} + +func TestVolumeMountsFollowTheCacheConfiguration(t *testing.T) { + reconciler := &ServerReconciler{} + server := newTestServerWithBothTiers(t) + + assert.Subset(t, volumeMountNames(reconciler.buildVolumeMounts(server)), + []string{"data", "cachetier1", "cachetier2"}) + + server.Spec.Tier1.Cache = nil + server.Spec.Tier2.Cache = nil + + mounts := volumeMountNames(reconciler.buildVolumeMounts(server)) + assert.Contains(t, mounts, "data") + assert.NotContains(t, mounts, "cachetier1") + assert.NotContains(t, mounts, "cachetier2") +} diff --git a/operator/pkg/config/server.go b/operator/pkg/config/server.go index 09b77188..5ee045bc 100644 --- a/operator/pkg/config/server.go +++ b/operator/pkg/config/server.go @@ -98,6 +98,11 @@ type Tier2Config struct { // CacheDirectory is the directory of the Kopia cache CacheDirectory string `mapstructure:"cache"` + // StaleCacheDirectory is a cache directory that is no longer in use and + // whose space must be reclaimed at startup. It is set when the cache moved + // to a different volume. Empty when there is nothing to reclaim. + StaleCacheDirectory string `mapstructure:"stale_cache"` + // S3 contains the configuration parameters for an S3-based tier 2 S3 S3Configuration `json:"s3" mapstructure:"s3"` } @@ -108,6 +113,11 @@ type BaseServerConfig struct { // CacheDirectory is the directory of the Kopia cache CacheDirectory string `mapstructure:"cache"` + // StaleCacheDirectory is a cache directory that is no longer in use and + // whose space must be reclaimed at startup. It is set when the cache moved + // to a different volume. Empty when there is nothing to reclaim. + StaleCacheDirectory string `mapstructure:"stale_cache"` + // RepositoryDirectory is the directory where the Kopia repository is stored. RepositoryDirectory string `mapstructure:"repository"` diff --git a/operator/test/e2e/server_reconfig_test.go b/operator/test/e2e/server_reconfig_test.go index 54d4cbd6..1c658635 100644 --- a/operator/test/e2e/server_reconfig_test.go +++ b/operator/test/e2e/server_reconfig_test.go @@ -34,6 +34,7 @@ import ( k8stypes "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/e2e-framework/klient/k8s/resources" "sigs.k8s.io/e2e-framework/klient/wait" + waitConditions "sigs.k8s.io/e2e-framework/klient/wait/conditions" "sigs.k8s.io/e2e-framework/pkg/envconf" "sigs.k8s.io/e2e-framework/pkg/types" @@ -138,6 +139,59 @@ func (s *serverReconfigScenario) Teardown( return ctx } +// waitForVolumeClaimTemplates waits until the StatefulSet exposes exactly the +// given VolumeClaimTemplates, i.e. the operator has finished recreating it. +func waitForVolumeClaimTemplates( + t *testing.T, + r *resources.Resources, + stsName, namespace string, + expected []string, +) { + t.Helper() + + want := slices.Sorted(slices.Values(expected)) + + var last []string + + err := wait.For(func(ctx context.Context) (bool, error) { + sts := &appsv1.StatefulSet{} + if err := r.Get(ctx, stsName, namespace, sts); err != nil { + return false, nil //nolint:nilerr // the StatefulSet is missing while it is recreated + } + + last = make([]string, 0, len(sts.Spec.VolumeClaimTemplates)) + for _, vct := range sts.Spec.VolumeClaimTemplates { + last = append(last, vct.Name) + } + slices.Sort(last) + + return slices.Equal(last, want), nil + }, wait.WithTimeout(5*time.Minute), wait.WithInterval(5*time.Second)) + + require.NoError(t, err, + "StatefulSet VolumeClaimTemplates never became %v, last seen %v", want, last) +} + +// waitForPVCDeleted waits until a PVC is gone. The deletion blocks on the +// pvc-protection finalizer until the pod that mounted it terminates. +func waitForPVCDeleted( + t *testing.T, + r *resources.Resources, + pvcName, namespace string, +) { + t.Helper() + + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: namespace}, + } + + require.NoError(t, wait.For( + waitConditions.New(r).ResourceDeleted(pvc), + wait.WithTimeout(5*time.Minute), + wait.WithInterval(5*time.Second), + ), "cache PVC %s was not reclaimed", pvcName) +} + // serverReconfigFeature implements the Feature interface for server tier reconfiguration testing. type serverReconfigFeature struct { name string @@ -258,6 +312,68 @@ func (f *serverReconfigFeature) Run() types.StepFunc { t.Logf("PVC %s retained with original UID %s", pvcName, pvc.UID) } + // Drop both dedicated cache volumes: the caches fall back to the data + // volume, the StatefulSet is recreated once more and the cache PVCs are + // reclaimed. + require.NoError(t, + r.Get(ctx, server.Name, f.scenario.namespace.Name, currentServer), + "failed to get current Server", + ) + originalTier1Cache := currentServer.Spec.Tier1.Cache + currentServer.Spec.Tier1.Cache = nil + currentServer.Spec.Tier2.Cache = nil + require.NoError(t, r.Update(ctx, currentServer), "failed to remove the cache volumes") + t.Log("Server updated without dedicated cache volumes") + + waitForVolumeClaimTemplates(t, r, stsName, f.scenario.namespace.Name, + []string{"data", "queue"}) + + err = wait.For( + conditions.KlioServerIsReady(r, server), + wait.WithTimeout(10*time.Minute), + wait.WithInterval(10*time.Second), + ) + require.NoError(t, err, "server Pod not ready after the cache volumes were removed") + + waitForPVCDeleted(t, r, "cachetier1-"+stsName+"-0", f.scenario.namespace.Name) + waitForPVCDeleted(t, r, cachetier2PVCName, f.scenario.namespace.Name) + + // The volumes holding data must survive the migration untouched. + for _, pvcName := range []string{"data-" + stsName + "-0", "queue-" + stsName + "-0"} { + pvc := &corev1.PersistentVolumeClaim{} + require.NoError(t, + r.Get(ctx, pvcName, f.scenario.namespace.Name, pvc), + "PVC %s no longer exists after the cache migration", pvcName, + ) + require.Equal(t, originalPVCUIDs[pvcName], pvc.UID, + "PVC %s was recreated during the cache migration", pvcName) + } + + // Give tier1 its dedicated cache volume back. + require.NoError(t, + r.Get(ctx, server.Name, f.scenario.namespace.Name, currentServer), + "failed to get current Server", + ) + currentServer.Spec.Tier1.Cache = originalTier1Cache + require.NoError(t, r.Update(ctx, currentServer), "failed to restore the tier1 cache volume") + t.Log("Server updated with the tier1 cache volume restored") + + waitForVolumeClaimTemplates(t, r, stsName, f.scenario.namespace.Name, + []string{"data", "cachetier1", "queue"}) + + err = wait.For( + conditions.KlioServerIsReady(r, server), + wait.WithTimeout(10*time.Minute), + wait.WithInterval(10*time.Second), + ) + require.NoError(t, err, "server Pod not ready after the tier1 cache volume was restored") + + require.NoError(t, + r.Get(ctx, "cachetier1-"+stsName+"-0", f.scenario.namespace.Name, + &corev1.PersistentVolumeClaim{}), + "cachetier1 PVC was not recreated", + ) + t.Log("Server tier reconfiguration test passed: all verifications succeeded") return ctx diff --git a/operator/test/klio/features/pvc_resize.go b/operator/test/klio/features/pvc_resize.go index 320b4a93..4eea675b 100644 --- a/operator/test/klio/features/pvc_resize.go +++ b/operator/test/klio/features/pvc_resize.go @@ -179,7 +179,7 @@ func (f *PVCResizeFeature) updateServerPVCSizes( t.Logf("Updating data PVC size to %s", f.newDataSize.String()) } - if server.Spec.Tier1 != nil && !f.newCacheSize.IsZero() { + if server.Spec.Tier1 != nil && server.Spec.Tier1.Cache != nil && !f.newCacheSize.IsZero() { server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage] = f.newCacheSize expectedSizes["cachetier1"] = f.newCacheSize t.Logf("Updating cachetier1 PVC size to %s", f.newCacheSize.String()) diff --git a/operator/test/utils/templates/klio/klio.go b/operator/test/utils/templates/klio/klio.go index 3612e864..8169773b 100644 --- a/operator/test/utils/templates/klio/klio.go +++ b/operator/test/utils/templates/klio/klio.go @@ -86,7 +86,7 @@ func BuildTier2Configuration( } return kliov1alpha1.Tier2Configuration{ - Cache: kliov1alpha1.Cache{ + Cache: &kliov1alpha1.Cache{ PersistentVolumeClaimTemplate: corev1.PersistentVolumeClaimSpec{ StorageClassName: sc, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOncePod}, @@ -184,7 +184,7 @@ func GetServerObject( server := newBaseServer(name, namespace, opts) server.Spec.Mode = kliov1alpha1.ModeStandard server.Spec.Tier1 = &kliov1alpha1.Tier1Configuration{ - Cache: kliov1alpha1.Cache{ + Cache: &kliov1alpha1.Cache{ PersistentVolumeClaimTemplate: corev1.PersistentVolumeClaimSpec{ StorageClassName: sc, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOncePod}, From c2624747a68d3276bfbbfa12187ea7d7c1e9d8a5 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Mon, 31 Aug 2026 16:55:50 +0200 Subject: [PATCH 3/6] docs: document the optional cache volumes Explain when to dedicate a PVC to a tier's cache and what happens when you do not, and drop the cache stanza from the quickstart, whose data PVC now has to accommodate the cache. Signed-off-by: Armando Ruocco --- .../web/docs/developer/running-e2e-tests.md | 3 +- documentation/web/docs/user/api/_klio_api.md | 4 +-- documentation/web/docs/user/klio_server.md | 28 +++++++++++++++++-- .../web/docs/user/managing_storage.md | 6 ++++ documentation/web/docs/user/quickstart.md | 15 +++------- 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/documentation/web/docs/developer/running-e2e-tests.md b/documentation/web/docs/developer/running-e2e-tests.md index 5c173203..55f1ab4e 100644 --- a/documentation/web/docs/developer/running-e2e-tests.md +++ b/documentation/web/docs/developer/running-e2e-tests.md @@ -128,7 +128,8 @@ The E2E tests are located in `operator/test/e2e/` and include: driven by backup completion rather than a client command (`WALRetentionQueueAwareness`) - **`server_reconfig_test.go`** - Adding tier2 storage to an existing - tier1+queue server (`ServerTierReconfiguration`) + tier1+queue server, then removing and restoring the dedicated cache + volumes (`ServerTierReconfiguration`) - **`pluginconfiguration_update_test.go`** - PluginConfiguration updates and sidecar restart behavior (`PluginConfigurationUpdate`) - **`pvc_resize_test.go`** - PVC resize for data, cache, and queue diff --git a/documentation/web/docs/user/api/_klio_api.md b/documentation/web/docs/user/api/_klio_api.md index 751b54d1..adadcd05 100644 --- a/documentation/web/docs/user/api/_klio_api.md +++ b/documentation/web/docs/user/api/_klio_api.md @@ -362,7 +362,7 @@ _Appears in:_ | Field | Description | Required | Default | Validation | | --- | --- | --- | --- | --- | -| `cache` _[Cache](#cache)_ | Cache is the configuration of the PVC that should be
used for the cache. | True | | | +| `cache` _[Cache](#cache)_ | Cache is the configuration of the PVC that should be
used for the cache. When omitted, the Kopia cache is stored in a
directory inside the tier1 data volume. | | | Optional: \{\}
| | `data` _[Data](#data)_ | Data is the configuration of the PVC that should be used
for the base backups. | True | | | | `encryptionKeyFile` _[FileSource](#filesource)_ | EncryptionKeyFile specifies the Age-encrypted encryption key file. | True | | ExactlyOneOf: [fileReference]
| | `identityFile` _[FileSource](#filesource)_ | IdentityFile specifies the Age identity (private key) file used to
decrypt the encryption key. | True | | ExactlyOneOf: [fileReference]
| @@ -397,7 +397,7 @@ _Appears in:_ | Field | Description | Required | Default | Validation | | --- | --- | --- | --- | --- | -| `cache` _[Cache](#cache)_ | Cache is the configuration of the PVC that should be
used for the cache. | True | | | +| `cache` _[Cache](#cache)_ | Cache is the configuration of the PVC that should be
used for the cache. When omitted, the Kopia cache is stored in a
directory inside the tier1 data volume, and is therefore required
when tier1 is not configured. | | | Optional: \{\}
| | `s3` _[S3Configuration](#s3configuration)_ | S3 contains the configuration parameters for an S3-based tier 2. | True | | | | `encryptionKeyFile` _[FileSource](#filesource)_ | EncryptionKeyFile specifies the Age-encrypted encryption key file. | True | | ExactlyOneOf: [fileReference]
| | `identityFile` _[FileSource](#filesource)_ | IdentityFile specifies the Age identity (private key) file used to
decrypt the encryption key. | True | | ExactlyOneOf: [fileReference]
| diff --git a/documentation/web/docs/user/klio_server.md b/documentation/web/docs/user/klio_server.md index 20717323..ddc0843c 100644 --- a/documentation/web/docs/user/klio_server.md +++ b/documentation/web/docs/user/klio_server.md @@ -74,9 +74,20 @@ The following factors should be considered when defining the PVC size: ### Cache PVCs -The cache PVCs (one for Tier 1 and Tier 2 each) are used by Kopia for its -[caching operations](https://kopia.io/docs/advanced/caching/). -They are used to speed up snapshot operations. +Kopia keeps a [cache](https://kopia.io/docs/advanced/caching/) to speed up +snapshot operations. Each tier has its own cache, and the optional +`tier1.cache` and `tier2.cache` stanzas dedicate a PVC to it. + +When a tier has no `cache` stanza, its cache is stored inside the Tier 1 data +volume, under `/data/cache_tier1` and `/data/cache_tier2` respectively. This is +the simplest configuration and the one to start from: size the data PVC so that +it accommodates the cache as well. Dedicate a cache PVC when you want the cache +on a different storage class, or when you want its growth to be unable to eat +into the space of your backups. + +Since the fallback location lives in the Tier 1 data volume, `tier2.cache` is +required on a [read-only server](#read-only-mode), where Tier 1 is not +configured. :::warning Klio is currently limited to use the default cache size when creating a Kopia @@ -86,6 +97,17 @@ so users should have a space buffer to account for this additional space. This limitation will be removed in a future version. ::: +#### Moving a cache between volumes + +The `cache` stanza can be added to, or removed from, a running server. The +operator recreates the StatefulSet, which restarts the server pod, and Kopia +rebuilds the cache in its new location on demand: no backup or WAL file is +affected, only the first operations after the restart are slower. + +The old location is reclaimed without manual intervention. The PVC of a cache +that is no longer dedicated is deleted once the pod no longer mounts it, and a +cache left behind in the data volume is removed when the server starts. + ### Queue PVC The queue PVC is required when Tier 1 is configured. It stores the NATS diff --git a/documentation/web/docs/user/managing_storage.md b/documentation/web/docs/user/managing_storage.md index 66f3cf3b..bcea469a 100644 --- a/documentation/web/docs/user/managing_storage.md +++ b/documentation/web/docs/user/managing_storage.md @@ -126,6 +126,12 @@ Apply the updated Server resource: kubectl apply -f klio-server.yaml ``` +:::note +`cache` is optional. When a tier has no dedicated cache volume its cache lives +inside the data PVC, so expanding `tier1.data` covers it too. See +[Cache PVCs](klio_server.md#cache-pvcs). +::: + #### What Happens During Resize When you update the Server spec with larger PVC sizes, the following diff --git a/documentation/web/docs/user/quickstart.md b/documentation/web/docs/user/quickstart.md index a8e7eaa3..0260c4d6 100644 --- a/documentation/web/docs/user/quickstart.md +++ b/documentation/web/docs/user/quickstart.md @@ -256,23 +256,16 @@ spec: caSecretName: klio-server-ca tier1: - # Kopia cache. The default Kopia cache is 5 GB of content plus - # 5 GB of metadata, so leave some headroom. - cache: - pvcTemplate: - accessModes: - - ReadWriteOnce - resources: - requests: - storage: 10Gi - # Base backups and the WAL archive + # Base backups, the WAL archive and, with no dedicated cache volume, the + # Kopia cache. The default Kopia cache is 5 GB of content plus 5 GB of + # metadata, so leave some headroom on top of your backup size. data: pvcTemplate: accessModes: - ReadWriteOnce resources: requests: - storage: 20Gi + storage: 30Gi encryptionKeyFile: fileReference: volume: From b798a1be26286489556caba302790c55ca783e28 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Mon, 31 Aug 2026 17:16:20 +0200 Subject: [PATCH 4/6] refactor(operator): build the server volume claim templates in one place Every volume the server claims was built by its own injector, each repeating the same labelled PersistentVolumeClaim literal, and the cache work added a third guard to that pattern. A single volumeClaimTemplates function now returns them all, in the order the StatefulSet had before, and the same collapsing applies to the cache paths in the environment builder and to the desired PVC sizes. The StatefulSet spec is unchanged, hash included, so no server is restarted by this. Signed-off-by: Armando Ruocco --- core/cmd/server/initialize_test.go | 4 - .../controller/server_cachevalidation_test.go | 66 ++----- .../internal/controller/server_envbuilder.go | 40 ++-- .../controller/server_envbuilder_test.go | 87 ++++---- .../internal/controller/server_pvc_resize.go | 72 ++----- .../internal/controller/server_reconciler.go | 186 +++++++++--------- .../controller/server_statefulset_test.go | 47 +++-- 7 files changed, 206 insertions(+), 296 deletions(-) diff --git a/core/cmd/server/initialize_test.go b/core/cmd/server/initialize_test.go index f0924326..59952b99 100644 --- a/core/cmd/server/initialize_test.go +++ b/core/cmd/server/initialize_test.go @@ -67,7 +67,3 @@ func TestReclaimStaleCacheIsANoOpWhenUnset(t *testing.T) { assert.DirExists(t, inUse) } - -func TestReclaimStaleCacheRefusesRelativePaths(t *testing.T) { - require.Error(t, reclaimStaleCache(context.Background(), "cache", "/data/cache")) -} diff --git a/operator/internal/controller/server_cachevalidation_test.go b/operator/internal/controller/server_cachevalidation_test.go index df3f8bb1..68c0db8e 100644 --- a/operator/internal/controller/server_cachevalidation_test.go +++ b/operator/internal/controller/server_cachevalidation_test.go @@ -23,64 +23,30 @@ import ( "context" "fmt" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kliov1alpha1 "github.com/cloudnative-pg/klio/operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -func cacheTestPVCTemplate(size string) corev1.PersistentVolumeClaimSpec { - return corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse(size), - }, - }, - } -} - -func cacheTestFileSource(secretName, path string) kliov1alpha1.FileSource { - return kliov1alpha1.FileSource{ - FileReference: &kliov1alpha1.FileReference{ - Volume: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{SecretName: secretName}, - }, - Path: path, - }, - } -} - +// cacheTestServer returns a Server accepted by the API server, with no +// dedicated cache volume on tier1. Each test needs its own name because the +// objects live in the same namespace for the whole suite. func cacheTestServer(name string) *kliov1alpha1.Server { - return &kliov1alpha1.Server{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, - Spec: kliov1alpha1.ServerSpec{ - ImageConfiguration: kliov1alpha1.ImageConfiguration{Image: "klio:test"}, - TLSConfiguration: kliov1alpha1.TLSConfiguration{ - TLSSecretName: "tls-secret", - ClientCASecretName: "ca-secret", - }, - Mode: kliov1alpha1.ModeStandard, - Tier1: &kliov1alpha1.Tier1Configuration{ - Data: kliov1alpha1.Data{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("1Gi")}, - EncryptionKeyFile: cacheTestFileSource("enc-secret", "encryption-key.age"), - IdentityFile: cacheTestFileSource("id-secret", "identity.txt"), - }, - Queue: &kliov1alpha1.Queue{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("1Gi")}, - }, - } + server := newTestServerForStatefulSet() + server.Name = name + server.UID = "" + server.Spec.Tier1.Cache = nil + + return server } func cacheTestTier2(cache *kliov1alpha1.Cache) *kliov1alpha1.Tier2Configuration { return &kliov1alpha1.Tier2Configuration{ Cache: cache, S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, - EncryptionKeyFile: cacheTestFileSource("enc-secret", "encryption-key.age"), - IdentityFile: cacheTestFileSource("id-secret", "identity.txt"), + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), } } @@ -132,7 +98,7 @@ var _ = Describe("Server cache validation", func() { server.Spec.Tier1 = nil server.Spec.Queue = nil server.Spec.Tier2 = cacheTestTier2(&kliov1alpha1.Cache{ - PersistentVolumeClaimTemplate: cacheTestPVCTemplate("1Gi"), + PersistentVolumeClaimTemplate: newPVCSpec("1Gi"), }) Expect(create(server)).To(Succeed()) @@ -151,18 +117,18 @@ var _ = Describe("Server cache validation", func() { server.Spec.Tier2.Cache = after Expect(k8sClient.Update(ctx, server)).To(Succeed()) }, - Entry("adding", nil, &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("2Gi")}), - Entry("removing", &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: cacheTestPVCTemplate("2Gi")}, nil), + Entry("adding", nil, &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("2Gi")}), + Entry("removing", &kliov1alpha1.Cache{PersistentVolumeClaimTemplate: newPVCSpec("2Gi")}, nil), ) It("still refuses to shrink a cache volume that stays configured", func() { server := cacheTestServer("cache-shrink") server.Spec.Tier1.Cache = &kliov1alpha1.Cache{ - PersistentVolumeClaimTemplate: cacheTestPVCTemplate("2Gi"), + PersistentVolumeClaimTemplate: newPVCSpec("2Gi"), } Expect(create(server)).To(Succeed()) - server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate = cacheTestPVCTemplate("1Gi") + server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate = newPVCSpec("1Gi") Expect(k8sClient.Update(ctx, server)).To(MatchError( ContainSubstring("tier1.cache PVC size cannot be decreased"))) }) diff --git a/operator/internal/controller/server_envbuilder.go b/operator/internal/controller/server_envbuilder.go index 83037f2d..2b61a319 100644 --- a/operator/internal/controller/server_envbuilder.go +++ b/operator/internal/controller/server_envbuilder.go @@ -30,27 +30,19 @@ import ( const kopiaCacheSubdirectory = "kopia-cache" -// cacheDirectory returns the Kopia cache directory for a tier: the dedicated -// cache volume when one is configured, the fallback location inside the tier1 -// data volume otherwise. The cache always lives one level below the mount -// point so that wiping it never touches the volume root. -func cacheDirectory(cache *kliov1alpha1.Cache, mountPath, fallbackPath string) string { +// cachePaths returns the Kopia cache directory a tier uses and the one it may +// have left behind. A tier with a dedicated cache volume keeps its cache there, +// so a cache sitting in the tier1 data volume is stale; a tier without one +// falls back to the data volume and has nothing to reclaim. The cache always +// lives one level below the mount point so that wiping it never touches the +// volume root. +func cachePaths(cache *kliov1alpha1.Cache, mountPath, fallbackPath string) (string, string) { + fallback := path.Join(fallbackPath, kopiaCacheSubdirectory) if cache == nil { - return path.Join(fallbackPath, kopiaCacheSubdirectory) + return fallback, "" } - return path.Join(mountPath, kopiaCacheSubdirectory) -} - -// staleCacheDirectory returns the cache location a tier is configured *not* to -// use, so that the server can reclaim the space of a cache left behind by a -// migration to a dedicated cache volume. -func staleCacheDirectory(cache *kliov1alpha1.Cache, fallbackPath string) string { - if cache == nil { - return "" - } - - return path.Join(fallbackPath, kopiaCacheSubdirectory) + return path.Join(mountPath, kopiaCacheSubdirectory), fallback } type envBuilder struct { @@ -133,15 +125,17 @@ func (e *envBuilder) getCoreEnvVars() []corev1.EnvVar { } if e.tier1 != nil { + tier1Cache, tier1StaleCache := cachePaths(e.tier1.Cache, kopiaCacheTier1MountPath, fallbackCacheTier1Path) + tier1Envs := make([]corev1.EnvVar, 0, 8) //nolint:mnd tier1Envs = append(tier1Envs, corev1.EnvVar{ Name: "TIER1_BASE_CACHE", - Value: cacheDirectory(e.tier1.Cache, kopiaCacheTier1MountPath, fallbackCacheTier1Path), + Value: tier1Cache, }, corev1.EnvVar{ Name: "TIER1_BASE_STALE_CACHE", - Value: staleCacheDirectory(e.tier1.Cache, fallbackCacheTier1Path), + Value: tier1StaleCache, }, corev1.EnvVar{ Name: "TIER1_BASE_REPOSITORY", @@ -193,6 +187,8 @@ func (e *envBuilder) getTier2EnvVars() []corev1.EnvVar { return nil } + tier2Cache, tier2StaleCache := cachePaths(e.tier2.Cache, kopiaCacheTier2MountPath, fallbackCacheTier2Path) + result := []corev1.EnvVar{ { Name: "TIER2_S3_ENABLED", @@ -204,11 +200,11 @@ func (e *envBuilder) getTier2EnvVars() []corev1.EnvVar { }, { Name: "TIER2_CACHE", - Value: cacheDirectory(e.tier2.Cache, kopiaCacheTier2MountPath, fallbackCacheTier2Path), + Value: tier2Cache, }, { Name: "TIER2_STALE_CACHE", - Value: staleCacheDirectory(e.tier2.Cache, fallbackCacheTier2Path), + Value: tier2StaleCache, }, { Name: "TIER2_BASE_LISTEN_ADDRESS", diff --git a/operator/internal/controller/server_envbuilder_test.go b/operator/internal/controller/server_envbuilder_test.go index 3bed53ae..c66497c3 100644 --- a/operator/internal/controller/server_envbuilder_test.go +++ b/operator/internal/controller/server_envbuilder_test.go @@ -334,58 +334,51 @@ func TestBuildIdentityVolMountProjected(t *testing.T) { assert.True(t, mount.ReadOnly) } -func TestCacheEnvVarsUseTheDedicatedVolumeWhenConfigured(t *testing.T) { - builder := &envBuilder{ - tier1: &kliov1alpha1.Tier1Configuration{ - Cache: &kliov1alpha1.Cache{}, - EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), - IdentityFile: newTestFileSource("id-secret", "identity.txt"), +func TestCacheEnvVars(t *testing.T) { + tests := map[string]struct { + dedicated bool + // The cache in use, and the location to reclaim because the cache + // moved away from it. + tier1, tier1Stale string + tier2, tier2Stale string + }{ + "dedicated volumes": { + dedicated: true, + tier1: "/cache_tier1/kopia-cache", + tier1Stale: "/data/cache_tier1/kopia-cache", + tier2: "/cache_tier2/kopia-cache", + tier2Stale: "/data/cache_tier2/kopia-cache", }, - tier2: &kliov1alpha1.Tier2Configuration{ - Cache: &kliov1alpha1.Cache{}, - S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, - EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), - IdentityFile: newTestFileSource("id-secret", "identity.txt"), + "fallback to the data volume": { + tier1: "/data/cache_tier1/kopia-cache", + tier2: "/data/cache_tier2/kopia-cache", }, } - envVars := append(builder.getCoreEnvVars(), builder.getTier2EnvVars()...) - - assert.Equal(t, "/cache_tier1/kopia-cache", findEnvVar(envVars, "TIER1_BASE_CACHE").Value) - assert.Equal(t, "/cache_tier2/kopia-cache", findEnvVar(envVars, "TIER2_CACHE").Value) + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + builder := &envBuilder{ + tier1: &kliov1alpha1.Tier1Configuration{ + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + }, + tier2: &kliov1alpha1.Tier2Configuration{ + S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, + EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("id-secret", "identity.txt"), + }, + } + if tc.dedicated { + builder.tier1.Cache = &kliov1alpha1.Cache{} + builder.tier2.Cache = &kliov1alpha1.Cache{} + } - // The fallback locations are reported as stale so that a cache left there by - // a previous configuration is reclaimed. - assert.Equal(t, "/data/cache_tier1/kopia-cache", findEnvVar(envVars, "TIER1_BASE_STALE_CACHE").Value) - assert.Equal(t, "/data/cache_tier2/kopia-cache", findEnvVar(envVars, "TIER2_STALE_CACHE").Value) -} + envVars := append(builder.getCoreEnvVars(), builder.getTier2EnvVars()...) -func TestCacheEnvVarsFallBackToTheDataVolume(t *testing.T) { - builder := &envBuilder{ - tier1: &kliov1alpha1.Tier1Configuration{ - EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), - IdentityFile: newTestFileSource("id-secret", "identity.txt"), - }, - tier2: &kliov1alpha1.Tier2Configuration{ - S3: &kliov1alpha1.S3Configuration{BucketName: "test-bucket"}, - EncryptionKeyFile: newTestFileSource("enc-secret", "encryption-key.age"), - IdentityFile: newTestFileSource("id-secret", "identity.txt"), - }, + assert.Equal(t, tc.tier1, findEnvVar(envVars, "TIER1_BASE_CACHE").Value) + assert.Equal(t, tc.tier1Stale, findEnvVar(envVars, "TIER1_BASE_STALE_CACHE").Value) + assert.Equal(t, tc.tier2, findEnvVar(envVars, "TIER2_CACHE").Value) + assert.Equal(t, tc.tier2Stale, findEnvVar(envVars, "TIER2_STALE_CACHE").Value) + }) } - - envVars := append(builder.getCoreEnvVars(), builder.getTier2EnvVars()...) - - assert.Equal(t, "/data/cache_tier1/kopia-cache", findEnvVar(envVars, "TIER1_BASE_CACHE").Value) - assert.Equal(t, "/data/cache_tier2/kopia-cache", findEnvVar(envVars, "TIER2_CACHE").Value) - - // Nothing to reclaim: the fallback locations are the ones in use. - assert.Empty(t, findEnvVar(envVars, "TIER1_BASE_STALE_CACHE").Value) - assert.Empty(t, findEnvVar(envVars, "TIER2_STALE_CACHE").Value) -} - -// The two tiers must never share a cache directory: the server refuses to start -// when they do. -func TestFallbackCachePathsDiffer(t *testing.T) { - assert.NotEqual(t, fallbackCacheTier1Path, fallbackCacheTier2Path) - assert.NotEqual(t, fallbackCacheTier1Path, kopiaDataMountPath) } diff --git a/operator/internal/controller/server_pvc_resize.go b/operator/internal/controller/server_pvc_resize.go index c25e4f99..43061564 100644 --- a/operator/internal/controller/server_pvc_resize.go +++ b/operator/internal/controller/server_pvc_resize.go @@ -150,74 +150,34 @@ func (r *ServerReconciler) expandPVC( return nil } +// addPVCSize records the storage a PVC template requests, if it requests any. +func addPVCSize(sizes map[string]resource.Quantity, pvcType string, spec corev1.PersistentVolumeClaimSpec) { + if size, ok := spec.Resources.Requests[corev1.ResourceStorage]; ok { + sizes[pvcType] = size + } +} + // buildDesiredPVCSizes returns a map of PVC type labels to their desired sizes. func (r *ServerReconciler) buildDesiredPVCSizes(server *kliov1alpha1.Server) map[string]resource.Quantity { sizes := make(map[string]resource.Quantity) - if server.Spec.Tier1 != nil { - if size, ok := server.Spec.Tier1.Data.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeData] = size - } - if server.Spec.Tier1.Cache != nil { - if size, ok := server.Spec.Tier1.Cache. - PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeCacheTier1] = size - } - } - } + if tier1 := server.Spec.Tier1; tier1 != nil { + addPVCSize(sizes, pvcTypeData, tier1.Data.PersistentVolumeClaimTemplate) - if server.Spec.Tier2 != nil && server.Spec.Tier2.Cache != nil { - if size, ok := server.Spec.Tier2.Cache. - PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeCacheTier2] = size + if tier1.Cache != nil { + addPVCSize(sizes, pvcTypeCacheTier1, tier1.Cache.PersistentVolumeClaimTemplate) } } - if server.Spec.Queue != nil { - if size, ok := server.Spec.Queue.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeQueue] = size - } - } - - return sizes -} - -// deleteOrphanCachePVCs removes the cache PVCs of tiers that no longer request a -// dedicated cache volume. Only cache PVCs are reclaimed: they hold no backup -// data, and Kopia rebuilds the cache on demand. -func (r *ServerReconciler) deleteOrphanCachePVCs(ctx context.Context, server *kliov1alpha1.Server) error { - contextLogger := logf.FromContext(ctx) - - orphaned := map[string]bool{ - pvcTypeCacheTier1: server.Spec.Tier1 == nil || server.Spec.Tier1.Cache == nil, - pvcTypeCacheTier2: server.Spec.Tier2 == nil || server.Spec.Tier2.Cache == nil, - } - - var pvcList corev1.PersistentVolumeClaimList - if err := r.List(ctx, &pvcList, - client.InNamespace(server.Namespace), - client.MatchingLabels{klioServerLabel: server.Name}, - ); err != nil { - return fmt.Errorf("failed to list PVCs: %w", err) + if tier2 := server.Spec.Tier2; tier2 != nil && tier2.Cache != nil { + addPVCSize(sizes, pvcTypeCacheTier2, tier2.Cache.PersistentVolumeClaimTemplate) } - for i := range pvcList.Items { - pvc := &pvcList.Items[i] - if !orphaned[pvc.Labels[pvcTypeLabel]] { - continue - } - - contextLogger.Info("Deleting cache PVC of a tier that no longer requests one", "pvc", pvc.Name) - - if err := r.Delete(ctx, pvc); err != nil && !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to delete orphan cache PVC %s: %w", pvc.Name, err) - } - - r.Recorder.Eventf(server, nil, corev1.EventTypeNormal, "CachePVCDeleted", - "DeleteOrphanCachePVC", "Cache PVC %s deleted: the tier has no dedicated cache volume", pvc.Name) + if queue := server.Spec.Queue; queue != nil { + addPVCSize(sizes, pvcTypeQueue, queue.PersistentVolumeClaimTemplate) } - return nil + return sizes } // isVolumeExpansionError checks if the error indicates the StorageClass doesn't support volume expansion. diff --git a/operator/internal/controller/server_reconciler.go b/operator/internal/controller/server_reconciler.go index 90a4c68e..9a5a23f8 100644 --- a/operator/internal/controller/server_reconciler.go +++ b/operator/internal/controller/server_reconciler.go @@ -118,6 +118,44 @@ func (r *ServerReconciler) reconcile(ctx context.Context, server *kliov1alpha1.S return ctrl.Result{}, r.deleteOrphanCachePVCs(ctx, server) } +// deleteOrphanCachePVCs removes the cache PVCs of tiers that no longer request a +// dedicated cache volume. Only cache PVCs are reclaimed: they hold no backup +// data, and Kopia rebuilds the cache on demand. +func (r *ServerReconciler) deleteOrphanCachePVCs(ctx context.Context, server *kliov1alpha1.Server) error { + contextLogger := logf.FromContext(ctx) + + orphaned := map[string]bool{ + pvcTypeCacheTier1: server.Spec.Tier1 == nil || server.Spec.Tier1.Cache == nil, + pvcTypeCacheTier2: server.Spec.Tier2 == nil || server.Spec.Tier2.Cache == nil, + } + + var pvcList corev1.PersistentVolumeClaimList + if err := r.List(ctx, &pvcList, + client.InNamespace(server.Namespace), + client.MatchingLabels{klioServerLabel: server.Name}, + ); err != nil { + return fmt.Errorf("failed to list PVCs: %w", err) + } + + for i := range pvcList.Items { + pvc := &pvcList.Items[i] + if !orphaned[pvc.Labels[pvcTypeLabel]] { + continue + } + + contextLogger.Info("Deleting cache PVC of a tier that no longer requests one", "pvc", pvc.Name) + + if err := r.Delete(ctx, pvc); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete orphan cache PVC %s: %w", pvc.Name, err) + } + + r.Recorder.Eventf(server, nil, corev1.EventTypeNormal, "CachePVCDeleted", + "DeleteOrphanCachePVC", "Cache PVC %s deleted: the tier has no dedicated cache volume", pvc.Name) + } + + return nil +} + //nolint:cyclop func (r *ServerReconciler) reconcileStatefulSet( ctx context.Context, server *kliov1alpha1.Server, @@ -205,18 +243,7 @@ func (r *ServerReconciler) reconcileStatefulSet( Status: appsv1.StatefulSetStatus{}, } - if server.Spec.Tier1 != nil { - injectTier1VolumeClaimTemplates(expected, *server) - } - - if server.Spec.Queue != nil { - injectQueueConfiguration(expected, *server) - } - - // Add Tier2 containers if the server has Tier 2 configuration - if server.Spec.Tier2 != nil { - injectTier2VolumeClaimTemplates(expected, *server) - } + expected.Spec.VolumeClaimTemplates = volumeClaimTemplates(*server) if server.Spec.Template != nil { merged, err := podtemplate.Merge(&expected.Spec.Template, server.Spec.Template.ToCoreV1()) @@ -320,19 +347,6 @@ func (r *ServerReconciler) reconcileStatefulSet( return ctrl.Result{}, nil } -func injectQueueConfiguration(expected *appsv1.StatefulSet, server kliov1alpha1.Server) { - expected.Spec.VolumeClaimTemplates = append(expected.Spec.VolumeClaimTemplates, corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "queue", - Labels: map[string]string{ - klioServerLabel: server.Name, - pvcTypeLabel: pvcTypeQueue, - }, - }, - Spec: server.Spec.Queue.PersistentVolumeClaimTemplate, - }) -} - func enablePProf(containers []corev1.Container) { for i := range containers { // No pprof on NATS, there's no such option @@ -365,59 +379,54 @@ func (r *ServerReconciler) serverPodSecurityContext() *corev1.PodSecurityContext } } -func injectTier1VolumeClaimTemplates( - ss *appsv1.StatefulSet, +// serverPVC builds a VolumeClaimTemplate labelled so that the reconciler can +// find the PVCs the StatefulSet generated from it. +func serverPVC( server kliov1alpha1.Server, -) { - ss.Spec.VolumeClaimTemplates = append(ss.Spec.VolumeClaimTemplates, - corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "data", - Labels: map[string]string{ - klioServerLabel: server.Name, - pvcTypeLabel: pvcTypeData, - }, + name string, + pvcType string, + spec corev1.PersistentVolumeClaimSpec, +) corev1.PersistentVolumeClaim { + return corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + klioServerLabel: server.Name, + pvcTypeLabel: pvcType, }, - Spec: server.Spec.Tier1.Data.PersistentVolumeClaimTemplate, }, - ) + Spec: spec, + } +} + +// volumeClaimTemplates returns the volumes the server needs to claim. A tier +// with no cache stanza claims no cache volume: its cache lives in the tier1 +// data volume. The order is part of the StatefulSet spec, so it must stay +// stable to avoid pointless recreations. +func volumeClaimTemplates(server kliov1alpha1.Server) []corev1.PersistentVolumeClaim { + var templates []corev1.PersistentVolumeClaim - if server.Spec.Tier1.Cache == nil { - return + if tier1 := server.Spec.Tier1; tier1 != nil { + templates = append(templates, + serverPVC(server, "data", pvcTypeData, tier1.Data.PersistentVolumeClaimTemplate)) + + if tier1.Cache != nil { + templates = append(templates, + serverPVC(server, "cachetier1", pvcTypeCacheTier1, tier1.Cache.PersistentVolumeClaimTemplate)) + } } - ss.Spec.VolumeClaimTemplates = append(ss.Spec.VolumeClaimTemplates, - corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cachetier1", - Labels: map[string]string{ - klioServerLabel: server.Name, - pvcTypeLabel: pvcTypeCacheTier1, - }, - }, - Spec: server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate, - }) -} + if queue := server.Spec.Queue; queue != nil { + templates = append(templates, + serverPVC(server, "queue", pvcTypeQueue, queue.PersistentVolumeClaimTemplate)) + } -func injectTier2VolumeClaimTemplates( - ss *appsv1.StatefulSet, - server kliov1alpha1.Server, -) { - if server.Spec.Tier2.Cache == nil { - return + if tier2 := server.Spec.Tier2; tier2 != nil && tier2.Cache != nil { + templates = append(templates, + serverPVC(server, "cachetier2", pvcTypeCacheTier2, tier2.Cache.PersistentVolumeClaimTemplate)) } - ss.Spec.VolumeClaimTemplates = append(ss.Spec.VolumeClaimTemplates, - corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cachetier2", - Labels: map[string]string{ - klioServerLabel: server.Name, - pvcTypeLabel: pvcTypeCacheTier2, - }, - }, - Spec: server.Spec.Tier2.Cache.PersistentVolumeClaimTemplate, - }) + return templates } func (r *ServerReconciler) reconcileService(ctx context.Context, server *kliov1alpha1.Server) error { @@ -615,19 +624,12 @@ func (r *ServerReconciler) buildVolumeMounts(server *kliov1alpha1.Server) []core } if server.Spec.Tier1 != nil { - volumeMounts = append( - volumeMounts, - corev1.VolumeMount{ - Name: "data", - MountPath: kopiaDataMountPath, - }, - ) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "data", MountPath: kopiaDataMountPath}) if server.Spec.Tier1.Cache != nil { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: "cachetier1", - MountPath: kopiaCacheTier1MountPath, - }) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "cachetier1", MountPath: kopiaCacheTier1MountPath}) } _, mount := buildFileSourceVolMount(tier1EncKeyFileVolName, server.Spec.Tier1.EncryptionKeyFile) @@ -638,29 +640,17 @@ func (r *ServerReconciler) buildVolumeMounts(server *kliov1alpha1.Server) []core } if server.Spec.Queue != nil { - volumeMounts = append( - volumeMounts, - corev1.VolumeMount{ - Name: "queue", - MountPath: "/queue", - }, - ) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "queue", MountPath: "/queue"}) } if server.Spec.Tier2 != nil { - volumeMounts = append( - volumeMounts, - corev1.VolumeMount{ - Name: "tier2", - MountPath: "/tier2", - }, - ) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "tier2", MountPath: "/tier2"}) if server.Spec.Tier2.Cache != nil { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: "cachetier2", - MountPath: kopiaCacheTier2MountPath, - }) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "cachetier2", MountPath: kopiaCacheTier2MountPath}) } _, mount := buildFileSourceVolMount(tier2EncKeyFileVolName, server.Spec.Tier2.EncryptionKeyFile) diff --git a/operator/internal/controller/server_statefulset_test.go b/operator/internal/controller/server_statefulset_test.go index a6bc9d2d..35aa471f 100644 --- a/operator/internal/controller/server_statefulset_test.go +++ b/operator/internal/controller/server_statefulset_test.go @@ -248,26 +248,35 @@ func newTestServerWithBothTiers(t *testing.T) *kliov1alpha1.Server { return server } -func TestVolumeClaimTemplatesIncludeTheRequestedCaches(t *testing.T) { - server := newTestServerWithBothTiers(t) - - ss := &appsv1.StatefulSet{} - injectTier1VolumeClaimTemplates(ss, *server) - injectTier2VolumeClaimTemplates(ss, *server) - - assert.Equal(t, []string{"data", "cachetier1", "cachetier2"}, volumeClaimTemplateNames(ss)) -} - -func TestVolumeClaimTemplatesOmitTheCachesThatFallBackToData(t *testing.T) { - server := newTestServerWithBothTiers(t) - server.Spec.Tier1.Cache = nil - server.Spec.Tier2.Cache = nil - - ss := &appsv1.StatefulSet{} - injectTier1VolumeClaimTemplates(ss, *server) - injectTier2VolumeClaimTemplates(ss, *server) +func TestVolumeClaimTemplatesFollowTheCacheConfiguration(t *testing.T) { + tests := map[string]struct { + tier1Cache bool + tier2Cache bool + expected []string + }{ + "dedicated caches": {true, true, []string{"data", "cachetier1", "queue", "cachetier2"}}, + "no dedicated caches": {false, false, []string{"data", "queue"}}, + "only tier1": {true, false, []string{"data", "cachetier1", "queue"}}, + "only tier2": {false, true, []string{"data", "queue", "cachetier2"}}, + } - assert.Equal(t, []string{"data"}, volumeClaimTemplateNames(ss)) + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + server := newTestServerWithBothTiers(t) + if !tc.tier1Cache { + server.Spec.Tier1.Cache = nil + } + if !tc.tier2Cache { + server.Spec.Tier2.Cache = nil + } + + ss := &appsv1.StatefulSet{Spec: appsv1.StatefulSetSpec{ + VolumeClaimTemplates: volumeClaimTemplates(*server), + }} + + assert.Equal(t, tc.expected, volumeClaimTemplateNames(ss)) + }) + } } func TestVolumeMountsFollowTheCacheConfiguration(t *testing.T) { From 340955f32db34715712d792334de0a887f60ea95 Mon Sep 17 00:00:00 2001 From: Gabriele Quaresima Date: Fri, 4 Sep 2026 12:03:54 +0200 Subject: [PATCH 5/6] docs: rewrap the cache PVC paragraph to the 80-column limit The paragraph describing the fallback cache location exceeded the documentation line-length limit. Signed-off-by: Gabriele Quaresima --- documentation/web/docs/user/klio_server.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/documentation/web/docs/user/klio_server.md b/documentation/web/docs/user/klio_server.md index ddc0843c..0b37cfb2 100644 --- a/documentation/web/docs/user/klio_server.md +++ b/documentation/web/docs/user/klio_server.md @@ -79,11 +79,11 @@ snapshot operations. Each tier has its own cache, and the optional `tier1.cache` and `tier2.cache` stanzas dedicate a PVC to it. When a tier has no `cache` stanza, its cache is stored inside the Tier 1 data -volume, under `/data/cache_tier1` and `/data/cache_tier2` respectively. This is -the simplest configuration and the one to start from: size the data PVC so that -it accommodates the cache as well. Dedicate a cache PVC when you want the cache -on a different storage class, or when you want its growth to be unable to eat -into the space of your backups. +volume, under `/data/cache_tier1` and `/data/cache_tier2` respectively. This +is the simplest configuration and the one to start from: size the data PVC +so that it accommodates the cache as well. Dedicate a cache PVC when you want +the cache on a different storage class, or when you want its growth to be +unable to eat into the space of your backups. Since the fallback location lives in the Tier 1 data volume, `tier2.cache` is required on a [read-only server](#read-only-mode), where Tier 1 is not From f31a6c748158d61d55858bec79cf8c45a518054e Mon Sep 17 00:00:00 2001 From: Gabriele Quaresima Date: Fri, 4 Sep 2026 12:04:01 +0200 Subject: [PATCH 6/6] test(e2e): verify the stale tier1 cache directory is reclaimed on disk ServerTierReconfiguration only checked the cache PVC lifecycle through the Kubernetes API, so a regression in the in-pod cache reclaim logic would have gone unnoticed. Assert, via du inside the server container, that the fallback cache directory in the data volume is actually populated while the dedicated tier1 cache volume is absent, and that it is empty again once the volume is restored. Verified against a live kind cluster. Assisted-by: Claude Signed-off-by: Gabriele Quaresima --- operator/test/e2e/server_reconfig_test.go | 47 +++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/operator/test/e2e/server_reconfig_test.go b/operator/test/e2e/server_reconfig_test.go index 1c658635..0c4836af 100644 --- a/operator/test/e2e/server_reconfig_test.go +++ b/operator/test/e2e/server_reconfig_test.go @@ -20,8 +20,12 @@ SPDX-License-Identifier: Apache-2.0 package e2e import ( + "bytes" "context" + "fmt" "slices" + "strconv" + "strings" "testing" "time" @@ -192,6 +196,36 @@ func waitForPVCDeleted( ), "cache PVC %s was not reclaimed", pvcName) } +// fallbackTier1CacheDir is where Kopia stores the tier1 cache when the server +// has no dedicated cache volume, mirroring fallbackCacheTier1Path in the +// controller package. +const fallbackTier1CacheDir = "/data/cache_tier1/kopia-cache" + +// cacheDirDiskUsage returns the disk usage, in KB, of a directory inside the +// server pod, or 0 when the directory does not exist. +func cacheDirDiskUsage( + ctx context.Context, + t *testing.T, + r *resources.Resources, + namespace, podName, dir string, +) int { + t.Helper() + + var stdout, stderr bytes.Buffer + cmd := []string{"sh", "-c", fmt.Sprintf("du -sk %s 2>/dev/null", dir)} + _ = r.ExecInPod(ctx, namespace, podName, serverContainerName, cmd, &stdout, &stderr) + + output := strings.TrimSpace(stdout.String()) + if output == "" { + return 0 + } + + size, err := strconv.Atoi(strings.Fields(output)[0]) + require.NoError(t, err, "unexpected du output for %s: %q (stderr: %q)", dir, output, stderr.String()) + + return size +} + // serverReconfigFeature implements the Feature interface for server tier reconfiguration testing. type serverReconfigFeature struct { name string @@ -335,6 +369,13 @@ func (f *serverReconfigFeature) Run() types.StepFunc { ) require.NoError(t, err, "server Pod not ready after the cache volumes were removed") + // Kopia must have populated the fallback cache directory in the data + // volume, so that removing it later is a meaningful check. + podName := stsName + "-0" + require.Positive(t, + cacheDirDiskUsage(ctx, t, r, f.scenario.namespace.Name, podName, fallbackTier1CacheDir), + "fallback tier1 cache directory %s was not populated", fallbackTier1CacheDir) + waitForPVCDeleted(t, r, "cachetier1-"+stsName+"-0", f.scenario.namespace.Name) waitForPVCDeleted(t, r, cachetier2PVCName, f.scenario.namespace.Name) @@ -374,6 +415,12 @@ func (f *serverReconfigFeature) Run() types.StepFunc { "cachetier1 PVC was not recreated", ) + // The dedicated cache volume is back, so the fallback cache directory + // left behind in the data volume must have been reclaimed at startup. + require.Zero(t, + cacheDirDiskUsage(ctx, t, r, f.scenario.namespace.Name, podName, fallbackTier1CacheDir), + "stale tier1 cache directory %s was not reclaimed", fallbackTier1CacheDir) + t.Log("Server tier reconfiguration test passed: all verifications succeeded") return ctx