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.

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

fix: add custom wait for retry logic #141

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

Shubhranshu153
Copy link
Contributor

Issue #, if available:

Description of changes:
Add a custom wait logic as the wait for goexec has an assertion, that marks the test fail in case of timeout.
eventually is usually also used with an assertion.
Without the wait logic the retry wont work due to assertion

Testing done:

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@pendo324
Copy link
Member

Description of changes: Add a custom wait logic as the wait for goexec has an assertion, that marks the test fail in case of timeout. eventually is usually also used with an assertion. Without the wait logic the retry wont work due to assertion

I skimmed through gexec's source and couldn't find the assertion you mentioned. Can you post a link in case something like this happens again?

@Shubhranshu153
Copy link
Contributor Author

/*
Wait waits until the wrapped command exits.  It can be passed an optional timeout.
If the command does not exit within the timeout, Wait will trigger a test failure.

Wait returns the session, making it possible to chain:

	session.Wait().Out.Contents()

will wait for the command to exit then return the entirety of Out's contents.

Wait uses eventually under the hood and accepts the same timeout/polling intervals that eventually does.
*/
func (s *Session) Wait(timeout ...interface{}) *Session {
	EventuallyWithOffset(1, s, timeout...).Should(Exit())
	return s
}

its goexec internals.

@pendo324
Copy link
Member


/*

Wait waits until the wrapped command exits.  It can be passed an optional timeout.

If the command does not exit within the timeout, Wait will trigger a test failure.



Wait returns the session, making it possible to chain:



	session.Wait().Out.Contents()



will wait for the command to exit then return the entirety of Out's contents.



Wait uses eventually under the hood and accepts the same timeout/polling intervals that eventually does.

*/

func (s *Session) Wait(timeout ...interface{}) *Session {

	EventuallyWithOffset(1, s, timeout...).Should(Exit())

	return s

}



its goexec internals.

I see, it either exits before timeout, or it fails due to the .Should(Exit())

@Shubhranshu153
Copy link
Contributor Author

yes

@pendo324
Copy link
Member

pendo324 commented Mar 28, 2024

I think this might be a little easier to read, lmk what you think

exitCodeChan := make(chan int)
exitCode := -1
for i := 0; i < retryPull && exitCode != 0; i++ {
	go func(retry int) {
		s := command.New(o, "pull", ref).WithoutWait().Run()
		for {
			select {
			case <-s.Exited:
				fmt.Printf("\nExited (attempt %d)\n", retry)
				exitCodeChan <- s.ExitCode()
				return
			case <-time.After(30 * time.Second):
				fmt.Printf("\nTimed out (attempt %d)\n", retry)
				exitCodeChan <- -1
				s.Interrupt().Kill()
				return
			}
		}
	}(i)

	exitCode = <-exitCodeChan
}

if exitCode != 0 {
	ginkgo.Fail(fmt.Sprintf("Failed to pull image: %s", ref))
}

tests/tests.go Outdated Show resolved Hide resolved
@Shubhranshu153
Copy link
Contributor Author

simplified the code.

@pendo324 pendo324 self-requested a review March 28, 2024 21:03
tests/tests.go Show resolved Hide resolved
tests/tests.go Outdated Show resolved Hide resolved
Signed-off-by: Shubharanshu Mahapatra <shubhum@amazon.com>
@Shubhranshu153 Shubhranshu153 merged commit 3b69319 into runfinch:main Mar 28, 2024
3 checks passed
Shubhranshu153 pushed a commit that referenced this pull request Mar 28, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.7.21](v0.7.20...v0.7.21)
(2024-03-28)


### Bug Fixes

* add a delay for system event monitoring to start before pull completes
as the run commands are async
([#144](#144))
([5de585f](5de585f))
* add custom wait for retry logic
([#141](#141))
([3b69319](3b69319))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants