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

Fix daemon missing group #513

Merged
merged 9 commits into from
Dec 11, 2018
Merged

Fix daemon missing group #513

merged 9 commits into from
Dec 11, 2018

Conversation

townsend2010
Copy link
Contributor

I combined a few fixes in this as separate commits since they all went hand in hand and solve a couple of issues together.

Fixes #456, fixes #460

include/multipass/cli/command.h Outdated Show resolved Hide resolved
include/multipass/cli/command.h Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 11, 2018

Codecov Report

Merging #513 into master will decrease coverage by 0.12%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
- Coverage   66.45%   66.32%   -0.13%     
==========================================
  Files         146      146              
  Lines        5676     5687      +11     
==========================================
  Hits         3772     3772              
- Misses       1904     1915      +11
Impacted Files Coverage Δ
src/client/cmd/common_cli.cpp 52.94% <0%> (-2.17%) ⬇️
include/multipass/cli/command.h 64.51% <0%> (-26.4%) ⬇️
src/client/cmd/launch.cpp 54.47% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbe1bde...2fadd5a. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 11, 2018

Codecov Report

Merging #513 into master will decrease coverage by 0.16%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
- Coverage   66.45%   66.29%   -0.17%     
==========================================
  Files         146      146              
  Lines        5679     5693      +14     
==========================================
  Hits         3774     3774              
- Misses       1905     1919      +14
Impacted Files Coverage Δ
src/client/cmd/common_cli.cpp 52.94% <0%> (-2.17%) ⬇️
include/multipass/cli/command.h 58.82% <0%> (-32.09%) ⬇️
src/client/cmd/launch.cpp 54.47% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a960ae...8c00a1d. Read the comment docs.

@Saviq
Copy link
Collaborator

Saviq commented Dec 11, 2018

This should give me a better error now, no?

$ git log --oneline master..HEAD | cat
d4d8c6d Merge 2fadd5a91751bb9333815096ffff655b9b591799 into 59243da528631ae63586c6a3765e034a6e1c9d78
2fadd5a Reword text for telling user to check permissions on the socket
f928677 launch: Change error formatting strings based on last commit
5f4f702 client: Check if socket can be connected on rpc failure
5587747 daemon: Use the 'sudo' or 'adm' group for the socket

$ multipass version
multipass  2018.10.1-135-gd4d8c6d
$ snap info multipass
name:      multipass
summary:   Ubuntu at your fingertips
publisher: –
license:   unset
description: |
  Multipass gives you Ubuntu VMs in seconds. Just run `multipass launch`
  and it'll do all the setup for you. Check out `multipass find` for a list
  of available images. More details you can find under `multipass help`.
commands:
  - multipass
  - multipass.virsh
services:
  multipass.libvirt-bin: simple, enabled, inactive
  multipass.multipassd:  simple, enabled, inactive
tracking:     edge
refresh-date: today at 15:16 CET
channels:
  stable:    –
  candidate: –
  beta:      2018.10.1              (461) 104MB classic
  edge:      2018.10.1-130-g59243da (527) 109MB classic
installed:   2018.10.1-135-gd4d8c6d (x2)  18B   classic,try
$ multipass ls
list failed: Connect Failed

@townsend2010
Copy link
Contributor Author

@Saviq,

Not if multipassd is not running, which seems to be your case based on the multipass version output. The error message will be better only if multipassd is running and you don't have permission to access the socket.

include/multipass/cli/command.h Outdated Show resolved Hide resolved
include/multipass/cli/command.h Show resolved Hide resolved
Chris Townsend and others added 7 commits December 11, 2018 10:49
Fall back to 'root' if those groups don't exist on the host instead of throwing an exception.

Fixes #456
This will detect if the user has proper permission to access the Unix domain socket file.

Fixes #460
Co-Authored-By: townsend2010 <[email protected]>
@townsend2010 townsend2010 force-pushed the fix-daemon-missing-group branch from 2624d7f to 1eddc05 Compare December 11, 2018 15:50
include/multipass/cli/command.h Outdated Show resolved Hide resolved
return on_failure(status);
else
{
fmt::print("{}\n", context.peer());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Debug leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops:) Done

Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

bors delegate+

Please drop the debug leftover and you can land.

@Saviq
Copy link
Collaborator

Saviq commented Dec 11, 2018

bors delegate+

bors?

@townsend2010 townsend2010 force-pushed the fix-daemon-missing-group branch from ac82958 to 8c00a1d Compare December 11, 2018 18:20
@townsend2010
Copy link
Contributor Author

bors r=Saviq

bors bot added a commit that referenced this pull request Dec 11, 2018
513: Fix daemon missing group r=Saviq a=townsend2010

I combined a few fixes in this as separate commits since they all went hand in hand and solve a couple of issues together.

Fixes #456, fixes #460 

Co-authored-by: Chris Townsend <[email protected]>
Co-authored-by: Michał Sawicz <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 11, 2018

Build failed

@townsend2010
Copy link
Contributor Author

Ok, try again...

bors r=Saviq

bors bot added a commit that referenced this pull request Dec 11, 2018
513: Fix daemon missing group r=Saviq a=townsend2010

I combined a few fixes in this as separate commits since they all went hand in hand and solve a couple of issues together.

Fixes #456, fixes #460 

Co-authored-by: Chris Townsend <[email protected]>
Co-authored-by: Michał Sawicz <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 11, 2018

Build failed

@townsend2010
Copy link
Contributor Author

One more time...

bors r=Saviq

bors bot added a commit that referenced this pull request Dec 11, 2018
513: Fix daemon missing group r=Saviq a=townsend2010

I combined a few fixes in this as separate commits since they all went hand in hand and solve a couple of issues together.

Fixes #456, fixes #460 

Co-authored-by: Chris Townsend <[email protected]>
Co-authored-by: Michał Sawicz <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 11, 2018

Build failed

@townsend2010 townsend2010 merged commit 8c00a1d into master Dec 11, 2018
@bors bors bot deleted the fix-daemon-missing-group branch December 11, 2018 20:58
@Saviq Saviq mentioned this pull request Dec 20, 2018
bors bot added a commit that referenced this pull request Dec 20, 2018
551: Release 2018.12.1 r=townsend2010 a=Saviq

### Highlights

- On Linux, suspending/resuming the instance to/from disk is now supported. (#374)
- Better handling of delayed shutdown including posting `wall` messages to logged in users and allowing log ins to the instance unless 1 minute or less remains until shutdown. (#461, #50) 
- On Linux, all CPU flags should be passed into the running instance on newly created instances. (#516)
- Fixed some races around mount handling. (#514, #520)

### Bugs fixed:

- make the recover command idempotent (#528)
- explicitly stop mounts when deleting an instance to avoid a race (#520)
- be smarter about what group owns the multipass socket (#513, #523) 
- pass through all CPU flags when launching QEMU or libvirt instances (#516)
- use `info` log level for metrics issues (#515)
- fix potential race when starting a mount (#514)
- use `wall` shutdown messages for users logged into VM when delayed shutdown is initiated (#501)
- fix crash if exception during daemon start up (#487)
- refactor CLI code (#468)
- add default uid/gid mapping (#331)
- fix file metadata passthrough
- display uid/gid maps in info command (#439)
- add support for the suspend command (#374)
- shell to machine in delayed stop state (#461)
- improve uid/gid validation (#479)
- avoid leaking the libvirt bridge (#327, #413)
- add a restart command (#217)
- upgrade 3rd-party versions (#471)

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
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.

Bad messaging when multipass can't connect to the socket multipassd fails when sudo group does not exist
2 participants