-
Notifications
You must be signed in to change notification settings - Fork 97
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
Enable CLI container port expose test #7000
Enable CLI container port expose test #7000
Conversation
Signed-off-by: Young Bu Park <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: Young Bu Park <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: Young Bu Park <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: Young Bu Park <[email protected]>
Signed-off-by: Young Bu Park <[email protected]>
Signed-off-by: Young Bu Park <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: Young Bu Park <[email protected]>
Signed-off-by: Young Bu Park <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
@@ -228,7 +229,6 @@ func verifyCLIBasics(ctx context.Context, t *testing.T, test shared.RPTest) { | |||
}) | |||
|
|||
t.Run("Validate rad resource expose Container", func(t *testing.T) { | |||
t.Skip("https://github.com/radius-project/radius/issues/3232") |
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.
do you think this test is still flaky?
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.
Don't know, but I believe it will barely happen. The previous retry logic was linear back-off with 1 second interval. total 10 seconds might be too short to run container or app up completely in some cases. in new change, I used 5 second interval with jitter. This will give enough time to run container or app up.
Signed-off-by: Young Bu Park <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
@@ -237,7 +237,8 @@ func verifyCLIBasics(ctx context.Context, t *testing.T, test shared.RPTest) { | |||
|
|||
done := make(chan error) | |||
go func() { | |||
_, err = cli.ResourceExpose(child, appName, containerName, port, 3000) | |||
output, err := cli.ResourceExpose(child, appName, containerName, port, 3000) |
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.
The port could be a const
|
||
retryClient := retryablehttp.NewClient() | ||
retryClient.RetryMax = retries | ||
retryClient.RetryWaitMin = 5 * time.Second |
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.
This looks much better than the old code!
# Description This is to enable CLI container port expose test with the better retry. ## Type of change - This pull request fixes a bug in Radius and has an approved issue (issue link required). Fixes: radius-project#3232 --------- Signed-off-by: Young Bu Park <[email protected]> Signed-off-by: willdavsmith <[email protected]>
Description
This is to enable CLI container port expose test with the better retry.
Type of change
Fixes: #3232