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 (community) 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
diff --git a/utilities/snap-bind-mounts b/utilities/snap-bind-mounts
index 3890813..f995480 100755
--- a/utilities/snap-bind-mounts
+++ b/utilities/snap-bind-mounts
@@ -10,7 +10,12 @@ example, if the local installed snap is 2.9.2-9165-g.c3e7848d1, checkout
10c3e7848d1. Otherwise, the file you change might depend on something that10c3e7848d1. Otherwise, the file you change might depend on something that
11isn't in the snap.11isn't in the snap.
1212
13It uses 'git ls-files -m' to determine which files have been modified.13It gets the revision the snap is based on by looking at its version, and
14then it will get all files that have been changed since that commit.
15
16It considers files both that have been committed, and files that are in
17the staging area, and files that have been modified but not added to the
18staging area yet.
14"""19"""
1520
16import argparse21import argparse
@@ -18,6 +23,8 @@ from pathlib import Path
18import subprocess23import subprocess
19import sys24import sys
2025
26import yaml
27
21GIT_BASE_PATHS = [28GIT_BASE_PATHS = [
22 Path("src/maasserver"),29 Path("src/maasserver"),
23 Path("src/provisioningserver"),30 Path("src/provisioningserver"),
@@ -39,6 +46,16 @@ def get_snap_base_path():
39 return Path("/snap/maas/current/lib/python3.8/site-packages").resolve()46 return Path("/snap/maas/current/lib/python3.8/site-packages").resolve()
4047
4148
49def get_installed_revision():
50 snap_meta = Path("/snap/maas/current/meta/snap.yaml")
51 if not snap_meta.exists():
52 return
53 with snap_meta.open() as meta_fh:
54 metadata = yaml.safe_load(meta_fh)
55 version = metadata["version"]
56 return version.rsplit(".", 1)[-1]
57
58
42def get_active_mounts(base_path):59def get_active_mounts(base_path):
43 mounts = []60 mounts = []
44 for mount_line in Path("/proc/mounts").read_text().splitlines():61 for mount_line in Path("/proc/mounts").read_text().splitlines():
@@ -49,15 +66,27 @@ def get_active_mounts(base_path):
4966
5067
51def get_modified_files():68def get_modified_files():
69 parent_commit = get_installed_revision()
70 command = ["git", "diff", "--name-only", parent_commit]
71
52 proc = subprocess.run(72 proc = subprocess.run(
53 ["git", "ls-files", "-m"], text=True, stdout=subprocess.PIPE73 command, text=True, stdout=subprocess.PIPE, check=True
54 )74 )
55 modified_files = []75 modified_files = set(Path(path) for path in proc.stdout.splitlines())
56 for modified_file in [Path(path) for path in proc.stdout.splitlines()]:76
77 return sorted(modified_files)
78
79
80def get_files_to_mount(modified_files):
81 files_to_mount = []
82 for modified_file in modified_files:
57 for base_path in GIT_BASE_PATHS:83 for base_path in GIT_BASE_PATHS:
84 if "tests" in modified_file.parts[:-1]:
85 # Unittests are not in the snap.
86 continue
58 if is_relative_to(modified_file, base_path):87 if is_relative_to(modified_file, base_path):
59 modified_files.append(modified_file)88 files_to_mount.append(modified_file)
60 return modified_files89 return files_to_mount
6190
6291
63def unmount(target):92def unmount(target):
@@ -95,7 +124,7 @@ def revert_mounts():
95124
96def apply_mounts():125def apply_mounts():
97 snap_base_path = get_snap_base_path()126 snap_base_path = get_snap_base_path()
98 local_files = get_modified_files()127 local_files = get_files_to_mount(get_modified_files())
99 active_mounts = get_active_mounts(snap_base_path)128 active_mounts = get_active_mounts(snap_base_path)
100 for active_mount in active_mounts:129 for active_mount in active_mounts:
101 local_file = Path("src") / active_mount.relative_to(snap_base_path)130 local_file = Path("src") / active_mount.relative_to(snap_base_path)
@@ -107,6 +136,12 @@ def apply_mounts():
107 # Unmount the old mount, so that it won't reference an older136 # Unmount the old mount, so that it won't reference an older
108 # version of the file.137 # version of the file.
109 unmount(snap_path)138 unmount(snap_path)
139 if not snap_path.parent.exists():
140 print(
141 f"Ignoring {local_file} since parent directory doesn't exist "
142 "in snap."
143 )
144 continue
110 mount(local_file, snap_path)145 mount(local_file, snap_path)
111 print("Restarting MAAS...")146 print("Restarting MAAS...")
112 subprocess.run(["sudo", "snap", "restart", "maas"], check=True)147 subprocess.run(["sudo", "snap", "restart", "maas"], check=True)
@@ -126,6 +161,8 @@ def main():
126 )161 )
127 subparsers.add_parser("status", help="Show active bind mounts")162 subparsers.add_parser("status", help="Show active bind mounts")
128 args = parser.parse_args()163 args = parser.parse_args()
164 if get_installed_revision() is None:
165 return "The 'maas' snap isn't installed. Please install it."
129 if args.command is None:166 if args.command is None:
130 args.command = "status"167 args.command = "status"
131 if args.command == "status":168 if args.command == "status":

Subscribers

People subscribed via source and target branches