Merge ~bjornt/maas:snap-bind-mounts-committed into maas:master

Proposed by Björn Tillenius
Status: Merged
Approved by: Björn Tillenius
Approved revision: b3193d52c935c4b91df7699237c94b5eb1a9ecb9
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~bjornt/maas:snap-bind-mounts-committed
Merge into: maas:master
Diff against target: 108 lines (+44/-7)
1 file modified
utilities/snap-bind-mounts (+44/-7)
Reviewer Review Type Date Requested Status
Adam Collard Approve
MAAS Lander Approve
Review via email: mp+404703@code.launchpad.net

Commit message

Make snap-bind-mount consider committed files as well.

It now gets the revision the currently installed snap is based on and
looks at the changes since that commit.

This makes it easier to test larger changes where you want to commit
what you're working on.

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b snap-bind-mounts-committed lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10323/console
COMMIT: db558b652194fa786d130600e5a794635ec6af4b

review: Needs Fixing
Revision history for this message
Adam Collard (adam-collard) :
review: Needs Information
Revision history for this message
Alberto Donato (ack) :
e7a5850... by Björn Tillenius

One diff command is enough.

cf693df... by Björn Tillenius

Read snap.yaml instead of parsing the snap info output.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b snap-bind-mounts-committed lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10612/console
COMMIT: 632305623be6f3ccd0e5ecb0d9342d5e3fbf43de

review: Needs Fixing
Revision history for this message
Björn Tillenius (bjornt) :
Revision history for this message
Adam Collard (adam-collard) :
b3193d5... by Björn Tillenius

Check to ensure that the 'maas' snap is installed.

Revision history for this message
Björn Tillenius (bjornt) :
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b snap-bind-mounts-committed lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10638/console
COMMIT: b3193d52c935c4b91df7699237c94b5eb1a9ecb9

review: Needs Fixing
Revision history for this message
Adam Collard (adam-collard) wrote (last edit ):

jenkins: !test

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b snap-bind-mounts-committed lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: b3193d52c935c4b91df7699237c94b5eb1a9ecb9

review: Approve
Revision history for this message
Adam Collard (adam-collard) wrote :

New files that need to be present in the snap is still an open question

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/utilities/snap-bind-mounts b/utilities/snap-bind-mounts
2index 3890813..f995480 100755
3--- a/utilities/snap-bind-mounts
4+++ b/utilities/snap-bind-mounts
5@@ -10,7 +10,12 @@ example, if the local installed snap is 2.9.2-9165-g.c3e7848d1, checkout
6 c3e7848d1. Otherwise, the file you change might depend on something that
7 isn't in the snap.
8
9-It uses 'git ls-files -m' to determine which files have been modified.
10+It gets the revision the snap is based on by looking at its version, and
11+then it will get all files that have been changed since that commit.
12+
13+It considers files both that have been committed, and files that are in
14+the staging area, and files that have been modified but not added to the
15+staging area yet.
16 """
17
18 import argparse
19@@ -18,6 +23,8 @@ from pathlib import Path
20 import subprocess
21 import sys
22
23+import yaml
24+
25 GIT_BASE_PATHS = [
26 Path("src/maasserver"),
27 Path("src/provisioningserver"),
28@@ -39,6 +46,16 @@ def get_snap_base_path():
29 return Path("/snap/maas/current/lib/python3.8/site-packages").resolve()
30
31
32+def get_installed_revision():
33+ snap_meta = Path("/snap/maas/current/meta/snap.yaml")
34+ if not snap_meta.exists():
35+ return
36+ with snap_meta.open() as meta_fh:
37+ metadata = yaml.safe_load(meta_fh)
38+ version = metadata["version"]
39+ return version.rsplit(".", 1)[-1]
40+
41+
42 def get_active_mounts(base_path):
43 mounts = []
44 for mount_line in Path("/proc/mounts").read_text().splitlines():
45@@ -49,15 +66,27 @@ def get_active_mounts(base_path):
46
47
48 def get_modified_files():
49+ parent_commit = get_installed_revision()
50+ command = ["git", "diff", "--name-only", parent_commit]
51+
52 proc = subprocess.run(
53- ["git", "ls-files", "-m"], text=True, stdout=subprocess.PIPE
54+ command, text=True, stdout=subprocess.PIPE, check=True
55 )
56- modified_files = []
57- for modified_file in [Path(path) for path in proc.stdout.splitlines()]:
58+ modified_files = set(Path(path) for path in proc.stdout.splitlines())
59+
60+ return sorted(modified_files)
61+
62+
63+def get_files_to_mount(modified_files):
64+ files_to_mount = []
65+ for modified_file in modified_files:
66 for base_path in GIT_BASE_PATHS:
67+ if "tests" in modified_file.parts[:-1]:
68+ # Unittests are not in the snap.
69+ continue
70 if is_relative_to(modified_file, base_path):
71- modified_files.append(modified_file)
72- return modified_files
73+ files_to_mount.append(modified_file)
74+ return files_to_mount
75
76
77 def unmount(target):
78@@ -95,7 +124,7 @@ def revert_mounts():
79
80 def apply_mounts():
81 snap_base_path = get_snap_base_path()
82- local_files = get_modified_files()
83+ local_files = get_files_to_mount(get_modified_files())
84 active_mounts = get_active_mounts(snap_base_path)
85 for active_mount in active_mounts:
86 local_file = Path("src") / active_mount.relative_to(snap_base_path)
87@@ -107,6 +136,12 @@ def apply_mounts():
88 # Unmount the old mount, so that it won't reference an older
89 # version of the file.
90 unmount(snap_path)
91+ if not snap_path.parent.exists():
92+ print(
93+ f"Ignoring {local_file} since parent directory doesn't exist "
94+ "in snap."
95+ )
96+ continue
97 mount(local_file, snap_path)
98 print("Restarting MAAS...")
99 subprocess.run(["sudo", "snap", "restart", "maas"], check=True)
100@@ -126,6 +161,8 @@ def main():
101 )
102 subparsers.add_parser("status", help="Show active bind mounts")
103 args = parser.parse_args()
104+ if get_installed_revision() is None:
105+ return "The 'maas' snap isn't installed. Please install it."
106 if args.command is None:
107 args.command = "status"
108 if args.command == "status":

Subscribers

People subscribed via source and target branches