Merge lp:~afreiberger/landscape-client-charm/add-juju-annotations into lp:landscape-client-charm

Proposed by Drew Freiberger
Status: Rejected
Rejected by: Haw Loeung
Proposed branch: lp:~afreiberger/landscape-client-charm/add-juju-annotations
Merge into: lp:landscape-client-charm
Diff against target: 193 lines (+119/-3)
4 files modified
config.yaml (+13/-2)
hooks/hooks.py (+8/-1)
hooks/install.py (+34/-0)
scripts/write_juju_annotations.py (+64/-0)
To merge this branch: bzr merge lp:~afreiberger/landscape-client-charm/add-juju-annotations
Reviewer Review Type Date Requested Status
Landscape Builder test results Needs Fixing
Simon Poirier (community) Needs Fixing
Review via email: mp+413315@code.launchpad.net

Commit message

Add cronjob and charm config for registering juju agents as Landscape annotations.

Description of the change

Based on previous efforts (which are then obsolete) by:
* @xavpaice https://code.launchpad.net/~xavpaice/landscape-client-charm/add_tags/+merge/407578
* @exsdev https://code.launchpad.net/~exsdev/landscape-client-charm/juju-app-annotator
* @stephanpampel https://code.launchpad.net/~stephanpampel/landscape-client-charm/landscape-client-charm/+merge/411409

Add hook and cronjob for registering juju apps as Landscape annotations
Can be enabled via config flag juju-annotations-enabled (default=false)

Details 'juju-annotations-enabled':
        Periodically updates landscape annotations (key:value) for each juju agent
        found on the machine and the juju availability-zone (AZ) of the machine.
        Key is 'juju-${application}' OR 'juju-az-${AZ}' if an AZ is set.
        Value is 'deployed=true' OR 'deployed=false' if the agent got removed.
        Example: juju-landscape-client:deployed=true
        Cronjob runs every 5 min, update frequency depends on exchange-interval.

To post a comment you must log in.
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
75. By Drew Freiberger

Address import issues

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
76. By Drew Freiberger

py3 updates for write-juju-annotations.py

77. By Drew Freiberger

Rename annotations script

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
78. By Drew Freiberger

fix data-path for juju annotations cronjob

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
79. By Drew Freiberger

Fixed path to annotations script

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
80. By Drew Freiberger

Annotations - Detect multiple dashes in unit names and add unit number along with app name

Revision history for this message
Drew Freiberger (afreiberger) wrote :

I've completed manual functional testing and am satisfied with this feature implementation now.

Ready for review by Landscape team.

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
81. By Drew Freiberger

Add clarification to config.yaml for annotations

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
82. By Drew Freiberger

Fix ordering of config-changed for juju-annotations cronjob

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
83. By Drew Freiberger

Fix regex for annotations

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Drew Freiberger (afreiberger) wrote :

Update from landscape team is that this is going to be superceded by work being done to allow client-side tag manipulation directly. This branch is being held available as the underlying branch published at cs:~afreiberger/landscape-client.

Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Simon Poirier (simpoir) wrote :

First, this changes needs some test coverage (for both hooks and the externally-called `write_juju_annotations.py` script), and fixing broken tests (which might be mostly covered by my comments below).

review: Needs Fixing
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)
Revision history for this message
Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
Landscape Builder (landscape-builder) wrote :
review: Needs Fixing (test results)

Unmerged revisions

83. By Drew Freiberger

Fix regex for annotations

82. By Drew Freiberger

Fix ordering of config-changed for juju-annotations cronjob

81. By Drew Freiberger

Add clarification to config.yaml for annotations

80. By Drew Freiberger

Annotations - Detect multiple dashes in unit names and add unit number along with app name

79. By Drew Freiberger

Fixed path to annotations script

78. By Drew Freiberger

fix data-path for juju annotations cronjob

77. By Drew Freiberger

Rename annotations script

76. By Drew Freiberger

py3 updates for write-juju-annotations.py

75. By Drew Freiberger

Address import issues

74. By Drew Freiberger

