Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

feat: apple-silicon support #920

Closed
wants to merge 16 commits into from
Closed

feat: apple-silicon support #920

wants to merge 16 commits into from

Conversation

johnnagro
Copy link
Contributor

@johnnagro johnnagro commented Apr 13, 2022

Description

Putting together a branch that supports Apple M1 macs. So far it is backwards compatible with Intel macs but reserving the right to make breaking changes.

Unresolved Issues

  • some node/npm pngquant version incompatibilities

Notes

  • M1 platform is arm64v8, although we haven't had to specify that anywhere (yet)
  • MySQL volumes are renamed to preserve old data and avoid data incompatibilities for Intel users who are upgrading

Changes

Testing

  • so far mostly local testing on volunteer laptops
  • we have a request into AWS for access to M1 hardware (in preview) for a CI setup

Related Work


I've completed each of the following or determined they are not applicable:

  • Made a plan to communicate any major developer interface changes (or N/A)

- working mysql image
- updated user/grant syntax for mysql8
@jmbowman
Copy link
Contributor

There's a previous effort towards this in #852 that was abandoned after hitting a couple of blockers, it may be useful for reference.

- redis workaround
Merge branch 'master' into apple-silicon
Merge branch 'master' into apple-silicon
- Merge branch 'master' into apple-silicon
@@ -258,7 +258,7 @@ services:
container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.redis"
hostname: redis.devstack.edx
image: redis:2.8
command: redis-server --requirepass password
command: env -i /usr/local/bin/redis-server --requirepass password
Copy link
Contributor Author

Choose a reason for hiding this comment

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

redis 5 shouldn't require this hack if we chose to upgrade

- rename mysql57 assets -> mysql80 to preserve old data
- todo: migration pathway for folks who want to keep their old data
@@ -237,28 +237,28 @@ services:
volumes:
- mongo_data:/data/db

mysql57:
mysql80:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The dev-env working group decided the most reasonable path forward here was to rename MySQL-related docker assets. this avoids data incompatibility and allows existing volumes (with data) to be preserved in the event we want to access that preexisting data.

- adding a backwards compatible mysql hostname alias
networks:
default:
aliases:
- edx.devstack.mysql80
- edx.devstack.mysql57
Copy link
Contributor Author

@johnnagro johnnagro May 11, 2022

Choose a reason for hiding this comment

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

keeping the old edx.devstack.mysql57 alias provides backwards comparability for projects with local config that point to the old hostname

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put an expiration date on this?

- Merge branch 'master' into apple-silicon
- Merge branch 'master' into apple-silicon
- Merge branch 'master' into apple-silicon
@johnnagro johnnagro marked this pull request as ready for review June 2, 2022 18:25
- Merge branch 'master' into apple-silicon
@johnnagro johnnagro requested a review from jmbowman June 21, 2022 17:28
Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

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

Some questions, but it looks good to me. Definitely would advise waiting on review from Jeremy or someone with more authority

environment:
MYSQL_ROOT_PASSWORD: ""
MYSQL_ALLOW_EMPTY_PASSWORD: "yes"
image: mysql:5.7
image: mysql:8.0.28-oracle
Copy link
Contributor

Choose a reason for hiding this comment

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

curiosity - why the oracle version?

networks:
default:
aliases:
- edx.devstack.mysql80
- edx.devstack.mysql57
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put an expiration date on this?

@rgraber
Copy link
Contributor

rgraber commented Jul 15, 2022

Just double checking before I approve: developers who are using devstack right now will not lose any data when they update their devstack from this PR, right? Will they need to do anything else (make dev.pull, I imagine?)? @johnnagro

@johnnagro
Copy link
Contributor Author

johnnagro commented Sep 13, 2022

The PR #968 seems to work and would negate the need for this apple-silicon branch. It also provides a possible solution for other incompatible images (use buildx ourselves to create compatible versions of public Dockerfiles)

@johnnagro
Copy link
Contributor Author

We've been able to build/use a cross-platform-compatible Mysql5.7 image via #968 and no longer need this apple-silicon branch / Mysql8.0 attempt to get around the M1 incompatibility. I can work with individual users if they want to move data back into mainline devstack - just reach out to me.

@johnnagro johnnagro closed this Sep 15, 2022
@johnnagro johnnagro deleted the apple-silicon branch March 7, 2023 16:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants