diff --git a/environment/kubernetes/power.go b/environment/kubernetes/power.go index 7576d8c2..69d4e3a6 100644 --- a/environment/kubernetes/power.go +++ b/environment/kubernetes/power.go @@ -117,6 +117,23 @@ func (e *Environment) Stop(ctx context.Context) error { // terminate. If the timeout is reached and terminate is true, the Pod is // forcefully deleted. func (e *Environment) WaitForStop(ctx context.Context, duration time.Duration, terminate bool) error { + // If the Pod is already gone, or exists but is no longer running, the server + // is effectively stopped and there is nothing to wait for. A stopped Pod is + // not automatically removed, so proceeding into waitForPodDeletion would block + // for the full duration (holding the power lock) waiting for a deletion that + // never happens. This is what gets hit when a stop/restart is issued against a + // server that is already offline. + if pod, err := e.getPod(ctx); err != nil { + if isNotFound(err) { + e.markOffline() + return nil + } + // Fall through on unexpected errors and attempt the normal stop flow. + } else if !isPodRunning(pod) { + e.markOffline() + return nil + } + tctx, cancel := context.WithTimeout(context.Background(), duration) defer cancel() @@ -161,10 +178,7 @@ func (e *Environment) Terminate(ctx context.Context, signal string) error { } if !isPodRunning(pod) { - if e.st.Load() != environment.ProcessOfflineState { - e.SetState(environment.ProcessStoppingState) - e.SetState(environment.ProcessOfflineState) - } + e.markOffline() return nil } @@ -182,6 +196,16 @@ func (e *Environment) Terminate(ctx context.Context, signal string) error { return nil } +// markOffline transitions the environment to the offline state, first passing +// through the stopping state so that crash detection is not triggered. It is a +// no-op if the environment already considers itself offline. +func (e *Environment) markOffline() { + if e.st.Load() != environment.ProcessOfflineState { + e.SetState(environment.ProcessStoppingState) + e.SetState(environment.ProcessOfflineState) + } +} + // waitForPodRunning blocks until the Pod reaches Running phase or the context // is canceled. func (e *Environment) waitForPodRunning(ctx context.Context) error { diff --git a/environment/kubernetes/power_test.go b/environment/kubernetes/power_test.go new file mode 100644 index 00000000..115c1e73 --- /dev/null +++ b/environment/kubernetes/power_test.go @@ -0,0 +1,77 @@ +package kubernetes + +import ( + "context" + "testing" + "time" + + . "github.com/franela/goblin" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + + "github.com/exonical/wings/config" + "github.com/exonical/wings/environment" + "github.com/exonical/wings/events" + "github.com/exonical/wings/system" +) + +func TestWaitForStop(t *testing.T) { + g := Goblin(t) + + config.Update(func(c *config.Configuration) { + c.Kubernetes.Namespace = "pelican" + }) + + newEnv := func(state string, objects ...runtime.Object) *Environment { + return &Environment{ + Id: "test-uuid", + meta: &Metadata{}, + client: fake.NewSimpleClientset(objects...), + st: system.NewAtomicString(state), + emitter: events.NewBus(), + } + } + + g.Describe("WaitForStop when the server is already offline", func() { + g.It("returns immediately when the Pod does not exist", func() { + env := newEnv(environment.ProcessOfflineState) + + done := make(chan error, 1) + go func() { + // A long duration would previously block here for the full period. + done <- env.WaitForStop(context.Background(), time.Minute, true) + }() + + select { + case err := <-done: + g.Assert(err).IsNil() + case <-time.After(2 * time.Second): + g.Fail("WaitForStop blocked when the Pod was absent") + } + }) + + g.It("returns immediately when the Pod exists but is not running", func() { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "test-uuid", Namespace: "pelican"}, + Status: corev1.PodStatus{Phase: corev1.PodSucceeded}, + } + // Start from a non-offline state to prove the helper drives it offline. + env := newEnv(environment.ProcessRunningState, pod) + + done := make(chan error, 1) + go func() { + done <- env.WaitForStop(context.Background(), time.Minute, true) + }() + + select { + case err := <-done: + g.Assert(err).IsNil() + g.Assert(env.State()).Equal(environment.ProcessOfflineState) + case <-time.After(2 * time.Second): + g.Fail("WaitForStop blocked when the Pod was not running") + } + }) + }) +}