From 33832a761fb97fee48a2852aee686582fe4d5bed Mon Sep 17 00:00:00 2001 From: Andrey Velichkevich Date: Thu, 6 Aug 2020 13:32:55 +0100 Subject: [PATCH] Verify that Trials were successfully deleted (#1288) * Verify that trials were deleted Update suggestion status * Update suggestion requests * Fix tests * Fix comment * Add recorder to test controllers * Travis test * Change resume exp trial condition * Modify e2e for from volume experiment * Fix IsRestarting check * Fix comment --- .../controller/suggestions/v1beta1/util.go | 14 ++ .../experiment/experiment_controller.go | 56 ++++++ .../experiment/experiment_controller_test.go | 187 ++++++++++++------ .../experiment/experiment_util.go | 11 +- .../suggestion/suggestion_controller.go | 3 +- .../suggestion/suggestion_controller_test.go | 16 +- test/e2e/v1beta1/resume-e2e-experiment.go | 2 +- test/e2e/v1beta1/run-e2e-experiment.go | 10 +- test/scripts/v1beta1/run-from-volume.sh | 4 + 9 files changed, 225 insertions(+), 78 deletions(-) diff --git a/pkg/apis/controller/suggestions/v1beta1/util.go b/pkg/apis/controller/suggestions/v1beta1/util.go index a96adb46ae5..2273f918fef 100644 --- a/pkg/apis/controller/suggestions/v1beta1/util.go +++ b/pkg/apis/controller/suggestions/v1beta1/util.go @@ -5,6 +5,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // SuggestionRestartReason is the reason for suggestion status when experiment is restarting + SuggestionRestartReason = "Experiment is restarting" +) + func getCondition(suggestion *Suggestion, condType SuggestionConditionType) *SuggestionCondition { if suggestion.Status.Conditions != nil { for _, condition := range suggestion.Status.Conditions { @@ -64,6 +69,15 @@ func (suggestion *Suggestion) IsRunning() bool { return hasCondition(suggestion, SuggestionRunning) } +// IsRestarting returns true if suggestion running status is false and reason = SuggestionRestartReason +func (suggestion *Suggestion) IsRestarting() bool { + cond := getCondition(suggestion, SuggestionRunning) + if cond != nil && cond.Status == v1.ConditionFalse && cond.Reason == SuggestionRestartReason { + return true + } + return false +} + func (suggestion *Suggestion) IsDeploymentReady() bool { return hasCondition(suggestion, SuggestionDeploymentReady) } diff --git a/pkg/controller.v1beta1/experiment/experiment_controller.go b/pkg/controller.v1beta1/experiment/experiment_controller.go index d4dd1668944..b4c6cb81ebd 100644 --- a/pkg/controller.v1beta1/experiment/experiment_controller.go +++ b/pkg/controller.v1beta1/experiment/experiment_controller.go @@ -18,7 +18,9 @@ package experiment import ( "context" + "fmt" "sort" + "time" "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" @@ -385,12 +387,66 @@ func (r *ReconcileExperiment) deleteTrials(instance *experimentsv1beta1.Experime "expectedDeletions", expected, "trials", actual) expected = actual } + deletedNames := []string{} for i := 0; i < expected; i++ { if err := r.Delete(context.TODO(), &trialSlice[i]); err != nil { logger.Error(err, "Trial Delete error") return err } + deletedNames = append(deletedNames, trialSlice[i].Name) } + + // Check if trials were deleted + timeout := 60 * time.Second + endTime := time.Now().Add(timeout) + + for _, name := range deletedNames { + var err error + for !errors.IsNotFound(err) && time.Now().Before(endTime) { + err = r.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: instance.GetNamespace()}, &trialsv1beta1.Trial{}) + } + // If trials were deleted, err == IsNotFound, return error if timeout is out + if !errors.IsNotFound(err) { + return fmt.Errorf("Unable to delete trials %v, error: %v", deletedNames, err) + } + } + + // We have to delete trials from suggestion status and update SuggestionCount + suggestion := &suggestionsv1beta1.Suggestion{} + err := r.Get(context.TODO(), types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}, suggestion) + if err != nil { + logger.Error(err, "Suggestion Get error") + return err + } + + deletedNamesMap := make(map[string]bool) + for _, name := range deletedNames { + deletedNamesMap[name] = true + } + // Create new Trial Assignment without deleted trials + newTrialAssignment := []suggestionsv1beta1.TrialAssignment{} + for _, ta := range suggestion.Status.Suggestions { + if _, ok := deletedNamesMap[ta.Name]; !ok { + newTrialAssignment = append(newTrialAssignment, ta) + } + } + + // Update suggestion spec first. + // If Requests <= SuggestionCount suggestion controller returns nil. + suggestion.Spec.Requests = int32(len(newTrialAssignment)) + if err := r.UpdateSuggestion(suggestion); err != nil { + return err + } + + // Update suggestion status + suggestion.Status.Suggestions = newTrialAssignment + suggestion.Status.SuggestionCount = int32(len(newTrialAssignment)) + if err := r.UpdateSuggestionStatus(suggestion); err != nil { + return err + } + + logger.Info("Trials were successfully deleted", "trialNames", deletedNames) + return nil } diff --git a/pkg/controller.v1beta1/experiment/experiment_controller_test.go b/pkg/controller.v1beta1/experiment/experiment_controller_test.go index 2217a7bcfb7..e3f7da972ca 100644 --- a/pkg/controller.v1beta1/experiment/experiment_controller_test.go +++ b/pkg/controller.v1beta1/experiment/experiment_controller_test.go @@ -49,7 +49,7 @@ type statusMatcher struct { } func (statusM statusMatcher) Matches(x interface{}) bool { - // Cast interface to suggestion object + // Cast interface to suggestion s := x.(*suggestionsv1beta1.Suggestion) isMatch := false @@ -102,9 +102,18 @@ func TestReconcile(t *testing.T) { Suggestion: mockSuggestion, Generator: mockGenerator, collector: experimentUtil.NewExpsCollector(mgr.GetCache(), prometheus.NewRegistry()), + recorder: mgr.GetRecorder(ControllerName), } r.updateStatusHandler = func(instance *experimentsv1beta1.Experiment) error { - return r.updateStatus(instance) + var err error = errors.NewBadRequest("fake-error") + // Try to update status until it be succeeded + for err != nil { + updatedInstance := &experimentsv1beta1.Experiment{} + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, updatedInstance) + updatedInstance.Status = instance.Status + err = r.updateStatus(updatedInstance) + } + return err } recFn := SetupTestReconcile(r) @@ -129,44 +138,101 @@ func TestReconcile(t *testing.T) { returnedUnstructured, nil).AnyTimes() - suggestion := newFakeSuggestion() + suggestionRestartNo := newFakeSuggestion() mockSuggestion.EXPECT().GetOrCreateSuggestion(gomock.Any(), gomock.Any()).Return( - suggestion, nil).AnyTimes() + suggestionRestartNo, nil).AnyTimes() mockSuggestion.EXPECT().UpdateSuggestion(gomock.Any()).Return(nil).AnyTimes() - suggestionRestartNo := newFakeSuggestion() + reasonRestart := "Experiment is succeeded" + msgRestartNo := "Suggestion is succeeded, can't be restarted" + suggestionRestartNo.MarkSuggestionStatusSucceeded(reasonRestart, msgRestartNo) + suggestionRestartYes := newFakeSuggestion() suggestionRestartYes.Spec.ResumePolicy = experimentsv1beta1.FromVolume - suggestionRestarting := newFakeSuggestion() - reason := "Experiment is succeeded" - msg := "Suggestion is succeeded, can't be restarted" - suggestionRestartNo.MarkSuggestionStatusSucceeded(reason, msg) + msgRestartYes := "Suggestion is succeeded, suggestion volume is not deleted, can be restarted" + suggestionRestartYes.MarkSuggestionStatusSucceeded(reasonRestart, msgRestartYes) - msg = "Suggestion is succeeded, suggestion volume is not deleted, can be restarted" - suggestionRestartYes.MarkSuggestionStatusSucceeded(reason, msg) + suggestionRestarting := newFakeSuggestion() - reason = "Experiment is restarting" - msg = "Suggestion is not running" - suggestionRestarting.MarkSuggestionStatusRunning(corev1.ConditionFalse, reason, msg) + msgRestarting := "Suggestion is not running" + suggestionRestarting.MarkSuggestionStatusRunning(corev1.ConditionFalse, suggestionsv1beta1.SuggestionRestartReason, msgRestarting) // Manually update suggestion status after UpdateSuggestionStatus is called - mockSuggestion.EXPECT().UpdateSuggestionStatus(statusMatcher{suggestionRestartNo}).Return(nil).MinTimes(1).Do( + // Call when Trials are being deleted + deleteTrialsCall := mockSuggestion.EXPECT().UpdateSuggestionStatus(gomock.Any()).Return(nil).Do( + func(arg0 interface{}) { + var err error = errors.NewBadRequest("fake-error") + suggestion := &suggestionsv1beta1.Suggestion{} + // We should Get suggestion because resource version can be modified + for err != nil { + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) + suggestion.Status.Suggestions = suggestion.Status.Suggestions[1:] + err = c.Status().Update(context.TODO(), suggestion) + } + }) + + // Call when experiment is completed with ResumePolicy = NeverResume + restartNoCall := mockSuggestion.EXPECT().UpdateSuggestionStatus(statusMatcher{suggestionRestartNo}).Return(nil).Do( + func(arg0 interface{}) { + var err error = errors.NewBadRequest("fake-error") + suggestion := &suggestionsv1beta1.Suggestion{} + for err != nil { + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) + suggestion.MarkSuggestionStatusSucceeded(reasonRestart, msgRestartNo) + err = c.Status().Update(context.TODO(), suggestion) + } + }) + + // Call when experiment is completed with ResumePolicy = FromVolume + restartYesCall := mockSuggestion.EXPECT().UpdateSuggestionStatus(statusMatcher{suggestionRestartYes}).Return(nil).Do( func(arg0 interface{}) { - c.Status().Update(context.TODO(), suggestionRestartNo) + var err error = errors.NewBadRequest("fake-error") + suggestion := &suggestionsv1beta1.Suggestion{} + for err != nil { + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) + suggestion.MarkSuggestionStatusSucceeded(reasonRestart, msgRestartYes) + err = c.Status().Update(context.TODO(), suggestion) + } }) - mockSuggestion.EXPECT().UpdateSuggestionStatus(statusMatcher{suggestionRestartYes}).Return(nil).MinTimes(1).Do( + + // Call when experiment is restarting + experimentRestartingCall := mockSuggestion.EXPECT().UpdateSuggestionStatus(statusMatcher{suggestionRestarting}).Return(nil).Do( func(arg0 interface{}) { - c.Status().Update(context.TODO(), suggestionRestartYes) + suggestion := &suggestionsv1beta1.Suggestion{} + var err error = errors.NewBadRequest("fake-error") + for err != nil { + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) + suggestion.MarkSuggestionStatusRunning(corev1.ConditionFalse, suggestionsv1beta1.SuggestionRestartReason, msgRestarting) + err = c.Status().Update(context.TODO(), suggestion) + } }) - mockSuggestion.EXPECT().UpdateSuggestionStatus(statusMatcher{suggestionRestarting}).Return(nil).MinTimes(1) + + gomock.InOrder( + deleteTrialsCall, + restartNoCall, + restartYesCall, + experimentRestartingCall, + ) // Test 1 - Regural experiment run instance := newFakeInstance() - // Create the experiment object + // Create the suggestion with NeverResume + g.Expect(c.Create(context.TODO(), suggestionRestartNo)).NotTo(gomega.HaveOccurred()) + // Manually update suggestion's status with 3 suggestions + // Ones redundant trial is deleted, suggestion status must be updated + g.Eventually(func() error { + suggestion := &suggestionsv1beta1.Suggestion{} + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) + suggestion.Status.Suggestions = newFakeSuggestion().Status.Suggestions + errStatus := c.Status().Update(context.TODO(), suggestion) + return errStatus + }, timeout).ShouldNot(gomega.HaveOccurred()) + + // Create the experiment g.Expect(c.Create(context.TODO(), instance)).NotTo(gomega.HaveOccurred()) // Expect that experiment status is running @@ -177,28 +243,32 @@ func TestReconcile(t *testing.T) { }, timeout).Should(gomega.BeTrue()) // Expect that 2 trials are created, 1 should be deleted because ParallelTrialCount=2 - trials := &trialsv1beta1.TrialList{} - label := labels.Set{ - consts.LabelExperimentName: experimentName, - } g.Eventually(func() int { + trials := &trialsv1beta1.TrialList{} + label := labels.Set{ + consts.LabelExperimentName: experimentName, + } c.List(context.TODO(), &client.ListOptions{LabelSelector: label.AsSelector()}, trials) return len(trials.Items) }, timeout).Should(gomega.Equal(2)) - // Create the suggestion object with NeverResume - g.Expect(c.Create(context.TODO(), suggestionRestartNo)).NotTo(gomega.HaveOccurred()) - // Expect that suggestion is created + // Expect that suggestion status doesn't have first deleted trial + // test-trial-1 must be deleted from suggestion status + // UpdateSuggestionStatus with deleteTrialsCall call g.Eventually(func() bool { - test := &suggestionsv1beta1.Suggestion{} - c.Get(context.TODO(), - types.NamespacedName{Namespace: namespace, Name: experimentName}, test) - return errors.IsNotFound(c.Get(context.TODO(), - types.NamespacedName{Namespace: namespace, Name: experimentName}, &suggestionsv1beta1.Suggestion{})) - }, timeout).ShouldNot(gomega.BeTrue()) + suggestion := &suggestionsv1beta1.Suggestion{} + isDeleted := true + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) + for _, s := range suggestion.Status.Suggestions { + if s.Name == trialName+"-1" { + isDeleted = false + } + } + return isDeleted + }, timeout).Should(gomega.BeTrue()) - // Manually update suggestion status to failed to make experiment completed - // Expect that suggestion is updated + // Manually update experiment status to failed to make experiment completed + // Expect that experiment is updated g.Eventually(func() error { experiment = &experimentsv1beta1.Experiment{} c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, experiment) @@ -207,22 +277,22 @@ func TestReconcile(t *testing.T) { }, timeout).ShouldNot(gomega.HaveOccurred()) // Expect that suggestion with ResumePolicy = NeverResume is succeeded - // UpdateSuggestionStatus is executing with suggestionRestartNo + // UpdateSuggestionStatus with restartNoCall call g.Eventually(func() bool { suggestion := &suggestionsv1beta1.Suggestion{} c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) return suggestion.IsSucceeded() }, timeout).Should(gomega.BeTrue()) - // Delete the suggestion object with ResumePolicy = NeverResume - g.Expect(c.Delete(context.TODO(), suggestionRestartNo)).NotTo(gomega.HaveOccurred()) - // Expect that suggestion is deleted + // Expect that suggestion with ResumePolicy = NeverResume is deleted g.Eventually(func() bool { + // Delete the suggestion + c.Delete(context.TODO(), suggestionRestartNo) return errors.IsNotFound(c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, &suggestionsv1beta1.Suggestion{})) }, timeout).Should(gomega.BeTrue()) - // Create the suggestion object with ResumePolicy = FromVolume + // Create the suggestion with ResumePolicy = FromVolume g.Expect(c.Create(context.TODO(), suggestionRestartYes)).NotTo(gomega.HaveOccurred()) // Expect that suggestion is created g.Eventually(func() bool { @@ -231,57 +301,54 @@ func TestReconcile(t *testing.T) { }, timeout).ShouldNot(gomega.BeTrue()) // Manually update suggestion ResumePolicy to FromVolume and mark experiment succeeded to test resume experiment. - // Expect that suggestion is updated + // Expect that suggestion spec is updated. g.Eventually(func() bool { - experiment = &experimentsv1beta1.Experiment{} + experiment := &experimentsv1beta1.Experiment{} // Update ResumePolicy and maxTrialCount for resume c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, experiment) experiment.Spec.ResumePolicy = experimentsv1beta1.FromVolume var max int32 = 5 experiment.Spec.MaxTrialCount = &max errUpdate := c.Update(context.TODO(), experiment) - // Update status - c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, experiment) - experiment.MarkExperimentStatusSucceeded(experimentUtil.ExperimentMaxTrialsReachedReason, "Experiment is succeeded") - errStatus := c.Status().Update(context.TODO(), experiment) - return errUpdate == nil && errStatus == nil + return errUpdate == nil }, timeout).Should(gomega.BeTrue()) - // Expect that suggestion with ResumePolicy = FromVolume is succeeded - // UpdateSuggestionStatus is executing with suggestionRestartYes + // Expect that experiment status is updated g.Eventually(func() bool { - suggestion := &suggestionsv1beta1.Suggestion{} - c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, suggestion) - return suggestion.IsSucceeded() + experiment := &experimentsv1beta1.Experiment{} + // Update status to succeeded + c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, experiment) + experiment.MarkExperimentStatusSucceeded(experimentUtil.ExperimentMaxTrialsReachedReason, "Experiment is succeeded") + errStatus := c.Status().Update(context.TODO(), experiment) + return errStatus == nil }, timeout).Should(gomega.BeTrue()) // Expect that experiment with FromVolume is restarting. // Experiment should be not succeeded and not failed. - // UpdateSuggestionStatus is executing with suggestionRestarting + // UpdateSuggestionStatus with restartYesCall call and UpdateSuggestionStatus with experimentRestartingCall call. g.Eventually(func() bool { experiment := &experimentsv1beta1.Experiment{} c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, experiment) return experiment.IsRestarting() && !experiment.IsSucceeded() && !experiment.IsFailed() }, timeout).Should(gomega.BeTrue()) - // Delete the suggestion object with ResumePolicy = FromVolume - g.Expect(c.Delete(context.TODO(), suggestionRestartYes)).NotTo(gomega.HaveOccurred()) - // Expect that suggestion is deleted + // Expect that suggestion with ResumePolicy = FromVolume is deleted g.Eventually(func() bool { + // Delete the suggestion + c.Delete(context.TODO(), suggestionRestartYes) return errors.IsNotFound(c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, &suggestionsv1beta1.Suggestion{})) }, timeout).Should(gomega.BeTrue()) - // Delete the experiment object - g.Expect(c.Delete(context.TODO(), instance)).NotTo(gomega.HaveOccurred()) - // Expect that experiment is deleted g.Eventually(func() bool { + // Delete the experiment + c.Delete(context.TODO(), instance) return errors.IsNotFound(c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: experimentName}, &experimentsv1beta1.Experiment{})) }, timeout).Should(gomega.BeTrue()) - // Test 2 - Update status for empty experiment object + // Test 2 - Update status for empty experiment g.Expect(r.updateStatus(&experimentsv1beta1.Experiment{})).To(gomega.HaveOccurred()) // Test 3 - Cleanup suggestion resources without deployed suggestion diff --git a/pkg/controller.v1beta1/experiment/experiment_util.go b/pkg/controller.v1beta1/experiment/experiment_util.go index 410a37f880f..2ecf431a06b 100644 --- a/pkg/controller.v1beta1/experiment/experiment_util.go +++ b/pkg/controller.v1beta1/experiment/experiment_util.go @@ -115,8 +115,8 @@ func (r *ReconcileExperiment) cleanupSuggestionResources(instance *experimentsv1 return err } - // If Suggestion is failed or Suggestion is Succeeded, not needed to terminate Suggestion - if original.IsFailed() || original.IsSucceeded() { + // If Suggestion is completed or Suggestion is restarting not needed to terminate Suggestion + if original.IsCompleted() || original.IsRestarting() { return nil } @@ -154,13 +154,16 @@ func (r *ReconcileExperiment) restartSuggestion(instance *experimentsv1beta1.Exp } return err } + // If Suggestion is restarting not needed to restart Suggestion + if original.IsRestarting() { + return nil + } logger.Info("Suggestion is restarting, suggestion Running status is false") suggestion := original.DeepCopy() - reason := "Experiment is restarting" msg := "Suggestion is not running" // Mark suggestion status not running because experiment is restarting and suggestion deployment is not ready - suggestion.MarkSuggestionStatusRunning(corev1.ConditionFalse, reason, msg) + suggestion.MarkSuggestionStatusRunning(corev1.ConditionFalse, suggestionsv1beta1.SuggestionRestartReason, msg) if err := r.UpdateSuggestionStatus(suggestion); err != nil { return err diff --git a/pkg/controller.v1beta1/suggestion/suggestion_controller.go b/pkg/controller.v1beta1/suggestion/suggestion_controller.go index 7e955cbce44..340b58b706b 100644 --- a/pkg/controller.v1beta1/suggestion/suggestion_controller.go +++ b/pkg/controller.v1beta1/suggestion/suggestion_controller.go @@ -256,7 +256,8 @@ func (r *ReconcileSuggestion) ReconcileSuggestion(instance *suggestionsv1beta1.S msg := "Suggestion is running" instance.MarkSuggestionStatusRunning(corev1.ConditionTrue, SuggestionRunningReason, msg) } - logger.Info("Sync assignments", "suggestions", instance.Spec.Requests) + logger.Info("Sync assignments", "Suggestion Requests", instance.Spec.Requests, + "Suggestion Count", instance.Status.SuggestionCount) if err = r.SyncAssignments(instance, experiment, trials.Items); err != nil { return err } diff --git a/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go b/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go index 46300eeef22..c52cdb43478 100644 --- a/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go +++ b/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go @@ -80,6 +80,7 @@ func TestReconcile(t *testing.T) { scheme: mgr.GetScheme(), SuggestionClient: mockSuggestionClient, Composer: composer.New(mgr), + recorder: mgr.GetRecorder(ControllerName), } recFn := SetupTestReconcile(r) @@ -127,13 +128,13 @@ func TestReconcile(t *testing.T) { configMap := newKatibConfigMapInstance() // Test 1 - Regural suggestion run - // Create the suggestion object and expect the service, deployment, pvc and pv is created + // Create the suggestion and expect the service, deployment, pvc and pv is created g.Expect(c.Create(context.TODO(), instance)).NotTo(gomega.HaveOccurred()) // Create ConfigMap with suggestion data g.Expect(c.Create(context.TODO(), configMap)).NotTo(gomega.HaveOccurred()) - // Create experiment object + // Create experiment g.Expect(c.Create(context.TODO(), experiment)).NotTo(gomega.HaveOccurred()) - // Create trial object + // Create trial g.Expect(c.Create(context.TODO(), trial)).NotTo(gomega.HaveOccurred()) suggestionDeploy := &appsv1.Deployment{} @@ -195,11 +196,10 @@ func TestReconcile(t *testing.T) { errors.IsNotFound(c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: resourceName}, &corev1.Service{})) }, timeout).Should(gomega.BeTrue()) - // Delete the suggestion object - g.Expect(c.Delete(context.TODO(), instance)).NotTo(gomega.HaveOccurred()) - // Expect that suggestion is deleted g.Eventually(func() bool { + // Delete the suggestion + c.Delete(context.TODO(), instance) return errors.IsNotFound(c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: suggestionName}, &suggestionsv1beta1.Suggestion{})) }, timeout).Should(gomega.BeTrue()) @@ -231,13 +231,13 @@ func TestReconcile(t *testing.T) { }, } - // Test 2 - Update status for empty experiment object + // Test 2 - Update status for empty experiment g.Expect(r.updateStatus(&suggestionsv1beta1.Suggestion{}, oldS)).To(gomega.HaveOccurred()) // Test 3 - Update status condition g.Expect(r.updateStatusCondition(newS, oldS)).NotTo(gomega.HaveOccurred()) - // Test 4 - Update status condition for empty experiment object + // Test 4 - Update status condition for empty experiment g.Expect(r.updateStatusCondition(&suggestionsv1beta1.Suggestion{}, oldS)).To(gomega.HaveOccurred()) } diff --git a/test/e2e/v1beta1/resume-e2e-experiment.go b/test/e2e/v1beta1/resume-e2e-experiment.go index 2ca100aa8b7..85054b27574 100644 --- a/test/e2e/v1beta1/resume-e2e-experiment.go +++ b/test/e2e/v1beta1/resume-e2e-experiment.go @@ -90,7 +90,7 @@ func main() { if err != nil { log.Fatal("Get Experiment error ", err) } - if exp.IsRunning() && exp.Status.Trials == maxtrials { + if exp.IsRunning() { log.Printf("Experiment %v started running with %v MaxTrialCount", exp.Name, maxtrials) break } diff --git a/test/e2e/v1beta1/run-e2e-experiment.go b/test/e2e/v1beta1/run-e2e-experiment.go index bb4877608f5..0455eae8e5e 100644 --- a/test/e2e/v1beta1/run-e2e-experiment.go +++ b/test/e2e/v1beta1/run-e2e-experiment.go @@ -158,17 +158,19 @@ func main() { namespacedName := types.NamespacedName{Name: controllerUtil.GetAlgorithmServiceName(sug), Namespace: sug.Namespace} err := kclient.GetClient().Get(context.TODO(), namespacedName, &corev1.Service{}) - if err == nil || !errors.IsNotFound(err) { + if errors.IsNotFound(err) { + log.Printf("Suggestion service %v has been deleted", controllerUtil.GetAlgorithmServiceName(sug)) + } else { log.Fatalf("Suggestion service is still alive while ResumePolicy = %v", exp.Spec.ResumePolicy) } - log.Printf("Suggestion service %v has been deleted", controllerUtil.GetAlgorithmServiceName(sug)) namespacedName = types.NamespacedName{Name: controllerUtil.GetAlgorithmDeploymentName(sug), Namespace: sug.Namespace} err = kclient.GetClient().Get(context.TODO(), namespacedName, &appsv1.Deployment{}) - if err == nil || !errors.IsNotFound(err) { + if errors.IsNotFound(err) { + log.Printf("Suggestion deployment %v has been deleted", controllerUtil.GetAlgorithmDeploymentName(sug)) + } else { log.Fatalf("Suggestion deployment is still alive while ResumePolicy = %v", exp.Spec.ResumePolicy) } - log.Printf("Suggestion deployment %v has been deleted", controllerUtil.GetAlgorithmDeploymentName(sug)) if exp.Spec.ResumePolicy == experimentsv1beta1.FromVolume { namespacedName = types.NamespacedName{Name: controllerUtil.GetAlgorithmPersistentVolumeClaimName(sug), Namespace: sug.Namespace} diff --git a/test/scripts/v1beta1/run-from-volume.sh b/test/scripts/v1beta1/run-from-volume.sh index 4a1f52de4ec..bbd543a65a5 100755 --- a/test/scripts/v1beta1/run-from-volume.sh +++ b/test/scripts/v1beta1/run-from-volume.sh @@ -62,6 +62,10 @@ export KUBECONFIG=$HOME/.kube/config kubectl -n kubeflow describe suggestion from-volume-resume kubectl -n kubeflow describe experiment from-volume-resume +echo "Available volumes" +kubectl get pvc -n kubeflow +kubectl get pv + echo "Resuming the completed experiment with resume from volume" ./resume-e2e-experiment ../../../examples/v1beta1/resume-experiment/from-volume-resume.yaml