From cdbd0f2ae0c2bc06bb56d8e3e4fd4c0b97f3cbfa Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Wed, 25 Nov 2020 17:07:23 -0800 Subject: [PATCH 1/7] WIP fixes for #102 --- pkg/apis/pulumi/v1alpha1/stack_types.go | 16 ++++++++-- pkg/controller/stack/stack_controller.go | 40 +++++++----------------- test/stack_controller_test.go | 6 ++-- 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/pkg/apis/pulumi/v1alpha1/stack_types.go b/pkg/apis/pulumi/v1alpha1/stack_types.go index 8f838e08..2b66cce0 100644 --- a/pkg/apis/pulumi/v1alpha1/stack_types.go +++ b/pkg/apis/pulumi/v1alpha1/stack_types.go @@ -107,7 +107,11 @@ type StackOutputs map[string]apiextensionsv1.JSON // StackUpdateState is the status of a stack update type StackUpdateState struct { // State is the state of the stack update - one of `succeeded` or `failed` - State string `json:"state,omitempty"` + State StackUpdateStateMessage `json:"state,omitempty"` + // Last commit attempted + LastAttemptedCommit string `json:"lastAttemptedCommit,omitempty"` + // Last commit successfully applied + LastSuccessfulCommit string `json:"lastSuccessfulCommit,omitempty"` // Permalink is the Pulumi Console URL of the stack operation. Permalink Permalink `json:"permalink,omitempty"` } @@ -153,8 +157,14 @@ const ( StackNotFound StackUpdateStatus = 4 ) -// FailedStackStateMessage is a const to indicate stack failure in the status. -const FailedStackStateMessage = "failed" +type StackUpdateStateMessage string + +const ( + // SucceededStackStateMessage is a const to indicate success in stack status state. + SucceededStackStateMessage StackUpdateStateMessage = "succeeded" + // FailedStackStateMessage is a const to indicate stack failure in stack status state. + FailedStackStateMessage StackUpdateStateMessage = "failed" +) // Permalink is the Pulumi Service URL of the stack operation. type Permalink string diff --git a/pkg/controller/stack/stack_controller.go b/pkg/controller/stack/stack_controller.go index 9de8146f..47aff03f 100644 --- a/pkg/controller/stack/stack_controller.go +++ b/pkg/controller/stack/stack_controller.go @@ -210,20 +210,6 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, } } - // Run the Pulumi update iff the desired state has not already been - // reached, or if it is marked to be deleted. - err = sess.getLatestResource(instance, request.NamespacedName) - if err != nil { - sess.logger.Error(err, "Failed to get latest Stack before updating Stack", "Stack.Name", instance.Spec.Stack) - return reconcile.Result{}, err - } - isStackMarkedToBeDeleted = instance.GetDeletionTimestamp() != nil - // Don't run rest of loop if already at desired state, unless marked for deletion. - if !isStackMarkedToBeDeleted && (instance.Status.LastUpdate != nil && instance.Status.LastUpdate.State == instance.Spec.Commit) { - reqLogger.Info("Stack already at desired state", "Stack.Commit", instance.Spec.Commit) - return reconcile.Result{}, nil - } - // Step 3. If a stack refresh is requested, run it now. if sess.stack.Refresh { permalink, err := sess.RefreshStack(sess.stack.ExpectNoRefreshChanges) @@ -237,12 +223,10 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, return reconcile.Result{}, err } if instance.Status.LastUpdate == nil { - instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ - Permalink: permalink, - } - } else { - instance.Status.LastUpdate.Permalink = permalink + instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{} } + instance.Status.LastUpdate.Permalink = permalink + err = sess.updateResourceStatus(instance) if err != nil { reqLogger.Error(err, "Failed to update Stack status for refresh", "Stack.Name", stack.Stack) @@ -270,8 +254,9 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, reqLogger.Error(err, "Failed to update Stack", "Stack.Name", stack.Stack) // Update Stack status with failed state instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ - State: pulumiv1alpha1.FailedStackStateMessage, - Permalink: permalink, + LastAttemptedCommit: instance.Spec.Commit, + State: pulumiv1alpha1.FailedStackStateMessage, + Permalink: permalink, } if err2 := sess.updateResourceStatus(instance); err2 != nil { msg := "Failed to update status for a failed Stack update" @@ -299,14 +284,11 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, return reconcile.Result{}, err } instance.Status.Outputs = outs - if instance.Status.LastUpdate == nil { - instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ - State: instance.Spec.Commit, - Permalink: permalink, - } - } else { - instance.Status.LastUpdate.State = instance.Spec.Commit - instance.Status.LastUpdate.Permalink = permalink + instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ + State: pulumiv1alpha1.SucceededStackStateMessage, + LastAttemptedCommit: instance.Spec.Commit, + LastSuccessfulCommit: instance.Spec.Commit, + Permalink: permalink, } err = sess.updateResourceStatus(instance) if err != nil { diff --git a/test/stack_controller_test.go b/test/stack_controller_test.go index d47738f8..6be741ea 100644 --- a/test/stack_controller_test.go +++ b/test/stack_controller_test.go @@ -157,7 +157,7 @@ var _ = Describe("Stack Controller", func() { return false } if fetched.Status.LastUpdate != nil { - return fetched.Status.LastUpdate.State == stack.Spec.Commit + return fetched.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit } return false }, timeout, interval).Should(BeTrue()) @@ -207,7 +207,7 @@ var _ = Describe("Stack Controller", func() { return false } if original.Status.LastUpdate != nil { - return original.Status.LastUpdate.State == stack.Spec.Commit + return original.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit } return false }, timeout, interval).Should(BeTrue()) @@ -224,7 +224,7 @@ var _ = Describe("Stack Controller", func() { return false } if fetched.Status.LastUpdate != nil { - return fetched.Status.LastUpdate.State == commit + return fetched.Status.LastUpdate.LastSuccessfulCommit == commit } return false }, timeout, interval).Should(BeTrue()) From 50459f6182bfe1cfa45febb9a499eaf9f9f7b181 Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Sun, 29 Nov 2020 22:24:20 -0800 Subject: [PATCH 2/7] Update CRD and avoid clobbering last successful commit --- deploy/crds/pulumi.com_stacks.yaml | 6 ++++++ pkg/controller/stack/stack_controller.go | 9 ++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/deploy/crds/pulumi.com_stacks.yaml b/deploy/crds/pulumi.com_stacks.yaml index a0038065..37125118 100644 --- a/deploy/crds/pulumi.com_stacks.yaml +++ b/deploy/crds/pulumi.com_stacks.yaml @@ -101,6 +101,12 @@ spec: lastUpdate: description: LastUpdate contains details of the status of the last update. properties: + lastAttemptedCommit: + description: Last commit attempted + type: string + lastSuccessfulCommit: + description: Last commit successfully applied + type: string permalink: description: Permalink is the Pulumi Console URL of the stack operation. type: string diff --git a/pkg/controller/stack/stack_controller.go b/pkg/controller/stack/stack_controller.go index 47aff03f..de7c25f5 100644 --- a/pkg/controller/stack/stack_controller.go +++ b/pkg/controller/stack/stack_controller.go @@ -253,11 +253,10 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, if err != nil { reqLogger.Error(err, "Failed to update Stack", "Stack.Name", stack.Stack) // Update Stack status with failed state - instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ - LastAttemptedCommit: instance.Spec.Commit, - State: pulumiv1alpha1.FailedStackStateMessage, - Permalink: permalink, - } + instance.Status.LastUpdate.LastAttemptedCommit = instance.Spec.Commit + instance.Status.LastUpdate.State = pulumiv1alpha1.FailedStackStateMessage + instance.Status.LastUpdate.Permalink = permalink + if err2 := sess.updateResourceStatus(instance); err2 != nil { msg := "Failed to update status for a failed Stack update" err3 := errors.Wrapf(err, err2.Error()) From f5d54b90e80e34593df920044c63e6f5429e505a Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Mon, 30 Nov 2020 18:57:11 -0800 Subject: [PATCH 3/7] Record actual git commit instead of using spec value --- go.sum | 2 - pkg/controller/stack/stack_controller.go | 61 ++++++++++++------------ 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/go.sum b/go.sum index 38203cef..49391fc5 100644 --- a/go.sum +++ b/go.sum @@ -789,8 +789,6 @@ github.com/prometheus/prometheus v0.0.0-20180315085919-58e2a31db8de/go.mod h1:oA github.com/prometheus/prometheus v1.8.2-0.20200110114423-1e64d757f711/go.mod h1:7U90zPoLkWjEIQcy/rweQla82OCTUzxVHE51G3OhJbI= github.com/prometheus/prometheus v2.3.2+incompatible/go.mod h1:oAIUtOny2rjMX0OWN5vPR5/q/twIROJvdqnQKDdil/s= github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= -github.com/pulumi/pulumi/sdk/v2 v2.12.1 h1:rEQHyjaGSGybqqeKLMJZH034UemMPGw2AWzpGcn1tF4= -github.com/pulumi/pulumi/sdk/v2 v2.12.1/go.mod h1:WQ4WaHMA7mduVHAJi87iIqbWvqsuBUYccBiKK+FrayI= github.com/pulumi/pulumi/sdk/v2 v2.15.0 h1:gTiohXl5dyw3z/aKbuhrN50KQMYFFKnGwebPWvOIvs8= github.com/pulumi/pulumi/sdk/v2 v2.15.0/go.mod h1:Z9ifPo/Q0+hUpAyguVx2gp5Sx+CBumnWvYQDhrM8l3E= github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= diff --git a/pkg/controller/stack/stack_controller.go b/pkg/controller/stack/stack_controller.go index de7c25f5..908be3e6 100644 --- a/pkg/controller/stack/stack_controller.go +++ b/pkg/controller/stack/stack_controller.go @@ -25,7 +25,6 @@ import ( "github.com/pulumi/pulumi/sdk/v2/go/x/auto/optup" giturls "github.com/whilp/git-urls" git "gopkg.in/src-d/go-git.v4" - "gopkg.in/src-d/go-git.v4/plumbing" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -206,7 +205,9 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, } time.Sleep(2 * time.Second) // arbitrary sleep after finalizer add to avoid stale obj for permalink // Add default permalink for the stack in the Pulumi Service. - sess.addDefaultPermalink(instance) + if err := sess.addDefaultPermalink(instance); err != nil { + return reconcile.Result{}, err + } } } @@ -253,7 +254,7 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, if err != nil { reqLogger.Error(err, "Failed to update Stack", "Stack.Name", stack.Stack) // Update Stack status with failed state - instance.Status.LastUpdate.LastAttemptedCommit = instance.Spec.Commit + instance.Status.LastUpdate.LastAttemptedCommit = sess.currentCommit instance.Status.LastUpdate.State = pulumiv1alpha1.FailedStackStateMessage instance.Status.LastUpdate.Permalink = permalink @@ -285,8 +286,8 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, instance.Status.Outputs = outs instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ State: pulumiv1alpha1.SucceededStackStateMessage, - LastAttemptedCommit: instance.Spec.Commit, - LastSuccessfulCommit: instance.Spec.Commit, + LastAttemptedCommit: sess.currentCommit, + LastSuccessfulCommit: sess.currentCommit, Permalink: permalink, } err = sess.updateResourceStatus(instance) @@ -358,12 +359,13 @@ func (sess *reconcileStackSession) addFinalizer(stack *pulumiv1alpha1.Stack) err } type reconcileStackSession struct { - logger logr.Logger - kubeClient client.Client - accessToken string - stack pulumiv1alpha1.StackSpec - autoStack *auto.Stack - workdir string + logger logr.Logger + kubeClient client.Client + accessToken string + stack pulumiv1alpha1.StackSpec + autoStack *auto.Stack + workdir string + currentCommit string } // blank assignment to verify that reconcileStackSession implements pulumiv1alpha1.StackController. @@ -380,25 +382,6 @@ func newReconcileStackSession( } } -// gitCloneAndCheckoutCommit clones the Git repository and checkouts the specified commit hash or branch. -func gitCloneAndCheckoutCommit(url, hash, branch, path string) error { - repo, err := git.PlainClone(path, false, &git.CloneOptions{URL: url}) - if err != nil { - return err - } - - w, err := repo.Worktree() - if err != nil { - return err - } - - return w.Checkout(&git.CheckoutOptions{ - Hash: plumbing.NewHash(hash), - Branch: plumbing.ReferenceName(branch), - Force: true, - }) -} - // SetEnvs populates the environment the stack run with values // from an array of Kubernetes ConfigMaps in a Namespace. func (sess *reconcileStackSession) SetEnvs(configMapNames []string, namespace string) error { @@ -512,6 +495,10 @@ func (sess *reconcileStackSession) SetupPulumiWorkdir(gitAuth *auto.GitAuth) err if sess.accessToken != "" { w.SetEnvVar("PULUMI_ACCESS_TOKEN", sess.accessToken) } + sess.workdir = w.WorkDir() + if err = sess.updateGitRevisionAtWorkingDir(); err != nil { + return err + } // Create a new stack if the stack does not already exist, or fall back to // selecting the existing stack. If the stack does not exist, it will be created and selected. @@ -536,6 +523,20 @@ func (sess *reconcileStackSession) SetupPulumiWorkdir(gitAuth *auto.GitAuth) err return nil } +// Determine the actual commit information from the working directory (Spec commit etc. is optional). +func (sess *reconcileStackSession) updateGitRevisionAtWorkingDir() error { + gitRepo, err := git.PlainOpen(sess.workdir) + if err != nil { + return errors.Wrapf(err, "failed to resolve git repository from working directory: %s", sess.workdir) + } + headRef, err := gitRepo.Head() + if err != nil { + return errors.Wrapf(err, "failed to determine revision for git repository at %s", sess.workdir) + } + sess.currentCommit = headRef.Hash().String() + return nil +} + func (sess *reconcileStackSession) InstallProjectDependencies(ctx context.Context, workspace auto.Workspace) error { project, err := workspace.ProjectSettings(ctx) if err != nil { From 9dd97fe9b225d2dee7bc8f28ecdd0bfe1720ed8f Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Mon, 30 Nov 2020 19:42:44 -0800 Subject: [PATCH 4/7] Delete working directory after reconciliation - should help with #104 --- pkg/controller/stack/stack_controller.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/controller/stack/stack_controller.go b/pkg/controller/stack/stack_controller.go index 908be3e6..bff5e769 100644 --- a/pkg/controller/stack/stack_controller.go +++ b/pkg/controller/stack/stack_controller.go @@ -173,6 +173,15 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, return reconcile.Result{}, err } + // Delete the working directory after the reconciliation is completed (regardless of success or failure). + defer func() { + if sess.workdir != "" { + if err := os.RemoveAll(sess.workdir); err != nil { + sess.logger.Error(err, "Failed to delete working dir: %s", sess.workdir) + } + } + }() + // Step 2. If there are extra environment variables, read them in now and use them for subsequent commands. err = sess.SetEnvs(stack.Envs, request.Namespace) if err != nil { From 89aead3aca6d6ce9f575554975039bcf7a0cea10 Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Wed, 2 Dec 2020 20:45:05 -0800 Subject: [PATCH 5/7] Address PR comments --- pkg/controller/stack/stack_controller.go | 42 +++++++++++++----------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/pkg/controller/stack/stack_controller.go b/pkg/controller/stack/stack_controller.go index bff5e769..6c04616d 100644 --- a/pkg/controller/stack/stack_controller.go +++ b/pkg/controller/stack/stack_controller.go @@ -173,14 +173,13 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, return reconcile.Result{}, err } + currentCommit, err := revisionAtWorkingDir(sess.workdir) + if err != nil { + return reconcile.Result{}, err + } + // Delete the working directory after the reconciliation is completed (regardless of success or failure). - defer func() { - if sess.workdir != "" { - if err := os.RemoveAll(sess.workdir); err != nil { - sess.logger.Error(err, "Failed to delete working dir: %s", sess.workdir) - } - } - }() + defer sess.CleanupPulumiWorkdir() // Step 2. If there are extra environment variables, read them in now and use them for subsequent commands. err = sess.SetEnvs(stack.Envs, request.Namespace) @@ -263,7 +262,7 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, if err != nil { reqLogger.Error(err, "Failed to update Stack", "Stack.Name", stack.Stack) // Update Stack status with failed state - instance.Status.LastUpdate.LastAttemptedCommit = sess.currentCommit + instance.Status.LastUpdate.LastAttemptedCommit = currentCommit instance.Status.LastUpdate.State = pulumiv1alpha1.FailedStackStateMessage instance.Status.LastUpdate.Permalink = permalink @@ -295,8 +294,8 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result, instance.Status.Outputs = outs instance.Status.LastUpdate = &pulumiv1alpha1.StackUpdateState{ State: pulumiv1alpha1.SucceededStackStateMessage, - LastAttemptedCommit: sess.currentCommit, - LastSuccessfulCommit: sess.currentCommit, + LastAttemptedCommit: currentCommit, + LastSuccessfulCommit: currentCommit, Permalink: permalink, } err = sess.updateResourceStatus(instance) @@ -374,7 +373,6 @@ type reconcileStackSession struct { stack pulumiv1alpha1.StackSpec autoStack *auto.Stack workdir string - currentCommit string } // blank assignment to verify that reconcileStackSession implements pulumiv1alpha1.StackController. @@ -505,9 +503,6 @@ func (sess *reconcileStackSession) SetupPulumiWorkdir(gitAuth *auto.GitAuth) err w.SetEnvVar("PULUMI_ACCESS_TOKEN", sess.accessToken) } sess.workdir = w.WorkDir() - if err = sess.updateGitRevisionAtWorkingDir(); err != nil { - return err - } // Create a new stack if the stack does not already exist, or fall back to // selecting the existing stack. If the stack does not exist, it will be created and selected. @@ -532,18 +527,25 @@ func (sess *reconcileStackSession) SetupPulumiWorkdir(gitAuth *auto.GitAuth) err return nil } +func (sess *reconcileStackSession) CleanupPulumiWorkdir() { + if sess.workdir != "" { + if err := os.RemoveAll(sess.workdir); err != nil { + sess.logger.Error(err, "Failed to delete working dir: %s", sess.workdir) + } + } +} + // Determine the actual commit information from the working directory (Spec commit etc. is optional). -func (sess *reconcileStackSession) updateGitRevisionAtWorkingDir() error { - gitRepo, err := git.PlainOpen(sess.workdir) +func revisionAtWorkingDir(workingDir string) (string, error) { + gitRepo, err := git.PlainOpen(workingDir) if err != nil { - return errors.Wrapf(err, "failed to resolve git repository from working directory: %s", sess.workdir) + return "", errors.Wrapf(err, "failed to resolve git repository from working directory: %s", workingDir) } headRef, err := gitRepo.Head() if err != nil { - return errors.Wrapf(err, "failed to determine revision for git repository at %s", sess.workdir) + return "", errors.Wrapf(err, "failed to determine revision for git repository at %s", workingDir) } - sess.currentCommit = headRef.Hash().String() - return nil + return headRef.Hash().String(), nil } func (sess *reconcileStackSession) InstallProjectDependencies(ctx context.Context, workspace auto.Workspace) error { From fc5739e447f09c65908af3115a93fb0dbfd3f096 Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Wed, 2 Dec 2020 20:45:31 -0800 Subject: [PATCH 6/7] Improve tests --- test/stack_controller_test.go | 56 +++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/test/stack_controller_test.go b/test/stack_controller_test.go index 6be741ea..6e0d733d 100644 --- a/test/stack_controller_test.go +++ b/test/stack_controller_test.go @@ -157,7 +157,9 @@ var _ = Describe("Stack Controller", func() { return false } if fetched.Status.LastUpdate != nil { - return fetched.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit + return fetched.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit && + fetched.Status.LastUpdate.LastAttemptedCommit == stack.Spec.Commit && + fetched.Status.LastUpdate.State == pulumiv1alpha1.SucceededStackStateMessage } return false }, timeout, interval).Should(BeTrue()) @@ -207,16 +209,37 @@ var _ = Describe("Stack Controller", func() { return false } if original.Status.LastUpdate != nil { - return original.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit + return original.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit && + original.Status.LastUpdate.LastAttemptedCommit == stack.Spec.Commit && + original.Status.LastUpdate.State == pulumiv1alpha1.SucceededStackStateMessage } return false }, timeout, interval).Should(BeTrue()) - // Update the stack commit to a different commit. - original.Spec.Commit = commit + // Update the stack config (this time to cause a failure) + original.Spec.Config["aws:region"] = "us-nonexistent-1" Expect(k8sClient.Update(ctx, original)).Should(Succeed()) - // Check that the stack updated + // Check that the stack tried to update but failed + configChanged := &pulumiv1alpha1.Stack{} + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, configChanged) + if err != nil { + return false + } + if configChanged.Status.LastUpdate != nil { + return configChanged.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit && + configChanged.Status.LastUpdate.LastAttemptedCommit == stack.Spec.Commit && + configChanged.Status.LastUpdate.State == pulumiv1alpha1.FailedStackStateMessage + } + return false + }) + + // Update the stack commit to a different commit - should still fail. + configChanged.Spec.Commit = commit + Expect(k8sClient.Update(ctx, configChanged)).Should(Succeed()) + + // Check that the stack update attempted but failed fetched := &pulumiv1alpha1.Stack{} Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, fetched) @@ -224,7 +247,28 @@ var _ = Describe("Stack Controller", func() { return false } if fetched.Status.LastUpdate != nil { - return fetched.Status.LastUpdate.LastSuccessfulCommit == commit + return fetched.Status.LastUpdate.LastSuccessfulCommit == stack.Spec.Commit && + fetched.Status.LastUpdate.LastAttemptedCommit == commit && + fetched.Status.LastUpdate.State == pulumiv1alpha1.FailedStackStateMessage + } + return false + }, timeout, interval).Should(BeTrue()) + + // Update the stack config to now be valid + fetched.Spec.Config["aws:region"] = "us-east-2" + Expect(k8sClient.Update(ctx, fetched)).Should(Succeed()) + + // Check that the stack update attempted but failed + fetched = &pulumiv1alpha1.Stack{} + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, fetched) + if err != nil { + return false + } + if fetched.Status.LastUpdate != nil { + return fetched.Status.LastUpdate.LastSuccessfulCommit == commit && + fetched.Status.LastUpdate.LastAttemptedCommit == commit && + fetched.Status.LastUpdate.State == pulumiv1alpha1.SucceededStackStateMessage } return false }, timeout, interval).Should(BeTrue()) From 3103e59934622662a1a9a6f0f762b2a1b7ee8206 Mon Sep 17 00:00:00 2001 From: Vivek Lakshmanan Date: Wed, 2 Dec 2020 22:45:14 -0800 Subject: [PATCH 7/7] Adding more tests to cover lifecycle of stack with success and failure --- test/stack_controller_test.go | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/test/stack_controller_test.go b/test/stack_controller_test.go index 6e0d733d..855f1974 100644 --- a/test/stack_controller_test.go +++ b/test/stack_controller_test.go @@ -218,7 +218,7 @@ var _ = Describe("Stack Controller", func() { // Update the stack config (this time to cause a failure) original.Spec.Config["aws:region"] = "us-nonexistent-1" - Expect(k8sClient.Update(ctx, original)).Should(Succeed()) + Expect(k8sClient.Update(ctx, original)).Should(Succeed(), "%+v", original) // Check that the stack tried to update but failed configChanged := &pulumiv1alpha1.Stack{} @@ -235,11 +235,20 @@ var _ = Describe("Stack Controller", func() { return false }) - // Update the stack commit to a different commit - should still fail. - configChanged.Spec.Commit = commit - Expect(k8sClient.Update(ctx, configChanged)).Should(Succeed()) + // Update the stack commit to a different commit. Need retries because of + // competing retries within the operator due to failure. + Eventually(func() bool { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, configChanged); err != nil { + return false + } + configChanged.Spec.Commit = commit + if err := k8sClient.Update(ctx, configChanged); err != nil { + return false + } + return true + }, timeout, interval).Should(BeTrue(), "%#v", configChanged) - // Check that the stack update attempted but failed + // Check that the stack update was attempted but failed fetched := &pulumiv1alpha1.Stack{} Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, fetched) @@ -254,12 +263,19 @@ var _ = Describe("Stack Controller", func() { return false }, timeout, interval).Should(BeTrue()) - // Update the stack config to now be valid - fetched.Spec.Config["aws:region"] = "us-east-2" - Expect(k8sClient.Update(ctx, fetched)).Should(Succeed()) + Eventually(func() bool { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, fetched); err != nil { + return false + } + // Update the stack config to now be valid + fetched.Spec.Config["aws:region"] = "us-east-2" + if err := k8sClient.Update(ctx, fetched); err != nil { + return false + } + return true + }, timeout, interval).Should(BeTrue()) // Check that the stack update attempted but failed - fetched = &pulumiv1alpha1.Stack{} Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Name: stack.Name, Namespace: namespace}, fetched) if err != nil {