Merge lp:~harlowja/cloud-init/notify-on-reload into lp:~cloud-init-dev/cloud-init/trunk

Proposed by Joshua Harlow
Status: Rejected
Rejected by: Chad Smith
Proposed branch: lp:~harlowja/cloud-init/notify-on-reload
Merge into: lp:~cloud-init-dev/cloud-init/trunk
Diff against target: 110 lines (+32/-14)
2 files modified
cloudinit/sources/__init__.py (+12/-4)
cloudinit/stages.py (+20/-10)
To merge this branch: bzr merge lp:~harlowja/cloud-init/notify-on-reload
Reviewer Review Type Date Requested Status
Chad Smith Needs Fixing
Server Team CI bot continuous-integration Needs Fixing
Review via email: mp+140365@code.launchpad.net

Commit message

Use a notify/subscribe model for configuration/distro reloading.

  Instead of the previous way where we reached into the datasource
  object and messed with its properties its cleaner to create a
  tiny set of functions that can be used to notify others of changes
  and let them use those changes as they wish. So instead of messing
  with the datasource properties, just notify it something has changed
  and have the datasource modify itself instead.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

Hello,
Thank you for taking the time to contribute to cloud-init. Cloud-init has moved its revision control system to git. We are cleaning up old reviews that no longer apply to the current codebase. As a result, we are marking all bzr merge proposals as 'rejected'. If you would like to re-submit this proposal for review, please do so by following the current HACKING documentation at http://cloudinit.readthedocs.io/en/latest/topics/hacking.html.

review: Needs Fixing

Unmerged revisions

752. By Joshua Harlow

Use a notify/subscribe model for configuration/distro reloading.

Instead of the previous way where we reached into the datasource
object and messed with its properties its cleaner to create a
tiny set of functions that can be used to notify others of changes
and let them use those changes as they wish. So instead of messing
with the datasource properties, just notify it something has changed
and have the datasource modify itself instead.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cloudinit/sources/__init__.py'
2--- cloudinit/sources/__init__.py 2012-11-12 17:26:49 +0000
3+++ cloudinit/sources/__init__.py 2012-12-18 07:45:25 +0000
4@@ -52,16 +52,24 @@
5 self.userdata = None
6 self.metadata = None
7 self.userdata_raw = None
8- name = util.obj_name(self)
9- if name.startswith(DS_PREFIX):
10- name = name[len(DS_PREFIX):]
11+ self.name = util.obj_name(self)
12+ if self.name.startswith(DS_PREFIX):
13+ self.name = self.name[len(DS_PREFIX):]
14 self.ds_cfg = util.get_cfg_by_path(self.sys_cfg,
15- ("datasource", name), {})
16+ ("datasource", self.name), {})
17 if not ud_proc:
18 self.ud_proc = ud.UserDataProcessor(self.paths)
19 else:
20 self.ud_proc = ud_proc
21
22+ def notify_reloaded(self, distro, system_config, _cloud_config):
23+ if system_config:
24+ self.sys_cfg = system_config
25+ self.ds_cfg = util.get_cfg_by_path(self.sys_cfg,
26+ ("datasource", self.name), {})
27+ if distro:
28+ self.distro = distro
29+
30 def get_userdata(self, apply_filter=False):
31 if self.userdata is None:
32 self.userdata = self.ud_proc.process(self.get_userdata_raw())
33
34=== modified file 'cloudinit/stages.py'
35--- cloudinit/stages.py 2012-12-17 13:41:11 +0000
36+++ cloudinit/stages.py 2012-12-18 07:45:25 +0000
37@@ -62,6 +62,7 @@
38 self._distro = None
39 # Changed only when a fetch occurs
40 self.datasource = NULL_DATA_SOURCE
41+ self.notify_listeners = []
42
43 def _reset(self, reset_ds=False):
44 # Recreated on access
45@@ -74,20 +75,24 @@
46 @property
47 def distro(self):
48 if not self._distro:
49- # Try to find the right class to use
50+ # Try to find the right class to use
51 system_config = self._extract_cfg('system')
52 distro_name = system_config.pop('distro', 'ubuntu')
53 distro_cls = distros.fetch(distro_name)
54 LOG.debug("Using distro class %s", distro_cls)
55 self._distro = distro_cls(distro_name, system_config, self.paths)
56- # If we have an active datasource we need to adjust
57- # said datasource and move its distro/system config
58- # from whatever it was to a new set...
59- if self.datasource is not NULL_DATA_SOURCE:
60- self.datasource.distro = self._distro
61- self.datasource.sys_cfg = system_config
62+ # Let any listener know that we have reloaded
63+ self._notify_reloaded(self._distro, system_config, self.cfg)
64 return self._distro
65
66+ def add_reloaded_listener(self, listener):
67+ if listener not in self.notify_listeners:
68+ self.notify_listeners.append(listener)
69+
70+ def _notify_reloaded(self, distro, system_config, cloud_config):
71+ for i in self.notify_listeners:
72+ i.notify_reloaded(distro, system_config, cloud_config)
73+
74 @property
75 def cfg(self):
76 return self._extract_cfg('restricted')
77@@ -161,9 +166,11 @@
78 # None check so that we don't keep on re-loading if empty
79 if self._cfg is None:
80 self._cfg = self._read_cfg(extra_fns)
81- # LOG.debug("Loaded 'init' config %s", self._cfg)
82
83 def _read_cfg(self, extra_fns):
84+ # A no config path object is used since at the point where this
85+ # function is called, it is loading the configuration to be so
86+ # it doesn't have access to any previous configuration...
87 no_cfg_paths = helpers.Paths({}, self.datasource)
88 merger = helpers.ConfigMerger(paths=no_cfg_paths,
89 datasource=self.datasource,
90@@ -235,8 +242,10 @@
91 pkg_list)
92 LOG.debug("Loaded datasource %s - %s", dsname, ds)
93 self.datasource = ds
94- # Ensure we adjust our path members datasource
95- # now that we have one (thus allowing ipath to be used)
96+ # Automatically add the loaded datasource to the reload notification
97+ # listener list so that it gets notified when configuration and distro
98+ # reloads happen...
99+ self.add_reloaded_listener(ds)
100 self._reset()
101 return ds
102
103@@ -420,6 +429,7 @@
104 #
105 # They will be recreated on future access...
106 self._reset()
107+
108 # Note(harlowja): the 'active' datasource will have
109 # references to the previous config, distro, paths
110 # objects before the load of the userdata happened,