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

docs: added note on permission errors and multipass mount #1568

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PietroPasotti
Copy link
Contributor

added a note on how to solve a common error when setting up charm-dev on multipass

Copy link
Member

@tmihoc tmihoc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Nitpick: I'd put the if condition in bold and with a colon. It helps users skip over faster if the condition doesn't apply to them.

@dimaqq dimaqq requested a review from dwilding February 10, 2025 00:19
@@ -320,6 +320,12 @@ If packing failed - perhaps you forgot to make the charm.py executable earlier -

```{important}

If packing fails with a `PermissionError(13, 'Permission denied')`, you may need to `multipass mount -g 1000:1000 -u 1000:1000 -g 0:0 -u 0:0`
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @PietroPasotti, I also ran into this error recently, so this addition is very timely! I think it would be beneficial to expand on the steps required, as this tutorial is aimed at first-timers. How about something like this:

Suggested change
If packing fails with a `PermissionError(13, 'Permission denied')`, you may need to `multipass mount -g 1000:1000 -u 1000:1000 -g 0:0 -u 0:0`
If packing fails with a `PermissionError(13, 'Permission denied')`, do the following:
1. Run `multipass stop charm-dev` to stop the VM.
2. Run `multipass umount charm-dev:~/fastapi-demo` to unmount the local directory.
3. Run `multipass mount -g 1000:1000 -u 1000:1000 -g 0:0 -u 0:0 ~/fastapi-demo charm-dev:~/fastapi-demo` to re-mount the local directory.
4. Run `multipass shell charm-dev`, then inside the VM run `cd ~/fastapi-demo` and `charmcraft pack`.

Copy link
Contributor

@dwilding dwilding left a comment

Choose a reason for hiding this comment

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

@dwilding
Copy link
Contributor

@tonyandrewmeyer want to make sure you see this addition, in case the same guidance needs to be included in the cut-down version of the K8s tutorial.

@tonyandrewmeyer
Copy link
Contributor

@tonyandrewmeyer want to make sure you see this addition, in case the same guidance needs to be included in the cut-down version of the K8s tutorial.

Thanks! This part remains unchanged, so it should be fine.

I don't object to this change, at least for the short term, but it seems like we should be aiming to resolve this earlier. If this is always needed, then shouldn't we put it in the instructions telling people how to add the mount? If it's not always needed, can we figure out why and put that in the instructions there too?

It seems odd to have to tell Multipass that uid/gid 0 in the host maps to 0 in the VM - is that not the default behaviour? For the 1000 - that seems like it's assuming that the current user/group is 1000, which will be true in simple cases but presumably not always. Couldn't we use id -u and id -g in the command to make it more robust?

Do we know why this has become an issue? Did Multipass deliberately make some change that removed some previous default mapping or something like that?

@tonyandrewmeyer
Copy link
Contributor

Possibly this multipass bug is related?

@tonyandrewmeyer
Copy link
Contributor

The latest multipass release calls out fixing mount permission errors (canonical/multipass#3834 specifically). Maybe that's solved this?

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.

5 participants