-
Notifications
You must be signed in to change notification settings - Fork 820
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.
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
E2E test for Ready->Allocated->Ready #834
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,14 +103,14 @@ func (f *Framework) CreateGameServerAndWaitUntilReady(ns string, gs *stable.Game | |
// WaitForGameServerState Waits untils the gameserver reach a given state before the timeout expires | ||
func (f *Framework) WaitForGameServerState(gs *stable.GameServer, state stable.GameServerState, | ||
timeout time.Duration) (*stable.GameServer, error) { | ||
var pollErr error | ||
var readyGs *stable.GameServer | ||
|
||
err := wait.PollImmediate(2*time.Second, timeout, func() (bool, error) { | ||
readyGs, pollErr = f.AgonesClient.StableV1alpha1().GameServers(gs.Namespace).Get(gs.Name, metav1.GetOptions{}) | ||
var err error | ||
readyGs, err = f.AgonesClient.StableV1alpha1().GameServers(gs.Namespace).Get(gs.Name, metav1.GetOptions{}) | ||
|
||
if pollErr != nil { | ||
logrus.WithError(pollErr).Warn("error retrieving gameserver") | ||
if err != nil { | ||
logrus.WithError(err).Warn("error retrieving gameserver") | ||
return false, nil | ||
} | ||
|
||
|
@@ -120,11 +120,9 @@ func (f *Framework) WaitForGameServerState(gs *stable.GameServer, state stable.G | |
|
||
return false, nil | ||
}) | ||
if err != nil { | ||
return nil, errors.Wrapf(pollErr, "waiting for GameServer to be %v %v/%v: %v", | ||
state, gs.Namespace, gs.Name, err) | ||
} | ||
return readyGs, nil | ||
|
||
return readyGs, errors.Wrapf(err, "waiting for GameServer to be %v %v/%v", | ||
state, gs.Namespace, gs.Name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this would replace your previous change. pollErr is replaced with err which is local to the PollImmediate method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that was my intent with this change. I'm saying I don't think we need to do replace err with pollErr at all. The original code was in place to make the pollErr error visible, in case it is the cause of the test failing -- but we already have logging in place to show that error, so it seems redundant to do do both logging and bubble the error up. If we remove the bubble up, the code is cleaner, and we don't sacrifice the ability to discover the reasons for issues. At least - that is my intent 😄 |
||
} | ||
|
||
// WaitForFleetCondition waits for the Fleet to be in a specific condition or fails the test if the condition can't be met in 5 minutes. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readyGs, err := ...
and remove var err error?