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

Enable CLI container port expose test #7000

Merged
merged 10 commits into from
Jan 10, 2024
Merged

Enable CLI container port expose test #7000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8864e99
enable cli test
youngbupark Jan 8, 2024
3d220a5
Merge branch 'main' into youngp/enable-cli-test
youngbupark Jan 8, 2024
de5ceaa
fix test
youngbupark Jan 8, 2024
73b0246
adjust retry
youngbupark Jan 8, 2024
c8c4be8
revert
youngbupark Jan 9, 2024
a692894
add more log
youngbupark Jan 9, 2024
07a8a3c
adjust
youngbupark Jan 9, 2024
94c4397
improve code
youngbupark Jan 9, 2024
59ef24f
reorder
youngbupark Jan 10, 2024
60ee783
adjust
youngbupark Jan 10, 2024
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 go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ require (

require (
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/hashicorp/go-retryablehttp v0.7.5 // indirect
github.com/stretchr/objx v0.5.1 // indirect
github.com/tidwall/gjson v1.14.0 // indirect
github.com/tidwall/match v1.1.1 // indirect
Expand Down
3 changes: 3 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,14 @@ github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brv
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
github.com/hashicorp/go-retryablehttp v0.7.5 h1:bJj+Pj19UZMIweq/iie+1u5YCdGrnxCT9yvm0e+Nd5M=
github.com/hashicorp/go-retryablehttp v0.7.5/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8=
github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=
github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=
github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4=
Expand Down
53 changes: 19 additions & 34 deletions test/functional/shared/cli/cli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"testing"
"time"

"github.com/hashicorp/go-retryablehttp"
"github.com/radius-project/radius/pkg/cli/bicep"
"github.com/radius-project/radius/pkg/cli/clients"
"github.com/radius-project/radius/pkg/cli/cmd/radinit"
Expand Down Expand Up @@ -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")
Copy link
Contributor

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?

Copy link
Author

@youngbupark youngbupark Jan 10, 2024 •
edited
Loading

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.

port, err := GetAvailablePort()
require.NoError(t, err)

Expand All @@ -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)
Copy link
Contributor

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

t.Logf("ResourceExpose - output: %s", output)
done <- err
}()

Expand All @@ -254,40 +255,24 @@ func verifyCLIBasics(ctx context.Context, t *testing.T, test shared.RPTest) {
// callHealthEndpointOnLocalPort calls the magpie health endpoint '/healthz' with retries. It will fail the
// test if the exceed the number of retries without success.
func callHealthEndpointOnLocalPort(t *testing.T, retries int, port int) {
for i := 0; i < retries; i++ {
url := fmt.Sprintf("http://localhost:%d/healthz", port)
t.Logf("making request to %s", url)
response, err := http.Get(url)
if err != nil {
if i == retries-1 {
// last retry failed, report failure
require.NoError(t, err, "failed to get connect to resource after %d retries", retries)
}
t.Logf("got error %s", err.Error())
time.Sleep(1 * time.Second)
continue
}
if response.Body != nil {
defer response.Body.Close()
}

if response.StatusCode > 299 || response.StatusCode < 200 {
if i == retries-1 {
// last retry failed, report failure
require.NoError(t, err, "status code was a bad response after %d retries %d", retries, response.StatusCode)
}
t.Logf("got status %d", response.StatusCode)
time.Sleep(1 * time.Second)
continue
}
healthzURL := fmt.Sprintf("http://localhost:%d/healthz", port)

retryClient := retryablehttp.NewClient()
retryClient.RetryMax = retries
retryClient.RetryWaitMin = 5 * time.Second
Copy link
Contributor

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!

retryClient.RetryWaitMax = 20 * time.Second
retryClient.Backoff = retryablehttp.LinearJitterBackoff
retryClient.RequestLogHook = func(_ retryablehttp.Logger, req *http.Request, retry int) {
t.Logf("retry calling healthz endpoint %s, retry: %d", healthzURL, retry)
}

defer response.Body.Close()
content, err := io.ReadAll(response.Body)
require.NoError(t, err)
resp, err := retryClient.Get(healthzURL)
require.NoError(t, err, "failed to get connect to resource after %d retries", retries)
defer resp.Body.Close()
content, err := io.ReadAll(resp.Body)
require.NoError(t, err)

t.Logf("[response] %s", string(content))
return
}
t.Logf("[response] %s", string(content))
}

func Test_Run_Logger(t *testing.T) {
Expand Down
Loading