Merge ~andreserl/maas:chrony into maas:master

Proposed by Andres Rodriguez on 2018-02-26
Status: Merged
Merge reported by: Andres Rodriguez
Merged at revision: 97c7783728aafc4624634e56a3987f5c9a73bff6
Proposed branch: ~andreserl/maas:chrony
Merge into: maas:master
Diff against target: 488 lines (+102/-145)
13 files modified
dev/null (+0/-66)
run-skel/etc/chrony/.keep (+0/-0)
run-skel/etc/chrony/chrony.conf (+38/-0)
scripts/maas-write-file (+2/-2)
src/maasserver/regiondservices/ntp.py (+1/-1)
src/maasserver/service_monitor.py (+1/-1)
src/maastesting/fixtures.py (+6/-5)
src/provisioningserver/ntp/config.py (+20/-11)
src/provisioningserver/ntp/tests/test_config.py (+30/-55)
src/provisioningserver/rackdservices/ntp.py (+1/-1)
src/provisioningserver/service_monitor.py (+1/-1)
src/provisioningserver/tests/test_service_monitor.py (+1/-1)
utilities/setup-devel-environment (+1/-1)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve on 2018-02-27
Blake Rouse 2018-02-26 Approve on 2018-02-26
MAAS Lander Approve on 2018-02-26
Review via email: mp+339706@code.launchpad.net

Commit message

LP: #1744072 - Add chrony support instead of ntpd.

To post a comment you must log in.
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b chrony lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 97c7783728aafc4624634e56a3987f5c9a73bff6

review: Approve
Blake Rouse (blake-rouse) wrote :

Looks like a simple switch out, looks good!

review: Approve
Christian Ehrhardt  (paelzer) wrote :

I'm happy that you found chrony's orphan mode to work for you as well.
I also agree to initially set "allow" to be as NTP was for you.

And as blake said, other than that it seems like a direct replacement - LGTM!

review: Approve
Andres Rodriguez (andreserl) wrote :

