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

Jump to bottom

Arm virtualisation #1371

Closed
wants to merge 41 commits into from
Closed

Arm virtualisation #1371

wants to merge 41 commits into from

Conversation

H4LL
Copy link
Contributor

@H4LL H4LL commented Sep 24, 2024

I made some updates to dockerfiles/ compose/ devcontainer files to specify amd64 as the platform. This was to ensure docker knew to virtualise hardware as as there is currently no implementation for ARM architecture. It's been working well for me so I thought I would share in case it helps someone else.

I have an issue open for this on #1365.

@WadeBarnes
Copy link
Member

WadeBarnes commented Sep 24, 2024 •
edited
Loading

@H4LL, Here is an example of a simpler pattern to accomplish the same thing.

https://github.com/bcgov/von-network/blob/e32d1732963ee64eb6635614f1ffb37db8b0cf36/manage#L8-L19

It instructs docker to use linux/amd64 on ARM platforms without hard coding the use of linux/amd64 throughout all of the docker and docker compose files. I'm sure something similar can be done with the devcontainer to instruct it to use linux/amd64 as well, but not sure the exact mechanics off the top of my head.

cc @loneil @esune

@WadeBarnes WadeBarnes requested review from loneil and esune September 24, 2024 11:59
@H4LL
Copy link
Contributor Author

H4LL commented Sep 25, 2024

Ah, I see. Got it, I'll make some edits in due course to make it graceful like the von precedent, test and then try again. Thanks for the feedback!

@H4LL
Copy link
Contributor Author

H4LL commented Oct 6, 2024

@WadeBarnes Turns out it was super easy in the end and pretty much the command you passed me verbatim. I searched the von repo for any other reference to DOCKER_DEFAULT_PLATFORM but it turned up nothing so I suppose I just learned a handy environmental variable for docker.

I tested the code by wiping my images and running a fresh build using the convenience script I've added. Everything is working out on my end.
I think it may be getting into manage script territory though. If you'd like something along those lines, I'd be happy to put it together!

@WadeBarnes
Copy link
Member

That looks a lot cleaner. Yes, I'd rather see a simple ./manage script that can be used to build, start, and stop the services more easily.

Pro-Tip:

  • Use pushd and popd to manage the directory context within the script. This way you can change directories as needed throughout the script without inadvertently changing the working directory for the user.

esune
esune requested changes Oct 7, 2024
View reviewed changes
Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, looks a lot cleaner!

Some of your commits appear to not have DCO, could you please amend them to include it and push again?

scripts/README.md
@@ -147,6 +147,12 @@ This will leave the volume (data) intact and available on restart.
docker compose down
```

For mac users with ARM architecture, try using the conveniance script
Copy link
Member

Choose a reason for hiding this comment

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

Nit.: convenience

H4LL and others added 12 commits October 10, 2024 12:07
@H4LL
working
5f67cf8 
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Revert "working"
d86a6b4 
This reverts commit f1f998a.

Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Delete devcontainer.json
8b5f259 
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Revert "Delete devcontainer.json"
78019a9 
This reverts commit cb7e9a1.

Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Reapply "working"
d6abd20 
This reverts commit f13fd55.

Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Revert "working"
9371fe3 
This reverts commit f1f998a.

Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Revert "working"
70beebe 
This reverts commit a71fe1c.

Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Added convenience script for mac
edf3000 
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Updated readme with convenience script
6a8118a 
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
delete api records and changing wallet based tokens
db62719 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Properly override url
79e7456 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Improved UI options for deletion of suspended tenants
588bf85 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
Gavinok and others added 18 commits October 10, 2024 12:07
@Gavinok @H4LL
Change soft deletion to use suspend as the keyword
1196b8a 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Reduce duplicate code for overriding token creation from wallets
6e50d8e 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Removed unused code
c7aff1e 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Removed unused delete_auth_tokens
162ff44 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Delete tenant token during soft deletion
d4e7511 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Removed token deletion for implementation in a separate PR
c8629a8 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@loneil @esune @H4LL
Dont use endorser service (bcgov#1344)
bd08fe6 
Removed unused endorser code and references

Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>
Co-authored-by: Emiliano Suñé <emiliano.sune@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@i5okie @H4LL
disable image cleanup job
8f3e32f 
Signed-off-by: Ivan P <2119240+i5okie@users.noreply.github.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Added middleware to prevent suspended tenants from making use of tokens
7c9cb6b 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Corrected Dissallow to Disallow
1feab73 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
rename double_check_token to check_token_not_suspended for clarity
6300f59 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Simplify detection of suspended tenants
5346d77 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Relocate tenant checks to allow record logging to still take place
826c18c 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@Gavinok @H4LL
Relocated suspension check to ensure that record exists
3f9d9d9 
Signed-off-by: Gavin Jaeger-Freeborn <gavinfreeborn@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@loneil @H4LL
Library updates
c76a2ea 
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@loneil @H4LL
Compatibility/deprecation fixes
2ddf024 
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
manage script looking decent
d4bf6d2 
Signed-off-by: Adam J Hall <46713492+H4LL@users.noreply.github.com>
@H4LL
Merge branch 'arm_virtualisation' of https://github.com/H4LL/traction …
09b0eef 
…into arm_virtualisation
This was referenced Oct 10, 2024
Closed
Merged
@H4LL
Copy link
Contributor Author

H4LL commented Oct 10, 2024

Hey, I managed to make a hash of the amendments so I created a freshie here:
#1388

I hope it's what you were after!

@H4LL H4LL closed this Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@esune esune esune requested changes

@loneil loneil Awaiting requested review from loneil

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@H4LL @WadeBarnes @esune @Gavinok @loneil @i5okie