From 6ce6197e536c4aeeacfeaf4f854a23a54a2d1a03 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 29 Mar 2018 16:44:23 +0200 Subject: [PATCH 1/3] Fixing PV cleanup --- pkg/storage/pv_cleanup.go | 3 ++- pkg/storage/pv_inspector.go | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/storage/pv_cleanup.go b/pkg/storage/pv_cleanup.go index bac653572..5d07f0220 100644 --- a/pkg/storage/pv_cleanup.go +++ b/pkg/storage/pv_cleanup.go @@ -34,6 +34,7 @@ import ( "k8s.io/client-go/kubernetes" "github.com/arangodb/kube-arangodb/pkg/storage/provisioner" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/arangodb/kube-arangodb/pkg/util/trigger" ) @@ -159,7 +160,7 @@ func (c *pvCleaner) clean(pv v1.PersistentVolume) error { } // Remove persistent volume - if err := c.cli.CoreV1().PersistentVolumes().Delete(pv.GetName(), &metav1.DeleteOptions{}); err != nil { + if err := c.cli.CoreV1().PersistentVolumes().Delete(pv.GetName(), &metav1.DeleteOptions{}); err != nil && !k8sutil.IsNotFound(err) { log.Debug().Err(err). Str("name", pv.GetName()). Msg("Failed to remove PersistentVolume") diff --git a/pkg/storage/pv_inspector.go b/pkg/storage/pv_inspector.go index b9ee52041..14fe12163 100644 --- a/pkg/storage/pv_inspector.go +++ b/pkg/storage/pv_inspector.go @@ -23,6 +23,8 @@ package storage import ( + "time" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,6 +40,7 @@ func (ls *LocalStorage) inspectPVs() (int, error) { } spec := ls.apiObject.Spec availableVolumes := 0 + cleanupBeforeTimestamp := time.Now().Add(time.Hour * -24) for _, pv := range list.Items { if pv.Spec.StorageClassName != spec.StorageClass.Name { // Not our storage class @@ -45,7 +48,20 @@ func (ls *LocalStorage) inspectPVs() (int, error) { } switch pv.Status.Phase { case v1.VolumeAvailable: - availableVolumes++ + // Is this an old volume? + if pv.GetObjectMeta().GetCreationTimestamp().Time.Before(cleanupBeforeTimestamp) { + // Let's clean it up + if ls.isOwnerOf(&pv) { + // Cleanup this volume + log.Debug().Str("name", pv.GetName()).Msg("Added PersistentVolume to cleaner") + ls.pvCleaner.Add(pv) + } else { + log.Debug().Str("name", pv.GetName()).Msg("PersistentVolume is not owned by us") + availableVolumes++ + } + } else { + availableVolumes++ + } case v1.VolumeReleased: if ls.isOwnerOf(&pv) { // Cleanup this volume From 0c380031abefc73b6279a0c8621680d997381270 Mon Sep 17 00:00:00 2001 From: Jan Christoph Uhde Date: Fri, 30 Mar 2018 19:10:59 +0200 Subject: [PATCH 2/3] add notes about persistent-volumes to testing.md --- docs/design/testing.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/design/testing.md b/docs/design/testing.md index 786ca9646..25e4680ab 100644 --- a/docs/design/testing.md +++ b/docs/design/testing.md @@ -23,6 +23,14 @@ The following test scenario's must be covered by automated tests: - Restart Node - API server unavailable +- Persistent Volumes: + - hint: RBAC file might need to be changed + - hint: get info via - client-go.CoreV1() + - Number of volumes should stay in reasonable bounds + - For some cases it might be possible to check that, the amount before and after the test stays the same + - A Cluster start should need 6 Volumes (DBServer + Agents) + - The release of a volume-claim should result in a release of the volume + ## Test environments - Kubernetes clusters From f24b0f88c6df6a124e3cab100bb18109a22513aa Mon Sep 17 00:00:00 2001 From: Jan Christoph Uhde Date: Thu, 5 Apr 2018 12:56:55 +0200 Subject: [PATCH 3/3] WIP: begin writing persistent volume tests --- tests/persistent_volumes_test.go | 82 ++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 tests/persistent_volumes_test.go diff --git a/tests/persistent_volumes_test.go b/tests/persistent_volumes_test.go new file mode 100644 index 000000000..af0cb99ad --- /dev/null +++ b/tests/persistent_volumes_test.go @@ -0,0 +1,82 @@ +// +// DISCLAIMER +// +// Copyright 2018 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Jan Christoph Uhde +// +package tests + +import ( + "fmt" + "strings" + "testing" + + "github.com/dchest/uniuri" + "github.com/stretchr/testify/assert" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + kubeArangoClient "github.com/arangodb/kube-arangodb/pkg/client" + //"github.com/arangodb/kube-arangodb/pkg/util" +) + +// TODO - add description +func TestPersistence(t *testing.T) { + longOrSkip(t) + + k8sNameSpace := getNamespace(t) + k8sClient := mustNewKubeClient(t) + + volumesList, err := k8sClient.CoreV1().PersistentVolumes().List(metav1.ListOptions{}) + assert.NoError(t, err, "error while listing volumes") + claimsList, err := k8sClient.CoreV1().PersistentVolumeClaims(k8sNameSpace).List(metav1.ListOptions{}) + assert.NoError(t, err, "error while listing volume claims") + + fmt.Printf("----------------------------------------") + fmt.Printf("%v %v", volumesList, claimsList) + fmt.Printf("----------------------------------------") + fmt.Printf("%v %v", len(volumesList.Items), len(claimsList.Items)) + fmt.Printf("----------------------------------------") + + mode := api.DeploymentModeCluster + engine := api.StorageEngineRocksDB + + deploymentClient := kubeArangoClient.MustNewInCluster() + deploymentTemplate := newDeployment(strings.Replace(fmt.Sprintf("tpers-%s-%s-%s", mode[:2], engine[:2], uniuri.NewLen(4)), ".", "", -1)) + deploymentTemplate.Spec.Mode = api.NewMode(mode) + deploymentTemplate.Spec.StorageEngine = api.NewStorageEngine(engine) + deploymentTemplate.Spec.TLS = api.TLSSpec{} + //deploymentTemplate.Spec.Environment = api.NewEnvironment(api.EnvironmentDevelopment) + //deploymentTemplate.Spec.Image = util.NewString("arangodb/arangodb:3.3.4") + //deploymentTemplate.Spec.DBServers.Count = util.NewInt(numNodes + 1) + deploymentTemplate.Spec.SetDefaults(deploymentTemplate.GetName()) // this must be last + assert.NoError(t, deploymentTemplate.Spec.Validate()) + + // Create deployment + _, err = deploymentClient.DatabaseV1alpha().ArangoDeployments(k8sNameSpace).Create(deploymentTemplate) + assert.NoError(t, err, "failed to create deplyment: %s", err) + + _, err = waitUntilDeployment(deploymentClient, deploymentTemplate.GetName(), k8sNameSpace, deploymentIsReady()) + assert.NoError(t, err, fmt.Sprintf("Deployment not running in time: %s", err)) // <-- fails here at the moment + + // TODO - add tests that check the number of volumes and claims + + // Cleanup + removeDeployment(deploymentClient, deploymentTemplate.GetName(), k8sNameSpace) +}