Marking this merged as this landed as part of https://code.launchpad.net/~andreserl/maas/+git/maas/+merge/339723

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/run-skel/etc/ntp/.keep b/run-skel/etc/chrony/.keep
2index e69de29..e69de29 100644
3--- a/run-skel/etc/ntp/.keep
4+++ b/run-skel/etc/chrony/.keep
5diff --git a/run-skel/etc/chrony/chrony.conf b/run-skel/etc/chrony/chrony.conf
6new file mode 100644
7index 0000000..ca0fc3e
8--- /dev/null
9+++ b/run-skel/etc/chrony/chrony.conf
10@@ -0,0 +1,38 @@
11+# Welcome to the chrony configuration file. See chrony.conf(5) for more
12+# information about usuable directives.
13+
14+# Use servers from the NTP Pool Project. Approved by Ubuntu Technical Board
15+# on 2011-02-08 (LP: #104525). See http://www.pool.ntp.org/join.html for
16+# more information.
17+pool 0.ubuntu.pool.ntp.org iburst
18+pool 1.ubuntu.pool.ntp.org iburst
19+pool 2.ubuntu.pool.ntp.org iburst
20+pool 3.ubuntu.pool.ntp.org iburst
21+
22+# Use Ubuntu's ntp server as a fallback.
23+pool ntp.ubuntu.com
24+
25+# This directive specify the location of the file containing ID/key pairs for
26+# NTP authentication.
27+keyfile /etc/chrony/chrony.keys
28+
29+# This directive specify the file into which chronyd will store the rate
30+# information.
31+driftfile /var/lib/chrony/chrony.drift
32+
33+# Uncomment the following line to turn logging on.
34+#log tracking measurements statistics
35+
36+# Log files location.
37+logdir /var/log/chrony
38+
39+# Stop bad estimates upsetting machine clock.
40+maxupdateskew 100.0
41+
42+# This directive enables kernel synchronisation (every 11 minutes) of the
43+# real-time clock. Note that it can’t be used along with the 'rtcfile' directive.
44+rtcsync
45+
46+# Step the system clock instead of slewing it if the adjustment is larger than
47+# one second, but only in the first three clock updates.
48+makestep 1 3
49diff --git a/run-skel/etc/ntp.conf b/run-skel/etc/ntp.conf
50deleted file mode 100644
51index cd75b19..0000000
52--- a/run-skel/etc/ntp.conf
53+++ /dev/null
54@@ -1,66 +0,0 @@
55-# /etc/ntp.conf, configuration for ntpd; see ntp.conf(5) for help
56-
57-driftfile /var/lib/ntp/ntp.drift
58-
59-# Enable this if you want statistics to be logged.
60-#statsdir /var/log/ntpstats/
61-
62-statistics loopstats peerstats clockstats
63-filegen loopstats file loopstats type day enable
64-filegen peerstats file peerstats type day enable
65-filegen clockstats file clockstats type day enable
66-
67-# Specify one or more NTP servers.
68-
69-# Use servers from the NTP Pool Project. Approved by Ubuntu Technical Board
70-# on 2011-02-08 (LP: #104525). See http://www.pool.ntp.org/join.html for
71-# more information.
72-pool 0.ubuntu.pool.ntp.org iburst
73-pool 1.ubuntu.pool.ntp.org iburst
74-pool 2.ubuntu.pool.ntp.org iburst
75-pool 3.ubuntu.pool.ntp.org iburst
76-
77-# Use Ubuntu's ntp server as a fallback.
78-pool ntp.ubuntu.com
79-
80-# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html for
81-# details. The web page <http://support.ntp.org/bin/view/Support/AccessRestrictions>
82-# might also be helpful.
83-#
84-# Note that "restrict" applies to both servers and clients, so a configuration
85-# that might be intended to block requests from certain clients could also end
86-# up blocking replies from your own upstream servers.
87-
88-# By default, exchange time with everybody, but don't allow configuration.
89-restrict -4 default kod notrap nomodify nopeer noquery limited
90-restrict -6 default kod notrap nomodify nopeer noquery limited
91-
92-# Local users may interrogate the ntp server more closely.
93-restrict 127.0.0.1
94-restrict ::1
95-
96-# Needed for adding pool entries
97-restrict source notrap nomodify noquery
98-
99-# Clients from this (example!) subnet have unlimited access, but only if
100-# cryptographically authenticated.
101-#restrict 192.168.123.0 mask 255.255.255.0 notrust
102-
103-
104-# If you want to provide time to your local subnet, change the next line.
105-# (Again, the address is an example only.)
106-#broadcast 192.168.123.255
107-
108-# If you want to listen to time broadcasts on your local subnet, de-comment the
109-# next lines. Please do this only if you trust everybody on the network!
110-#disable auth
111-#broadcastclient
112-
113-#Changes recquired to use pps synchonisation as explained in documentation:
114-#http://www.ntp.org/ntpfaq/NTP-s-config-adv.htm#AEN3918
115-
116-#server 127.127.8.1 mode 135 prefer # Meinberg GPS167 with PPS
117-#fudge 127.127.8.1 time1 0.0042 # relative to PPS for my hardware
118-
119-#server 127.127.22.1 # ATOM(PPS)
120-#fudge 127.127.22.1 flag3 1 # enable PPS API
121diff --git a/scripts/maas-write-file b/scripts/maas-write-file
122index 8fb4981..97c230d 100755
123--- a/scripts/maas-write-file
124+++ b/scripts/maas-write-file
125@@ -18,8 +18,8 @@ from provisioningserver.utils.fs import atomic_write
126
127
128 whitelist = {
129- "/etc/ntp.conf",
130- "/etc/ntp/maas.conf",
131+ "/etc/chrony/chrony.conf",
132+ "/etc/chrony/maas.conf",
133 "/var/lib/maas/dhcpd-interfaces",
134 "/var/lib/maas/dhcpd.conf",
135 "/var/lib/maas/dhcpd6-interfaces",
136diff --git a/src/maasserver/regiondservices/ntp.py b/src/maasserver/regiondservices/ntp.py
137index 3d0e9d7..a5cdc21 100644
138--- a/src/maasserver/regiondservices/ntp.py
139+++ b/src/maasserver/regiondservices/ntp.py
140@@ -70,7 +70,7 @@ class RegionNetworkTimeProtocolService(TimerService):
141 def _maybeApplyConfiguration(self, configuration):
142 """Reconfigure the NTP server if the configuration changes.
143
144- Reconfigure and restart `ntpd` if the current configuration differs
145+ Reconfigure and restart `chrony` if the current configuration differs
146 from a previously applied configuration, otherwise do nothing.
147
148 :param configuration: The configuration object obtained from
149diff --git a/src/maasserver/service_monitor.py b/src/maasserver/service_monitor.py
150index 77ef473..6efd9b7 100644
151--- a/src/maasserver/service_monitor.py
152+++ b/src/maasserver/service_monitor.py
153@@ -34,7 +34,7 @@ class NTPServiceOnRegion(AlwaysOnService):
154 """Monitored NTP service on a region controller host."""
155
156 name = "ntp_region"
157- service_name = "ntp"
158+ service_name = "chrony"
159 snap_service_name = "ntp"
160
161
162diff --git a/src/maastesting/fixtures.py b/src/maastesting/fixtures.py
163index 4f66e1a..8e30e4f 100644
164--- a/src/maastesting/fixtures.py
165+++ b/src/maastesting/fixtures.py
166@@ -415,14 +415,15 @@ class MAASRootFixture(fixtures.Fixture):
167 self.path = self.useFixture(TempDirectory()).join("run")
168 # Work only in `run`; reference the old $MAAS_ROOT.
169 etc = Path(self.path).joinpath("etc")
170- # Create and populate $MAAS_ROOT/run/etc/{ntp,ntp.conf}. The
171- # `.keep` file is not strictly necessary, but it's created for
172+ # Create and populate $MAAS_ROOT/run/etc/{chrony,c/chrony.conf}.
173+ # The `.keep` file is not strictly necessary, but it's created for
174 # consistency with the source tree's `run` directory.
175- ntp = etc.joinpath("ntp")
176+ ntp = etc.joinpath("chrony")
177 ntp.mkdir(parents=True)
178 ntp.joinpath(".keep").touch()
179- ntp_conf = etc.joinpath("ntp.conf")
180- ntp_conf.write_bytes(skel.joinpath("etc", "ntp.conf").read_bytes())
181+ ntp_conf = ntp.joinpath("chrony.conf")
182+ ntp_conf.write_bytes(
183+ skel.joinpath("etc", "chrony", "chrony.conf").read_bytes())
184 # Create and populate $MAAS_ROOT/run/etc/maas.
185 maas = etc.joinpath("maas")
186 maas.mkdir(parents=True)
187diff --git a/src/provisioningserver/ntp/config.py b/src/provisioningserver/ntp/config.py
188index 9f1d765..d55d2fc 100644
189--- a/src/provisioningserver/ntp/config.py
190+++ b/src/provisioningserver/ntp/config.py
191@@ -24,15 +24,15 @@ from provisioningserver.path import get_tentative_data_path
192 from provisioningserver.utils.fs import sudo_write_file
193
194
195-_ntp_conf_name = "ntp.conf"
196-_ntp_maas_conf_name = "ntp/maas.conf"
197+_ntp_conf_name = "chrony/chrony.conf"
198+_ntp_maas_conf_name = "chrony/maas.conf"
199
200
201 def configure(servers, peers, offset):
202 """Configure the local NTP server with the given time references.
203
204- This writes new ``ntp.conf`` and ``ntp.maas.conf`` files, using ``sudo``
205- in production.
206+ This writes new ``chrony.chrony.conf`` and ``chrony.maas.conf`` files,
207+ using ``sudo`` in production.
208
209 :param servers: An iterable of server addresses -- IPv4, IPv6, hostnames
210 -- to use as time references.
211@@ -45,13 +45,13 @@ def configure(servers, peers, offset):
212 ntp_maas_conf_path = get_tentative_data_path("etc", _ntp_maas_conf_name)
213 sudo_write_file(
214 ntp_maas_conf_path,
215- ntp_maas_conf.encode("ascii"),
216+ ntp_maas_conf.encode("utf-8"),
217 mode=0o644)
218 ntp_conf = _render_ntp_conf(ntp_maas_conf_path)
219 ntp_conf_path = get_tentative_data_path("etc", _ntp_conf_name)
220 sudo_write_file(
221 ntp_conf_path,
222- ntp_conf.encode("ascii"),
223+ ntp_conf.encode("utf-8"),
224 mode=0o644)
225
226
227@@ -82,7 +82,7 @@ def _render_ntp_conf(includefile):
228 This configuration includes the file named by `includefile`.
229 """
230 ntp_conf_path = get_tentative_data_path("etc", _ntp_conf_name)
231- with open(ntp_conf_path, "r", encoding="ascii") as fd:
232+ with open(ntp_conf_path, "r", encoding="utf-8") as fd:
233 lines = _render_ntp_conf_from_source(fd, includefile)
234 return "".join(lines)
235
236@@ -97,7 +97,7 @@ def _render_ntp_conf_from_source(lines, includefile):
237 lines = _remove_maas_includefile_option(lines)
238 lines = _clean_whitespace(lines)
239 yield from lines # Has trailing blank line.
240- yield "includefile %s\n" % includefile
241+ yield "include %s\n" % includefile
242
243
244 def _render_ntp_maas_conf(servers, peers, offset):
245@@ -108,7 +108,7 @@ def _render_ntp_maas_conf(servers, peers, offset):
246 :param peers: An iterable of peer addresses -- IPv4, IPv6, hostnames -- to
247 use as time references.
248 :param offset: A relative stratum used when calculating the stratum for
249- orphan mode (see http://support.ntp.org/bin/view/Support/OrphanMode).
250+ orphan mode (https://chrony.tuxfamily.org/doc/3.2/chrony.conf.html).
251 """
252 lines = ["# MAAS NTP configuration."]
253 servers = map(normalise_address, servers)
254@@ -118,7 +118,16 @@ def _render_ntp_maas_conf(servers, peers, offset):
255 for server in servers)
256 peers = map(normalise_address, peers)
257 lines.extend("peer %s" % peer for peer in peers)
258- lines.append("tos orphan {:d}".format(offset + 8))
259+ # Chrony provides a special 'orphan' mode that is compatible
260+ # with ntpd's 'tos orphan' mode. (see
261+ # https://chrony.tuxfamily.org/doc/devel/chrony.conf.html)
262+ lines.append("local stratum {:d} orphan".format(offset + 8))
263+ # Chrony requires 'allow' option to specify which client IPs
264+ # or Networks can use it as a time source. For now, allow all
265+ # clients to be compatible to 'ntpd'. In the future, it would
266+ # be nice to limit this similarly to how we do proxy. (see
267+ # https://chrony.tuxfamily.org/doc/3.2/chrony.conf.html)
268+ lines.append("allow")
269 lines.append("") # Add newline at end.
270 return "\n".join(lines)
271
272@@ -150,7 +159,7 @@ def _disable_existing_pools_and_servers(lines):
273
274
275 _re_maas_includefile = re.compile(
276- r" ^ \s* includefile \s+ .* \b %s \b " % re.escape(_ntp_maas_conf_name),
277+ r" ^ \s* include \s+ .* \b %s \b " % re.escape(_ntp_maas_conf_name),
278 re.VERBOSE)
279
280
281diff --git a/src/provisioningserver/ntp/tests/test_config.py b/src/provisioningserver/ntp/tests/test_config.py
282index 53ca6b6..89ba737 100644
283--- a/src/provisioningserver/ntp/tests/test_config.py
284+++ b/src/provisioningserver/ntp/tests/test_config.py
285@@ -26,7 +26,7 @@ from testtools.matchers import (
286
287
288 def read_configuration(path):
289- with open(path, "r", encoding="ascii") as fd:
290+ with open(path, "r", encoding="utf-8") as fd:
291 return fd.read()
292
293
294@@ -58,7 +58,7 @@ def extract_peers_full(configuration):
295
296 def extract_included_files(configuration):
297 return re.findall(
298- r" ^ \s* includefile \s+ (\S*) $ ", configuration,
299+ r" ^ \s* include \s+ (\S*) $ ", configuration,
300 re.VERBOSE | re.MULTILINE)
301
302
303@@ -97,7 +97,7 @@ class TestConfigure(MAASTestCase):
304 extract_peers(ntp_maas_conf), Equals(peers))
305 self.assertThat(
306 extract_tos_options(ntp_maas_conf),
307- Equals(["orphan", str(offset + 8)]))
308+ Equals([str(offset + 8), "orphan"]))
309
310 def test_configure_region_is_alias(self):
311 self.assertThat(config.configure_region, IsInstance(partial))
312@@ -144,7 +144,8 @@ class TestNormaliseAddress(MAASTestCase):
313
314
315 def extract_tos_options(configuration):
316- commands = re.findall(r"^ *tos +([^\r\n]+)$", configuration, re.MULTILINE)
317+ commands = re.findall(
318+ r"^ *local stratum +([^\r\n]+)$", configuration, re.MULTILINE)
319 return list(chain.from_iterable(map(str.split, commands)))
320
321
322@@ -176,7 +177,7 @@ class TestRenderNTPConfFromSource(MAASTestCase):
323
324 def test_cleans_up_whitespace(self):
325 ntp_conf_lines = [
326- "# ntp.conf\n",
327+ "# chrony.conf\n",
328 "\n",
329 " \n",
330 "\t\r\n",
331@@ -190,12 +191,12 @@ class TestRenderNTPConfFromSource(MAASTestCase):
332 ntp_conf_lines, ntp_maas_conf_path)
333 self.assertThat(
334 list(ntp_conf_lines), Equals([
335- "# ntp.conf\n",
336+ "# chrony.conf\n",
337 "\n",
338 "foo",
339 "bar",
340 "\n",
341- "includefile %s\n" % ntp_maas_conf_path,
342+ "include %s\n" % ntp_maas_conf_path,
343 ]))
344
345
346@@ -266,23 +267,12 @@ class TestRenderNTPMAASConf(MAASTestCase):
347 ntp_maas_conf = config._render_ntp_maas_conf([], [], offset)
348 self.assertThat(
349 extract_tos_options(ntp_maas_conf),
350- Equals(["orphan", str(offset + 8)]))
351+ Equals([str(offset + 8), "orphan"]))
352
353
354 example_ntp_conf = """\
355-# /etc/ntp.conf, configuration for ntpd; see ntp.conf(5) for help
356-
357-driftfile /var/lib/ntp/ntp.drift
358-
359-# Enable this if you want statistics to be logged.
360-#statsdir /var/log/ntpstats/
361-
362-statistics loopstats peerstats clockstats
363-filegen loopstats file loopstats type day enable
364-filegen peerstats file peerstats type day enable
365-filegen clockstats file clockstats type day enable
366-
367-# Specify one or more NTP servers.
368+# Welcome to the chrony configuration file. See chrony.conf(5) for more
369+# information about usuable directives.
370
371 # Use servers from the NTP Pool Project. Approved by Ubuntu Technical Board
372 # on 2011-02-08 (LP: #104525). See http://www.pool.ntp.org/join.html for
373@@ -295,44 +285,29 @@ pool 3.ubuntu.pool.ntp.org iburst
374 # Use Ubuntu's ntp server as a fallback.
375 pool ntp.ubuntu.com
376
377-# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html
378-# for details. The web page <...> might also be helpful.
379-#
380-# Note that "restrict" applies to both servers and clients, so a configuration
381-# that might be intended to block requests from certain clients could also end
382-# up blocking replies from your own upstream servers.
383-
384-# By default, exchange time with everybody, but don't allow configuration.
385-restrict -4 default kod notrap nomodify nopeer noquery limited
386-restrict -6 default kod notrap nomodify nopeer noquery limited
387-
388-# Local users may interrogate the ntp server more closely.
389-restrict 127.0.0.1
390-restrict ::1
391-
392-# Needed for adding pool entries
393-restrict source notrap nomodify noquery
394-
395-# Clients from this (example!) subnet have unlimited access, but only if
396-# cryptographically authenticated.
397-#restrict 192.168.123.0 mask 255.255.255.0 notrust
398+# This directive specify the location of the file containing ID/key pairs for
399+# NTP authentication.
400+keyfile /etc/chrony/chrony.keys
401
402+# This directive specify the file into which chronyd will store the rate
403+# information.
404+driftfile /var/lib/chrony/chrony.drift
405
406-# If you want to provide time to your local subnet, change the next line.
407-# (Again, the address is an example only.)
408-#broadcast 192.168.123.255
409+# Uncomment the following line to turn logging on.
410+#log tracking measurements statistics
411
412-# If you want to listen to time broadcasts on your local subnet, de-comment the
413-# next lines. Please do this only if you trust everybody on the network!
414-#disable auth
415-#broadcastclient
416+# Log files location.
417+logdir /var/log/chrony
418
419-#Changes recquired to use pps synchonisation as explained in documentation:
420-#http://www.ntp.org/ntpfaq/NTP-s-config-adv.htm#AEN3918
421+# Stop bad estimates upsetting machine clock.
422+maxupdateskew 100.0
423
424-#server 127.127.8.1 mode 135 prefer # Meinberg GPS167 with PPS
425-#fudge 127.127.8.1 time1 0.0042 # relative to PPS for my hardware
426+# This directive enables kernel synchronisation (every 11 minutes) of the
427+# real-time clock. Note that it can’t be used along with the 'rtcfile'
428+# directive.
429+rtcsync
430
431-#server 127.127.22.1 # ATOM(PPS)
432-#fudge 127.127.22.1 flag3 1 # enable PPS API
433+# Step the system clock instead of slewing it if the adjustment is larger than
434+# one second, but only in the first three clock updates.
435+makestep 1 3
436 """
437diff --git a/src/provisioningserver/rackdservices/ntp.py b/src/provisioningserver/rackdservices/ntp.py
438index 39be675..c29dcb3 100644
439--- a/src/provisioningserver/rackdservices/ntp.py
440+++ b/src/provisioningserver/rackdservices/ntp.py
441@@ -80,7 +80,7 @@ class RackNetworkTimeProtocolService(TimerService):
442 def _maybeApplyConfiguration(self, configuration):
443 """Reconfigure the NTP server if the configuration changes.
444
445- Reconfigure and restart `ntpd` if the current configuration differs
446+ Reconfigure and restart `chrony` if the current configuration differs
447 from a previously applied configuration, otherwise do nothing.
448
449 :param configuration: The configuration object obtained from
450diff --git a/src/provisioningserver/service_monitor.py b/src/provisioningserver/service_monitor.py
451index 25d55e3..7c5d212 100644
452--- a/src/provisioningserver/service_monitor.py
453+++ b/src/provisioningserver/service_monitor.py
454@@ -62,7 +62,7 @@ class NTPServiceOnRack(Service):
455 """Monitored NTP service on a rack controller host."""
456
457 name = "ntp_rack"
458- service_name = "ntp"
459+ service_name = "chrony"
460 snap_service_name = "ntp"
461
462 def getExpectedState(self):
463diff --git a/src/provisioningserver/tests/test_service_monitor.py b/src/provisioningserver/tests/test_service_monitor.py
464index 5b841e2..7bab272 100644
465--- a/src/provisioningserver/tests/test_service_monitor.py
466+++ b/src/provisioningserver/tests/test_service_monitor.py
467@@ -101,7 +101,7 @@ class TestNTPServiceOnRack(MAASTestCase):
468
469 def test_name_and_service_name(self):
470 ntp = NTPServiceOnRack()
471- self.assertEqual("ntp", ntp.service_name)
472+ self.assertEqual("chrony", ntp.service_name)
473 self.assertEqual("ntp_rack", ntp.name)
474
475
476diff --git a/utilities/setup-devel-environment b/utilities/setup-devel-environment
477index a650be6..3766938 100755
478--- a/utilities/setup-devel-environment
479+++ b/utilities/setup-devel-environment
480@@ -4,7 +4,7 @@ ROOT_DIR="$( cd -P "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd)"
481 RUN_DIR=$ROOT_DIR=/.run
482
483 sudo add-apt-repository -u ppa:maas/next-proposed
484-sudo apt install authbind maas-dhcp ntp squid-deb-proxy
485+sudo apt install authbind maas-dhcp chrony squid-deb-proxy
486 if grep -q "^BindsTo=maas" /lib/systemd/system/maas-dhcpd.service
487 then
488 # Since we'll run the rackd service manuall, without using systemd,

Subscribers

People subscribed via source and target branches