Update juju-annotations to be an external root cron

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2019-02-03 14:50:36 +0000
+++ config.yaml 2021-12-16 20:35:21 +0000
@@ -21,7 +21,7 @@
21 origin:21 origin:
22 description: |22 description: |
23 *DEPRECATED*. See install_sources and install_keys23 *DEPRECATED*. See install_sources and install_keys
24 24
25 Origin of ppa or private deb repository from which to install25 Origin of ppa or private deb repository from which to install
26 landscape-client. May be one of the following:26 landscape-client. May be one of the following:
27 distro (default), ppa:somecustom/ppa or a full APT url source27 distro (default), ppa:somecustom/ppa or a full APT url source
@@ -131,4 +131,15 @@
131 regardless of the setting created by the unattended-upgrades package.131 regardless of the setting created by the unattended-upgrades package.
132 type: boolean132 type: boolean
133 default: False133 default: False
134134 juju-annotations-enabled:
135 description: |
136 Periodically updates landscape annotations (key:value) for each juju agent
137 found on the machine and the juju availability-zone (AZ) of the machine.
138 Key is 'juju-${application}' OR 'juju-az-${AZ}' if an AZ is set.
139 Additionally, annotations for 'juju-${application}-${unit_number}' will
140 be set. Values are 'deployed=true' OR 'deployed=false' denoting presence
141 of these agents.
142 Example: juju-landscape-client:deployed=true
143 Cronjob runs every 5 min, update frequency depends on exchange-interval.
144 type: boolean
145 default: False
135146
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py 2019-04-04 16:07:48 +0000
+++ hooks/hooks.py 2021-12-16 20:35:21 +0000
@@ -18,7 +18,8 @@
18from ceph import (18from ceph import (
19 get_ceph_client_path, write_ceph_client_keyring, write_ceph_client_config)19 get_ceph_client_path, write_ceph_client_keyring, write_ceph_client_config)
2020
21from install import install21from install import (
22 install, create_annotations_cronjob, remove_annotations_cronjob)
2223
23CERT_FILE = "/etc/ssl/certs/landscape_server_ca.crt"24CERT_FILE = "/etc/ssl/certs/landscape_server_ca.crt"
2425
@@ -62,6 +63,12 @@
62 else:63 else:
63 set_config[key] = value64 set_config[key] = value
6465
66 # Manage toggling of juju-annotations cronjob
67 if service_config["juju-annotations-enabled"]:
68 create_annotations_cronjob()
69 else:
70 remove_annotations_cronjob()
71
65 relation_data.update(set_config)72 relation_data.update(set_config)
66 if service_config.get("disable-unattended-upgrades"):73 if service_config.get("disable-unattended-upgrades"):
67 Cnf = apt_pkg.Configuration()74 Cnf = apt_pkg.Configuration()
6875
=== modified file 'hooks/install.py'
--- hooks/install.py 2019-02-03 14:50:36 +0000
+++ hooks/install.py 2021-12-16 20:35:21 +0000
@@ -7,14 +7,19 @@
77
8import subprocess8import subprocess
9import sys9import sys
10from os import environ, remove
1011
11from charmhelpers import fetch12from charmhelpers import fetch
12import charmhelpers.core.hookenv as hookenv13import charmhelpers.core.hookenv as hookenv
14import charmhelpers.core.host as host
1315
1416
15hooks = hookenv.Hooks()17hooks = hookenv.Hooks()
1618
1719
20ANNOTATIONS_CRON_PATH = '/etc/cron.d/landscape-client-juju-annotations'
21
22
18@hooks.hook("install")23@hooks.hook("install")
19def install(fetch=fetch, hookenv=hookenv):24def install(fetch=fetch, hookenv=hookenv):
20 charm_config = hookenv.config()25 charm_config = hookenv.config()
@@ -34,6 +39,11 @@
34 "install", "-o", "landscape", "-g", "root", "-m", "755", "-d",39 "install", "-o", "landscape", "-g", "root", "-m", "755", "-d",
35 data_path])40 data_path])
3641
42 if charm_config["juju-annotations-enabled"]:
43 create_annotations_cronjob()
44 else:
45 remove_annotations_cronjob()
46
3747
38def add_apt_source(url, fetch=fetch):48def add_apt_source(url, fetch=fetch):
39 """Add an apt source entry, with passed in key.49 """Add an apt source entry, with passed in key.
@@ -54,6 +64,30 @@
54 return fetch.add_source(url, key)64 return fetch.add_source(url, key)
5565
5666
67def create_annotations_cronjob():
68 hookenv.log('Create juju-annotations cronjob', hookenv.DEBUG)
69 unit_name = hookenv.local_unit()
70 agent_name = 'unit-' + unit_name.replace('/', '-')
71 zone_env = ""
72 if environ['JUJU_AVAILABILITY_ZONE']:
73 az = environ['JUJU_AVAILABILITY_ZONE']
74 zone_env = "JUJU_AVAILABILITY_ZONE=%s" % az
75 data_path = hookenv.config().get("data-path")
76 if not data_path:
77 data_path = "/var/lib/landscape/client"
78 cronjob = (
79 "*/5 * * * * root %s "
80 "/var/lib/juju/agents/%s/charm/scripts/write_juju_annotations.py "
81 "%s\n"
82 ) % (zone_env, agent_name, data_path)
83 host.write_file(ANNOTATIONS_CRON_PATH, cronjob)
84
85
86def remove_annotations_cronjob():
87 hookenv.log('Remove juju-annotations cronjob', hookenv.DEBUG)
88 remove(ANNOTATIONS_CRON_PATH)
89
90
57if __name__ == '__main__':91if __name__ == '__main__':
58 try:92 try:
59 sys.exit(hooks.execute(sys.argv))93 sys.exit(hooks.execute(sys.argv))
6094
=== added directory 'scripts'
=== added file 'scripts/write_juju_annotations.py'
--- scripts/write_juju_annotations.py 1970-01-01 00:00:00 +0000
+++ scripts/write_juju_annotations.py 2021-12-16 20:35:21 +0000
@@ -0,0 +1,64 @@
1#!/usr/bin/env python3
2"""Update landscape-client juju info annotations.
3
4usage: [JUJU_AVAILABILITY_ZONE="zone1"] write-juju-annotations [lds-data-path]
5"""
6
7import sys
8import os
9import re
10
11
12JUJU_AGENT_DIR = "/var/lib/juju/agents"
13LDS_CLIENT_DATA_PATH = "/var/lib/landscape/client"
14
15
16def write_juju_annotations(argv):
17 """Write annotation files in the '$DATA_PATH/annotations.d' folder.
18
19 For each currently deployed juju unit and one annotation for the juju
20 availability zone (if JUJU_AVAILABILITY_ZONE is set). The
21 annotation-value/file-content is 'deployed=true'.
22
23 For previously-deployed/removed units change the annotation value to
24 'deployed=false'.
25
26 This is used in conjunction with lp:charm-landscape-tagger.
27 """
28 if len(argv) == 2:
29 data_path = argv[1]
30 else:
31 data_path = LDS_CLIENT_DATA_PATH
32 annotations_folder = os.path.join(data_path, 'annotations.d')
33 unit_regex = re.compile(r"^unit-(((\w|-)+)-\d+)")
34 prefix = 'juju-'
35
36 target_annotations = []
37 for agent in os.listdir(JUJU_AGENT_DIR):
38 result = unit_regex.match(agent)
39 if result:
40 unit = result[1]
41 app = result[2]
42 target_annotations.append(prefix + unit)
43 target_annotations.append(prefix + app)
44
45 current_annotations = \
46 [a for a in os.listdir(annotations_folder) if a.startswith(prefix)]
47
48 if "JUJU_AVAILABILITY_ZONE" in os.environ:
49 target_annotations.append(
50 f"juju-az-{os.environ['JUJU_AVAILABILITY_ZONE']}"
51 )
52
53 all_annotations = list(set(target_annotations + current_annotations))
54 for annotation in all_annotations:
55 annotation_file = os.path.join(annotations_folder, annotation)
56 with open(annotation_file, "w") as f:
57 if annotation in target_annotations:
58 f.write("deployed=true")
59 else:
60 f.write("deployed=false")
61
62
63if __name__ == '__main__':
64 write_juju_annotations(sys.argv)

Subscribers

People subscribed via source and target branches

to all changes: