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

Import login.json automatically in docker compose #89

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

spuder
Copy link
Contributor

@spuder spuder commented May 19, 2023

Solves the problem that was discussed in the Discord which I think will make everyone happy 😄

Problem 1: Improve user experience so they only need to run 1 docker compose command (instead of 2)
Problem 2: Attempt to automatically discover login.json on mac/linux machines, and if found, import it. (Eliminating the need to manually import the config file in the gui. Users do not need to memorize cmd+shift+g or permanently alter their ~/Library visibility settings)

How it works

  • Adds 3 new docker compose steps that ankerctl service depends on
    ( ankerctl_seed_mac, ankerctl_seed_linux, ankerctl_import)
  • Attempt to bind mount to hard coded mac/linux paths for login.json
  • Copies login.json to /tmp/login.json in the docker container
  • runs ankerctl config import to copy the config to the docker volume
  • All of these commands run with || true to ignore any failures. Containers run with restart: "no" to happily continue if the import fails (falling back to the default behavior of manually importing config)

Known issues / Compromises

This doesn't solve the issue where the config file has to be re-imported every time the container starts, however by automating the import as part of the startup, that is effectively a non-issue.

Summary

This makes docker compose up just work on mac and linux with no additional commands

@spuder spuder marked this pull request as draft May 19, 2023 05:04
@spuder spuder marked this pull request as ready for review May 19, 2023 05:18
Copy link
Contributor

@billyjbryant billyjbryant left a comment

Choose a reason for hiding this comment

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

There isn't an official version of AnkerMake for Linux at the moment, the only way to circumvent this is with Wine. I think we should replace ankerseed_linux with ankerseed_win and let Linux users fend for themselves with the browser-based upload.

- ankerctl_vol:/root/.config/ankerctl
- "$HOME/Library/Application Support/AnkerMake/AnkerMake_64bit_fp/login.json:/tmp/login.json"
command: sh -c "cp /tmp/login.json /root/.config/ankerctl/login.json || true"
ankerctl_seed_linux:
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no version of AnkerMake for Linux just yet, and if there is in the future the path will look more like $HOME/.local/share/<AnkerMakeSpecificPath>

Copy link
Contributor

Choose a reason for hiding this comment

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

INFORMATIONAL: Not for v1.0.0 ship but for v.1.1 we can look at their alpha build of the new slicer since that does have a linux build.

Copy link
Contributor

@thomasjpatterson thomasjpatterson May 19, 2023

Choose a reason for hiding this comment

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

Ok after futzing with this for about an hour I have it working this way:

volumes: - ankerctl_vol:/root/.config/ankerctl - $HOME/Library/Application Support/AnkerMake/AnkerMake_64bit_fp/login.json:/tmp/login.json #command: sh -c "cp /tmp/login.json /root/.config/ankerctl/login.json || true"
NO command argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Hmm, that is interesting. If the command argument is commented out then the cp won't work.

I suspect what happened is another previous attempt copied the login.json to the volume.

If you purge your docker volumes, and then try again I'm fairly confident that this will not work with that line commented out.

docker system prune
docker volume rm ankermake-m5-protocol_ankerctl_vol
docker compose up

- ankerctl_vol:/root/.config/ankerctl
entrypoint: "/app/ankerctl.py"
command: ["config", "import", "/root/.config/ankerctl/login.json"]
depends_on:
Copy link
Contributor

Choose a reason for hiding this comment

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

No ankerseed for Windows? Bind volumes work in Windows too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this just a naming thing though? It's called ankerctl_import and is still importing the login.json file right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, lets scrap linux support, and add windows support. Do you know the path on windows?

Copy link
Contributor

Choose a reason for hiding this comment

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

The way I understand it, the ankerctl_seed_* containers try to copy-in a login.json file from the host system, and ankerctl_import runs the import action.

@spuder could we make ankerctl_import responsible for all the cp ... || true attempts, and also ankerctl.py config import? That would make this a simple two-step thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, lets scrap linux support, and add windows support. Do you know the path on windows?

%LOCALAPPDATA%\Ankermake\AnkerMake_64bit_fp\login.json

Copy link
Contributor

Choose a reason for hiding this comment

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

@spuder See above

Copy link
Contributor

Choose a reason for hiding this comment

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

Or c:\Users\$USER\AppData\Local\Ankermake\AnkerMake_64bit_fp\login.json

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried a few local edits and can't find a way of getting thsi to work for Windows using Docker Desktop on WSL.

@thomasjpatterson
Copy link
Contributor

Not working on Mac M2
CleanShot 2023-05-19 at 07 55 10
CleanShot 2023-05-19 at 07 55 39

@thomasjpatterson thomasjpatterson added this to the 1.0.0 Release milestone May 19, 2023
@chrivers
Copy link
Contributor

Should this target upcoming/v1.0.0?

@spuder
Copy link
Contributor Author

spuder commented May 19, 2023

Should this target upcoming/v1.0.0?

Its close, but I would say don't delay the release for it.

volumes:
- ankerctl_vol:/root/.config/ankerctl
# TODO: verify this is the correct linux path
- "$HOME/AnkerMake/AnkerMake_64bit_fp/login.json:/tmp/login.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be our best bet, for Linux users running ankermake through wine with default settings:

$HOME/.wine/drive_c/users/$USER/AppData/Local/AnkerMake/AnkerMake_64bit_fp/login.json


ankerctl:
# image: ghcr.io/ankermgmt/ankermake-m5-protocol:latest
image: ankerctl/ankerctl:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how ankerctl/ankerctl:latest works? I'm not quite familiar with that syntax.

Suppose we make sure ghcr.io/ankermgmt/ankermake-m5-protocol:latest always has a fresh image available. What are the pros/cons of using that image here, vs pointing to ankerctl/ankerctl, which (I imagine) points to the locally-built image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually ghcr.io/ankermgmt/ankermake-m5-protocol:latest will be preferred, however for testing it won't work since I need the latest docker container available on my machine.

We can switch ankerctl/ankerctl to ghcr once we have the updated web gui published.
I can actually change this in git, and just override it temporarily while testing.

volumes:
- ankerctl_vol:/root/.config/ankerctl
entrypoint: "/app/ankerctl.py"
command: ["config", "import", "/root/.config/ankerctl/login.json"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be simpler to put login.json in (say) /root/login.json?

In fact, this location clashes with an upcoming feature, where it will be possible to select other "profiles" than default.json. (profiles are "complete config imports" - thank of them like browser profiles).

Also, can't we stack ankerctl_seed_mac and ankerctl_seed_linux, by just trying several cp ... || true with different paths? We could try all well-known paths, and if one of them works, we can import it. How does that sound?

I realize now that /root isn't part of the volume, but since we import the config, I suppose that's actually a feature, since the copy of login.json will then be ephemeral, while the imported default.json will be kept. Seems good?

- ankerctl_vol:/root/.config/ankerctl
entrypoint: "/app/ankerctl.py"
command: ["config", "import", "/root/.config/ankerctl/login.json"]
depends_on:
Copy link
Contributor

Choose a reason for hiding this comment

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

The way I understand it, the ankerctl_seed_* containers try to copy-in a login.json file from the host system, and ankerctl_import runs the import action.

@spuder could we make ankerctl_import responsible for all the cp ... || true attempts, and also ankerctl.py config import? That would make this a simple two-step thing.

restart: "no"
volumes:
- ankerctl_vol:/root/.config/ankerctl
- "$HOME/Library/Application Support/AnkerMake/AnkerMake_64bit_fp/login.json:/tmp/login.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail on mac because of the space, you need to properly escape the space between Application and Support.

- ankerctl_vol:/root/.config/ankerctl
entrypoint: "/app/ankerctl.py"
command: ["config", "import", "/root/.config/ankerctl/login.json"]
depends_on:
Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried a few local edits and can't find a way of getting thsi to work for Windows using Docker Desktop on WSL.

@chrivers chrivers modified the milestones: 1.0.0 Release, v1.1 Release May 22, 2023
@chrivers
Copy link
Contributor

After trying numerous techniques, it's clear that this is not quite going to be ready for v1. I've put this on the v1.1 milestone, to give us some time to figure out if this is going to work well.

In the meantime, I'm going to implement a small helper script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants