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..59952b99 --- /dev/null +++ b/core/cmd/server/initialize_test.go @@ -0,0 +1,69 @@ +/* +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) +} 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"` 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..0b37cfb2 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: 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..68c0db8e --- /dev/null +++ b/operator/internal/controller/server_cachevalidation_test.go @@ -0,0 +1,135 @@ +/* +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" + + kliov1alpha1 "github.com/cloudnative-pg/klio/operator/api/v1alpha1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// 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 { + 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: newTestFileSource("enc-secret", "encryption-key.age"), + IdentityFile: newTestFileSource("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: newPVCSpec("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: 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: newPVCSpec("2Gi"), + } + Expect(create(server)).To(Succeed()) + + 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_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..2b61a319 100644 --- a/operator/internal/controller/server_envbuilder.go +++ b/operator/internal/controller/server_envbuilder.go @@ -30,6 +30,21 @@ import ( const kopiaCacheSubdirectory = "kopia-cache" +// 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 fallback, "" + } + + return path.Join(mountPath, kopiaCacheSubdirectory), fallback +} + type envBuilder struct { builtEnvs []corev1.EnvVar @@ -110,11 +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: path.Join(kopiaCacheTier1MountPath, kopiaCacheSubdirectory), + Value: tier1Cache, + }, + corev1.EnvVar{ + Name: "TIER1_BASE_STALE_CACHE", + Value: tier1StaleCache, }, corev1.EnvVar{ Name: "TIER1_BASE_REPOSITORY", @@ -166,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", @@ -177,7 +200,11 @@ func (e *envBuilder) getTier2EnvVars() []corev1.EnvVar { }, { Name: "TIER2_CACHE", - Value: path.Join(kopiaCacheTier2MountPath, kopiaCacheSubdirectory), + Value: tier2Cache, + }, + { + Name: "TIER2_STALE_CACHE", + 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 c41ca69f..c66497c3 100644 --- a/operator/internal/controller/server_envbuilder_test.go +++ b/operator/internal/controller/server_envbuilder_test.go @@ -333,3 +333,52 @@ func TestBuildIdentityVolMountProjected(t *testing.T) { assert.Equal(t, int32(0o400), *vol.Projected.DefaultMode) assert.True(t, mount.ReadOnly) } + +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", + }, + "fallback to the data volume": { + tier1: "/data/cache_tier1/kopia-cache", + tier2: "/data/cache_tier2/kopia-cache", + }, + } + + 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{} + } + + envVars := append(builder.getCoreEnvVars(), builder.getTier2EnvVars()...) + + 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) + }) + } +} diff --git a/operator/internal/controller/server_pvc_resize.go b/operator/internal/controller/server_pvc_resize.go index ee68c402..43061564 100644 --- a/operator/internal/controller/server_pvc_resize.go +++ b/operator/internal/controller/server_pvc_resize.go @@ -150,29 +150,31 @@ 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 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 tier1.Cache != nil { + addPVCSize(sizes, pvcTypeCacheTier1, tier1.Cache.PersistentVolumeClaimTemplate) } } - if server.Spec.Tier2 != nil { - if size, ok := server.Spec.Tier2.Cache.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeCacheTier2] = size - } + if tier2 := server.Spec.Tier2; tier2 != nil && tier2.Cache != nil { + addPVCSize(sizes, pvcTypeCacheTier2, tier2.Cache.PersistentVolumeClaimTemplate) } - if server.Spec.Queue != nil { - if size, ok := server.Spec.Queue.PersistentVolumeClaimTemplate.Resources.Requests[corev1.ResourceStorage]; ok { - sizes[pvcTypeQueue] = size - } + if queue := server.Spec.Queue; queue != nil { + addPVCSize(sizes, pvcTypeQueue, queue.PersistentVolumeClaimTemplate) } return sizes 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..9a5a23f8 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,52 @@ 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) +} + +// 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 @@ -192,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()) @@ -307,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 @@ -352,48 +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, }, - corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cachetier1", - Labels: map[string]string{ - klioServerLabel: server.Name, - pvcTypeLabel: pvcTypeCacheTier1, - }, - }, - Spec: server.Spec.Tier1.Cache.PersistentVolumeClaimTemplate, - }) + Spec: spec, + } } -func injectTier2VolumeClaimTemplates( - ss *appsv1.StatefulSet, - server kliov1alpha1.Server, -) { - 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, - }) +// 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 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)) + } + } + + if queue := server.Spec.Queue; queue != nil { + templates = append(templates, + serverPVC(server, "queue", pvcTypeQueue, queue.PersistentVolumeClaimTemplate)) + } + + if tier2 := server.Spec.Tier2; tier2 != nil && tier2.Cache != nil { + templates = append(templates, + serverPVC(server, "cachetier2", pvcTypeCacheTier2, tier2.Cache.PersistentVolumeClaimTemplate)) + } + + return templates } func (r *ServerReconciler) reconcileService(ctx context.Context, server *kliov1alpha1.Server) error { @@ -591,17 +624,14 @@ func (r *ServerReconciler) buildVolumeMounts(server *kliov1alpha1.Server) []core } if server.Spec.Tier1 != nil { - volumeMounts = append( - volumeMounts, - corev1.VolumeMount{ - Name: "data", - MountPath: kopiaDataMountPath, - }, - corev1.VolumeMount{ - Name: "cachetier1", - MountPath: kopiaCacheTier1MountPath, - }, - ) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "data", MountPath: kopiaDataMountPath}) + + 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) @@ -610,27 +640,19 @@ 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", - }, - corev1.VolumeMount{ - Name: "cachetier2", - MountPath: kopiaCacheTier2MountPath, - }, - ) + volumeMounts = append(volumeMounts, + corev1.VolumeMount{Name: "tier2", MountPath: "/tier2"}) + + 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..35aa471f 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,82 @@ 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 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"}}, + } + + 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) { + 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..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" @@ -34,6 +38,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 +143,89 @@ 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) +} + +// 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 @@ -258,6 +346,81 @@ 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") + + // 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) + + // 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", + ) + + // 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 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},