Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Flaky: Fix test for TestGameServerUnhealthyAfterDeletingPod #758

Merged
merged 2 commits into from
May 7, 2019
Merged

Flaky: Fix test for TestGameServerUnhealthyAfterDeletingPod #758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50f4fa7
Flaky: Fix test for TestGameServerUnhealthyAfterDeletingPod
markmandel May 6, 2019
b7c2492
Merge branch 'master' into flaky/TestGameServerUnhealthyAfterDeletingPod
markmandel May 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions test/e2e/framework/framework.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func (f *Framework) WaitForGameServerState(gs *v1alpha1.GameServer, state v1alph
readyGs, pollErr = f.AgonesClient.StableV1alpha1().GameServers(gs.Namespace).Get(gs.Name, metav1.GetOptions{})

if pollErr != nil {
logrus.WithError(pollErr).Warn("error retrieving gameserver")
return false, nil
}

Expand Down
25 changes: 24 additions & 1 deletion test/e2e/gameserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"agones.dev/agones/pkg/apis/stable"
"agones.dev/agones/pkg/apis/stable/v1alpha1"
e2eframework "agones.dev/agones/test/e2e/framework"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -192,6 +193,8 @@ func TestGameServerUnhealthyAfterDeletingPod(t *testing.T) {
t.Fatalf("Could not get a GameServer ready: %v", err)
}

logrus.WithField("gsKey", readyGs.ObjectMeta.Name).Info("GameServer Ready")

gsClient := framework.AgonesClient.StableV1alpha1().GameServers(defaultNs)
podClient := framework.KubeClient.CoreV1().Pods(defaultNs)

Expand All @@ -205,7 +208,27 @@ func TestGameServerUnhealthyAfterDeletingPod(t *testing.T) {
err = podClient.Delete(pod.ObjectMeta.Name, nil)
assert.NoError(t, err)

_, err = framework.WaitForGameServerState(readyGs, v1alpha1.GameServerStateUnhealthy, 10*time.Second)
// TODO [markmandel@google.com]: Should GameServers that are Unhealthy and not in a fleet be deleted?
err = wait.PollImmediate(2*time.Second, time.Minute, func() (bool, error) {
gs, err := framework.AgonesClient.StableV1alpha1().GameServers(readyGs.Namespace).Get(readyGs.ObjectMeta.Name, metav1.GetOptions{})

// just in case
if k8serrors.IsNotFound(err) {
return true, nil
}

if err != nil {
logrus.WithError(err).Warn("error retrieving gameserver")
return false, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be return false, err? The func returns (bool, error) but none of the cases actually return a non-nil error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question - looking at the code, if any error is returned from the function, then PollImmeadiate will fail, and return an error - in this case, I don't think we actually want that to happen, we want it to retry and try again if something weird happens - because it may be a timing issue or self resolve?

}

if gs.Status.State == v1alpha1.GameServerStateUnhealthy {
return true, nil
}

return false, nil
})

assert.NoError(t, err)
}

Expand Down