From f188721dae2b0377f0b29df0fad6e68ba880f74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Ma=CC=88der?= Date: Tue, 30 Mar 2021 17:07:48 +0200 Subject: [PATCH 1/3] replace 'sleep 99999' with 'sleep infinity' --- config/samples/k8up_v1alpha1_prebackuppod.yaml | 4 ++-- docs/modules/ROOT/pages/references/object-specifications.adoc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/samples/k8up_v1alpha1_prebackuppod.yaml b/config/samples/k8up_v1alpha1_prebackuppod.yaml index 9cea0350d..534bc22c6 100644 --- a/config/samples/k8up_v1alpha1_prebackuppod.yaml +++ b/config/samples/k8up_v1alpha1_prebackuppod.yaml @@ -22,7 +22,7 @@ spec: value: mariadb.example.com image: mariadb command: - - 'sleep' - - '9999999' + - sleep + - infinity imagePullPolicy: Always name: mysqldump diff --git a/docs/modules/ROOT/pages/references/object-specifications.adoc b/docs/modules/ROOT/pages/references/object-specifications.adoc index 806362868..255abd993 100644 --- a/docs/modules/ROOT/pages/references/object-specifications.adoc +++ b/docs/modules/ROOT/pages/references/object-specifications.adoc @@ -412,8 +412,8 @@ spec: value: mariadb.example.com image: mariadb command: - - 'sleep' - - '9999999' + - sleep + - infinity imagePullPolicy: Always name: mysqldump ---- From 8ee242983e23e7eee0e78e51b505a7d26bb9748b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Ma=CC=88der?= Date: Tue, 30 Mar 2021 17:08:27 +0200 Subject: [PATCH 2/3] Integration test to discover PreBackupPod zombies --- controllers/backup_integration_test.go | 25 +++++++-- controllers/backup_it_utils_test.go | 71 ++++++++++++++++++++++---- controllers/envsuite_test.go | 2 +- 3 files changed, 85 insertions(+), 13 deletions(-) diff --git a/controllers/backup_integration_test.go b/controllers/backup_integration_test.go index 00243af6a..eb958ff2e 100644 --- a/controllers/backup_integration_test.go +++ b/controllers/backup_integration_test.go @@ -12,6 +12,7 @@ import ( k8upv1a1 "github.com/vshn/k8up/api/v1alpha1" "github.com/vshn/k8up/controllers" + "github.com/vshn/k8up/observer" ) type BackupTestSuite struct { @@ -54,7 +55,7 @@ func (ts *BackupTestSuite) Test_GivenPreBackupPods_ExpectPreBackupDeployment() { ts.expectAPreBackupDeploymentEventually() ts.assertConditionWaitingForPreBackup(ts.BackupResource) - ts.afterDeploymentStarted() + ts.afterPreBackupDeploymentStarted() _ = ts.whenReconciling(ts.BackupResource) ts.assertConditionReadyForPreBackup(ts.BackupResource) ts.expectABackupContainer() @@ -77,7 +78,25 @@ func (ts *BackupTestSuite) Test_GivenFailedPreBackupDeployment_WhenCreatingNewBa result := ts.whenReconciling(ts.BackupResource) ts.Assert().GreaterOrEqual(result.RequeueAfter, 30*time.Second) ts.assertDeploymentIsDeleted(failedDeployment) - ts.assertConditionFailedBackup(ts.BackupResource) + ts.assertPreBackupPodConditionFailed(ts.BackupResource) +} + +func (ts *BackupTestSuite) Test_GivenFinishedPreBackupDeployment_WhenReconciling_ThenExpectPreBackupToBeRemoved() { + // The backup reconciliation loop must run in order to start a backup, + // so that the callback is registered which eventually cleans up the PreBackupPods. + ts.EnsureResources(ts.newPreBackupPod(), ts.BackupResource) + _ = ts.whenReconciling(ts.BackupResource) + ts.expectAPreBackupDeploymentEventually() + + ts.afterPreBackupDeploymentStarted() + _ = ts.whenReconciling(ts.BackupResource) + ts.expectABackupJobEventually() + ts.markBackupAsFinished(ts.BackupResource) + + suceeded := observer.Suceeded + ts.notifyObserverOfBackupJobStatusChange(suceeded) + + ts.assertDeploymentIsDeleted(ts.newPreBackupDeployment()) } func (ts *BackupTestSuite) Test_GivenPreBackupPods_WhenRestartingK8up_ThenExpectToContinueWhereItLeftOff() { @@ -87,7 +106,7 @@ func (ts *BackupTestSuite) Test_GivenPreBackupPods_WhenRestartingK8up_ThenExpect ts.expectAPreBackupDeploymentEventually() ts.whenCancellingTheContext() - ts.afterDeploymentStarted() + ts.afterPreBackupDeploymentStarted() _ = ts.whenReconciling(ts.BackupResource) ts.expectABackupContainer() } diff --git a/controllers/backup_it_utils_test.go b/controllers/backup_it_utils_test.go index 187a71e88..f6853fb61 100644 --- a/controllers/backup_it_utils_test.go +++ b/controllers/backup_it_utils_test.go @@ -4,9 +4,13 @@ package controllers_test import ( "context" + "fmt" "time" "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -15,11 +19,8 @@ import ( controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - appsv1 "k8s.io/api/apps/v1" - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" - k8upv1a1 "github.com/vshn/k8up/api/v1alpha1" + k8upObserver "github.com/vshn/k8up/observer" ) func (ts *BackupTestSuite) newPreBackupPod() *k8upv1a1.PreBackupPod { @@ -100,7 +101,7 @@ func (ts *BackupTestSuite) whenCancellingTheContext() { ts.Ctx, ts.CancelCtx = context.WithCancel(context.Background()) } -func (ts *BackupTestSuite) afterDeploymentStarted() { +func (ts *BackupTestSuite) afterPreBackupDeploymentStarted() { deployment := &appsv1.Deployment{} deploymentIdentifier := types.NamespacedName{Namespace: ts.NS, Name: ts.PreBackupPodName} ts.FetchResource(deploymentIdentifier, deployment) @@ -158,7 +159,7 @@ func (ts *BackupTestSuite) assertConditionReadyForPreBackup(backup *k8upv1a1.Bac }) } -func (ts *BackupTestSuite) assertConditionFailedBackup(backup *k8upv1a1.Backup) { +func (ts *BackupTestSuite) assertPreBackupPodConditionFailed(backup *k8upv1a1.Backup) { ts.RepeatedAssert(5*time.Second, time.Second, "backup does not have expected condition", func(timedCtx context.Context) (done bool, err error) { err = ts.Client.Get(timedCtx, k8upv1a1.MapToNamespacedName(backup), backup) ts.Require().NoError(err) @@ -172,15 +173,31 @@ func (ts *BackupTestSuite) assertConditionFailedBackup(backup *k8upv1a1.Backup) }) } -func (ts *BackupTestSuite) assertDeploymentIsDeleted(failedDeployment *appsv1.Deployment) { - ts.RepeatedAssert(5*time.Second, time.Second, "deployment still exists, but it shouldn't", func(timedCtx context.Context) (done bool, err error) { - if !ts.IsResourceExisting(timedCtx, failedDeployment) { +func (ts *BackupTestSuite) assertPreBackupPodConditionSucceeded(backup *k8upv1a1.Backup) { + ts.RepeatedAssert(5*time.Second, time.Second, "backup does not have expected condition", func(timedCtx context.Context) (done bool, err error) { + err = ts.Client.Get(timedCtx, k8upv1a1.MapToNamespacedName(backup), backup) + ts.Require().NoError(err) + preBackupCond := meta.FindStatusCondition(backup.Status.Conditions, k8upv1a1.ConditionPreBackupPodReady.String()) + if preBackupCond != nil && preBackupCond.Reason == k8upv1a1.ReasonFailed.String() { + ts.Assert().Equal(k8upv1a1.ReasonSucceeded.String(), preBackupCond.Reason) + ts.Assert().Equal(metav1.ConditionFalse, preBackupCond.Status) return true, nil } return false, nil }) } +func (ts *BackupTestSuite) assertDeploymentIsDeleted(deployment *appsv1.Deployment) { + ts.RepeatedAssert( + 5*time.Second, time.Second, + fmt.Sprintf("deployment '%s/%s' still exists, but it shouldn't", deployment.Namespace, deployment.Name), + func(timedCtx context.Context) (done bool, err error) { + isDeleted := !ts.IsResourceExisting(timedCtx, deployment) + return isDeleted, nil + }, + ) +} + func (ts *BackupTestSuite) newPreBackupDeployment() *appsv1.Deployment { return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -216,3 +233,39 @@ func (ts *BackupTestSuite) markDeploymentAsFailed(deployment *appsv1.Deployment) } ts.UpdateStatus(deployment) } + +func (ts *BackupTestSuite) markDeploymentAsFinished(deployment *appsv1.Deployment) { + deployment.Status = appsv1.DeploymentStatus{ + Conditions: []appsv1.DeploymentCondition{ + {Type: "Progressing", Status: corev1.ConditionTrue, LastUpdateTime: metav1.Now(), Reason: "NewReplicaSetAvailable", Message: "deployment successful"}, + {Type: "Available", Status: corev1.ConditionTrue, LastUpdateTime: metav1.Now(), Reason: "MinimumReplicasAvailable", Message: "deployment successful"}, + }, + } + ts.UpdateStatus(deployment) +} + +func (ts *BackupTestSuite) markBackupAsFinished(backup *k8upv1a1.Backup) { + backup.Status = k8upv1a1.Status{ + Started: true, + Finished: true, + Conditions: []metav1.Condition{ + {Type: "PreBackupPodReady", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Now(), Reason: "Ready", Message: "backup successful"}, + {Type: "Ready", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Now(), Reason: "Ready", Message: "backup successful"}, + {Type: "Progressing", Status: metav1.ConditionFalse, LastTransitionTime: metav1.Now(), Reason: "Finished", Message: "backup successful"}, + {Type: "Completed", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Now(), Reason: "Succeeded", Message: "backup successful"}, + }, + } +} + +func (ts *BackupTestSuite) notifyObserverOfBackupJobStatusChange(status k8upObserver.EventType) { + observer := k8upObserver.GetObserver() + event := observer.GetJobByName(ts.BackupResource.Namespace + "/" + ts.BackupResource.Name) + event.Job = &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: ts.BackupResource.Name, + Namespace: ts.BackupResource.Namespace, + }, + } + event.Event = status + observer.GetUpdateChannel() <- event +} diff --git a/controllers/envsuite_test.go b/controllers/envsuite_test.go index c30b47b58..3925941a3 100644 --- a/controllers/envsuite_test.go +++ b/controllers/envsuite_test.go @@ -233,7 +233,7 @@ func (ts *EnvTestSuite) SanitizeNameForNS(name string) string { } // IsResourceExisting tries to fetch the given resource and returns true if it exists. -// It will consider still-existing object with a deletion timestamp as not existing. +// It will consider still-existing object with a deletion timestamp as non-existing. // Any other errors will fail the test. func (ts *EnvTestSuite) IsResourceExisting(ctx context.Context, obj client.Object) bool { err := ts.Client.Get(ctx, k8upv1a1.MapToNamespacedName(obj), obj) From 062731f45cc7b3e174e0c61d7f368f5f765087cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Ma=CC=88der?= Date: Thu, 1 Apr 2021 11:46:15 +0200 Subject: [PATCH 3/3] Fix missing cleanup of completed PreBackupPod deployments --- executor/prebackup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/prebackup.go b/executor/prebackup.go index 92ea2e44f..1e5c4920c 100644 --- a/executor/prebackup.go +++ b/executor/prebackup.go @@ -92,7 +92,7 @@ func (b *BackupExecutor) StopPreBackupDeployments() { return } - deployments := b.generateDeployments(nil) + deployments := b.generateDeployments(templates.Items) for _, deployment := range deployments { // Avoid exportloopref deployment := deployment