feature: extend demo to work on arm64 #4499
Conversation
schristoff
requested review from
draychev,
jaellio,
nojnhuh,
shashankram and
snehachhabria
as code owners
Codecov Report
@@ Coverage Diff @@
## main #4499 +/- ##
==========================================
- Coverage 69.24% 69.05% -0.20%
==========================================
Files 216 216
Lines 14660 14660
==========================================
- Hits 10151 10123 -28
- Misses 4458 4486 +28
Partials 51 51
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
schristoff
changed the title
schristoff
force-pushed
the
schristoff_removenodeSelect
branch
from
78b531a to
d843116
Compare
.env.example
Outdated
| # Azure Container Registry (ACR) example: osmci.azurecr.io/osm | ||
| export CTR_REGISTRY=localhost:5000 | ||
| export CTR_REGISTRY=127.0.0.1:5000 |
There was a problem hiding this comment.
Sean and I were running into odd kind registry networking issues when communicating to localhost vs 127.0.0.1
I switched this over, and I do not see any issues hopefully :)
There was a problem hiding this comment.
Did you figure out why localhost doesn't work?
There was a problem hiding this comment.
No clue, still debugging. It works fine with any other application, though.
There was a problem hiding this comment.
Since this issue is very specific to your env and this is an example file, you could override it without needing to change the default here.
There was a problem hiding this comment.
It's entirely possible 127.0.0.1 is an objectively better default, but we won't know that for sure until we figure out why localhost isn't working.
There was a problem hiding this comment.
Regardless, this change seems unrelated to this PR. I'd make this and other similar changes in a separate PR so they can be reviewed in isolation.
There was a problem hiding this comment.
@shashankram I am confused - can you elaborate on how the change is unrelated to supporting arm users? Would be happy to separate the two out.
There was a problem hiding this comment.
running into odd kind registry networking issues when communicating to localhost vs 127.0.0.1
I switched this over
@schristoff This led me to think this is an issue specific to your environment (Kind cluster). Are you suggesting localhost doesn't work on arm64? If yes, I'd add this to the commit and PR description, and also a comment in the example file to not use localhost as we'd want to keep the demo as generic as possible.
There was a problem hiding this comment.
I think I've now narrowed down the issue (with the help of @nojnhuh ) - your original suggestion is correct @shashankram - I will open up another PR with the appropriate changes. :)
schristoff
changed the title
.env.example
Outdated
| # Azure Container Registry (ACR) example: osmci.azurecr.io/osm | ||
| export CTR_REGISTRY=localhost:5000 | ||
| export CTR_REGISTRY=127.0.0.1:5000 |
There was a problem hiding this comment.
Did you figure out why localhost doesn't work?
demo/deploy-mysql.sh
Outdated
| @@ -49,7 +49,7 @@ spec: | |||
| nodeSelector: | |||
| kubernetes.io/os: linux | |||
| containers: | |||
| - image: mysql:5.6 | |||
| - image: mysql/mysql-server:8.0.28 | |||
There was a problem hiding this comment.
@allenlsy Are there any concerns with bumping the MySQL version as long as the CI passes?
Description: Demo currently uses nodeSelector for scheduling - but this runs into issues when expanding usage to work with arm64.
Testing done:
make kind-demobuilds properly and was able to run through the demoAffected area:
Please answer the following questions with yes/no.
Does this change contain code from or inspired by another project?
no
Is this a breaking change?
no
Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?
n/a