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

IPv6 support #1340

Merged
merged 9 commits into from
Dec 5, 2022
Merged

IPv6 support #1340

merged 9 commits into from
Dec 5, 2022

Conversation

mathieu-benoit
Copy link
Contributor

@mathieu-benoit mathieu-benoit commented Dec 1, 2022

Support IPv6 on single-stack clusters.

2 issues are fixed:

We set [::] instead of 0.0.0.0 and will see from there. It seems that [::] works for both stacks: IPv4 and IPv6 (the app on our GKE-IPv4 test environment here http://34.27.51.101/ is still working). It was already the case in the code for recommendationservice and emailservice:

For the StackExchange.Redis issue, we need to add a direct dependency to StackExchange.Redis. See dotnet/aspnetcore#45424.

How to test this? Good question, you just have to deploy the following images:

  • gcr.io/online-boutique-ci/refs/pull/1340/paymentservice:1340
  • gcr.io/online-boutique-ci/refs/pull/1340/cartservice:1340
  • gcr.io/online-boutique-ci/refs/pull/1340/currencyservice:1340
  • gcr.io/online-boutique-ci/refs/pull/1340/emailservice:1340

Also, capturing some findings/learnings/pointers here too:

@mathieu-benoit mathieu-benoit requested a review from a team as a code owner December 1, 2022 21:02
@mathieu-benoit mathieu-benoit marked this pull request as draft December 1, 2022 21:02
@mathieu-benoit mathieu-benoit self-assigned this Dec 1, 2022
@mathieu-benoit mathieu-benoit added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 1, 2022
@github-actions
Copy link

github-actions bot commented Dec 1, 2022

🚲 PR staged at http://34.27.51.101

@mathieu-benoit mathieu-benoit changed the title [WIP] Tests with IPv6 [WIP] Experimental tests with IPv6 Dec 1, 2022
@mathieu-benoit mathieu-benoit added the needs more info This issue needs more information from the customer to proceed. label Dec 1, 2022
@github-actions
Copy link

github-actions bot commented Dec 1, 2022

🚲 PR staged at http://34.27.51.101

@naoki9911
Copy link

Thank you for your great work!
I've tested the patch in my ipv6 single-stack cluster, but it seems some errors happen.
image

rpc error: code = FailedPrecondition desc = Can't access cart storage. StackExchange.Redis.RedisConnectionException: It was not possible to connect to the redis server(s). UnableToConnect on redis-cart:6379/Interactive, Initializing/NotStarted, last: NONE, origin: BeginConnectAsync, outstanding: 0, last-read: 0s ago, last-write: 0s ago, keep-alive: 60s, state: Connecting, mgr: 10 of 10 available, last-heartbeat: never, global: 252s ago, v: 2.2.4.27433
   at StackExchange.Redis.ConnectionMultiplexer.ConnectImplAsync(ConfigurationOptions, TextWriter ) in /_/src/StackExchange.Redis/ConnectionMultiplexer.cs:line 879
   at Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.ConnectAsync(CancellationToken )
   at Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.GetAndRefreshAsync(String, Boolean, CancellationToken )
   at Microsoft.Extensions.Caching.StackExchangeRedis.RedisCache.GetAsync(String, CancellationToken )
   at cartservice.cartstore.RedisCartStore.GetCartAsync(String) in /app/cartstore/RedisCartStore.cs:line 104
could not retrieve cart
main.(*frontendServer).homeHandler
	/src/handlers.go:69
net/http.HandlerFunc.ServeHTTP
	/usr/local/go/src/net/http/server.go:2109
github.com/gorilla/mux.(*Router).ServeHTTP
	/go/pkg/mod/github.com/gorilla/mux@v1.8.0/mux.go:210
main.(*logHandler).ServeHTTP
	/src/middleware.go:82
main.ensureSessionID.func1
	/src/middleware.go:109
net/http.HandlerFunc.ServeHTTP
	/usr/local/go/src/net/http/server.go:2109
net/http.serverHandler.ServeHTTP
	/usr/local/go/src/net/http/server.go:2947
net/http.(*conn).serve
	/usr/local/go/src/net/http/server.go:1991
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1594

It seems cartservice fails to connect to redis server and may be related to the below issues.
StackExchange/StackExchange.Redis#2240
StackExchange/StackExchange.Redis#2277

cartservices uses Cahing.StackExchangeRedis with version 7.0.0.

<PackageReference Include="Microsoft.Extensions.Caching.StackExchangeRedis" Version="7.0.0" />

aspnetcore with .NET 7 uses StackExchangeRedis with version 2.2.4
https://github.com/dotnet/aspnetcore/blob/b9e4f660e6137756de9905b2b3463a9ea6e2de12/eng/Versions.props#L282

The version of StackExchangeRedis is released at Nov 19. 2020, so the fix in StackExchange/StackExchange.Redis#2240 is not applied and the same issue occures.

@mathieu-benoit
Copy link
Contributor Author

mathieu-benoit commented Dec 2, 2022

Wonderful, thanks for testing and reporting these detailed information, much appreciated!

So yeah, the StackExchange.Redis issue was reported in the past (#599) but you confirmed that it is still here. We will need to investigate this further. I updated the description of this PR accordingly. Thanks again!

So, just to confirm, do you now see that the previous errors related to 0.0.0.0 with currencyservice, emailservice and paymentservice are gone/fixed with this PR?

@naoki9911
Copy link

Thanks for the quick reply!

So, just to confirm, do you now see that the previous errors related to 0.0.0.0 with currencyservice, emailservice and paymentservice are gone/fixed with this PR?

Yes, the issue related to listening address seems to be fixed.
At least, services output errors related to cartservices(e.g. failed to connect to redis server)

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

🚲 PR staged at http://34.27.51.101

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

🚲 PR staged at http://34.27.51.101

@mathieu-benoit
Copy link
Contributor Author

mathieu-benoit commented Dec 2, 2022

Hi @naoki9911, could you give it another try by adding/updating the cartservice's container image to gcr.io/online-boutique-ci/refs/pull/1340/cartservice:1340 please? Thanks in advance!

Context: based on your detailed feedback (thanks again for that!), I filed dotnet/aspnetcore#45424 and they recommend to add an explicit direct dependency to StackExchange.Redis with a most recent version, it's now included in the cartservice's container image mentioned, so would like to see if it's working with this update, thanks!

@mathieu-benoit mathieu-benoit linked an issue Dec 2, 2022 that may be closed by this pull request
@naoki9911
Copy link

@mathieu-benoit Thanks!
I've tested the patch with newer version of StackExchange.Redis.
It works on my ipv6 single-stack cluster!
Services seem to work correctly, and the loadgenerator says the requests are completed successfully.
I appreciate your great effort on ipv6 single-stack cluster support!

Type     Name                                                                          # reqs      # fails |    Avg     Min     Max    Med |   req/s  failures/s
--------|----------------------------------------------------------------------------|-------|-------------|-------|-------|-------|-------|--------|-----------
GET      /                                                                                 28     0(0.00%) |     47      31     101     42 |    0.00        0.00
GET      /cart                                                                             48     0(0.00%) |     39      17     205     28 |    0.10        0.00
POST     /cart                                                                             50     0(0.00%) |     44      24     166     36 |    0.30        0.00
POST     /cart/checkout                                                                    12     0(0.00%) |     75      31     288     41 |    0.00        0.00
GET      /product/0PUK6V6EV0                                                               23     0(0.00%) |     30      20      47     30 |    0.10        0.00
GET      /product/1YMWWN1N4O                                                               25     0(0.00%) |     34      24      86     30 |    0.30        0.00
GET      /product/2ZYFJ3GM2N                                                               27     0(0.00%) |     33      23      53     29 |    0.10        0.00
GET      /product/66VCHSJNUP                                                               29     0(0.00%) |     31      23      72     29 |    0.50        0.00
GET      /product/6E92ZMYYFZ                                                               29     0(0.00%) |     32      20      58     30 |    0.10        0.00
GET      /product/9SIQT8TOJO                                                               30     0(0.00%) |     32      19      79     30 |    0.20        0.00
GET      /product/L9ECAV7KIM                                                               26     0(0.00%) |     36      24     129     30 |    0.30        0.00
GET      /product/LS4PSXUNUM                                                               29     0(0.00%) |     31      22      50     30 |    0.00        0.00
GET      /product/OLJCESPC7Z                                                               16     0(0.00%) |     31      21      49     30 |    0.00        0.00
POST     /setCurrency                                                                      35     0(0.00%) |     58      35     253     46 |    0.30        0.00
--------|----------------------------------------------------------------------------|-------|-------------|-------|-------|-------|-------|--------|-----------
         Aggregated                                                                       407     0(0.00%) |     39      17     288     32 |    2.30        0.00

@mathieu-benoit mathieu-benoit removed the needs more info This issue needs more information from the customer to proceed. label Dec 5, 2022
@mathieu-benoit
Copy link
Contributor Author

mathieu-benoit commented Dec 5, 2022

Really great, thanks for your help and collaboration on this, @naoki9911, much appreciated!

@mathieu-benoit mathieu-benoit changed the title [WIP] Experimental tests with IPv6 IPv6 support Dec 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

🚲 PR staged at http://34.27.51.101

@mathieu-benoit mathieu-benoit marked this pull request as ready for review December 5, 2022 13:28
@mathieu-benoit
Copy link
Contributor Author

Ready for your review, thanks!

@mathieu-benoit mathieu-benoit removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

🚲 PR staged at http://34.27.51.101

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

🚲 PR staged at http://34.27.51.101

Copy link
Collaborator

@NimJay NimJay left a comment

Choose a reason for hiding this comment

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

Thanks so much for @mathieu-benoit for adding this functionality to Online Boutique.

And thank you, @naoki9911, for not only testing this on a IPv6 single-stack cluster but also for providing detailed info about the errors you were witnessing! You made this possible.

Approved!

@NimJay NimJay merged commit 7fbbe03 into main Dec 5, 2022
@NimJay NimJay deleted the mathieu-benoit/ipv6 branch December 5, 2022 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants