Clean up optional OpenStack metadata enablement

Use the ``networking-ovn-metadata-agent`` package

Use flags for tracking OpenStack metadata enablement.  The initial
version used the c-h kv interface directly for reasons that astounds
the author.

Change-Id: I51a03651e1931b4194b09b43acfb873d0cc896f6

src/lib/charm/openstack/ovn_chassis.py

@@ -3,6 +3,8 @@ import os
 import socket
 import subprocess
 
+import charms.reactive as reactive
+
 import charmhelpers.core as ch_core
 import charmhelpers.contrib.openstack.context as os_context
 
@@ -64,42 +66,18 @@ class OVNChassisCharm(charms_openstack.charm.OpenStackCharm):
         '/etc/default/ovn-host': ['ovn-host'],
     }
     python_version = 3
-    # Name of unitdata key with information on whether to enable metadata
-    metadata_kv_key = 'ovn-chassis-enable-metadata'
 
     def __init__(self, **kwargs):
-        enable_metadata = ch_core.unitdata.kv().get(
-            self.metadata_kv_key, False)
-        print(enable_metadata)
-        if enable_metadata:
-            # XXX for Train onwards, we should use the
-            #     ``networking-ovn-metadata-agent`` package
+        if reactive.is_flag_set('charm.ovn-chassis.enable-openstack-metadata'):
             metadata_agent = 'networking-ovn-metadata-agent'
-            self.packages.extend(['python3-networking-ovn', 'haproxy'])
+            self.packages.extend(['networking-ovn-metadata-agent', 'haproxy'])
             self.services.append(metadata_agent)
             self.restart_map.update({
                 '/etc/neutron/'
                 'networking_ovn_metadata_agent.ini': [metadata_agent],
-                '/etc/init.d/''networking-ovn-metadata-agent': [
-                    metadata_agent],
-                '/lib/systemd/system/networking-ovn-metadata-agent.service': (
-                    [metadata_agent]),
             })
-            self.permission_override_map = {
-                '/etc/init.d/networking-ovn-metadata-agent': 0o755,
-            }
         super().__init__(**kwargs)
 
-    def disable_metadata(self):
-        db = ch_core.unitdata.kv()
-        db.unset(self.metadata_kv_key)
-        db.flush()
-
-    def enable_metadata(self):
-        db = ch_core.unitdata.kv()
-        db.set(self.metadata_kv_key, True)
-        db.flush()
-
     def run(self, *args):
         cp = subprocess.run(
             args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, check=True,

src/reactive/ovn_chassis_handlers.py

@@ -20,32 +20,15 @@ charm.use_defaults(
 
 @reactive.when_not('nova-compute.connected')
 def disable_metadata():
-    with charm.provide_charm_instance() as charm_instance:
-        charm_instance.disable_metadata()
-        charm_instance.assess_status()
+    reactive.clear_flag('charm.ovn-chassis.enable-openstack-metadata')
 
 
 @reactive.when('nova-compute.connected')
 def enable_metadata():
+    reactive.set_flag('charm.ovn-chassis.enable-openstack-metadata')
     nova_compute = reactive.endpoint_from_flag('nova-compute.connected')
     nova_compute.publish_shared_secret()
     with charm.provide_charm_instance() as charm_instance:
-        ch_core.hookenv.log(
-            'DEBUG: {} {} {} {}'
-            .format(charm_instance,
-                    charm_instance.packages,
-                    charm_instance.services,
-                    charm_instance.restart_map),
-            level=ch_core.hookenv.INFO)
-        charm_instance.enable_metadata()
-    with charm.provide_charm_instance() as charm_instance:
-        ch_core.hookenv.log(
-            'DEBUG: {} {} {} {}'
-            .format(charm_instance,
-                    charm_instance.packages,
-                    charm_instance.services,
-                    charm_instance.restart_map),
-            level=ch_core.hookenv.INFO)
         charm_instance.install()
         charm_instance.assess_status()
 

src/templates/networking-ovn-metadata-agent

@@ -1,236 +0,0 @@
-#!/bin/sh
-
-###############################################################################
-# [ WARNING ]
-# Configuration file maintained by Juju. Local changes may be overwritten.
-# Configuration managed by neutron-openvswitch charm
-###############################################################################
-
-### BEGIN INIT INFO
-# Provides:          networking-ovn-metadata-agent
-# Required-Start:    $network $local_fs $remote_fs $syslog
-# Required-Stop:     $remote_fs
-# Should-Start:
-# Should-Stop:
-# Default-Start:     2 3 4 5
-# Default-Stop:      0 1 6
-# Short-Description: OpenStack OVN metadata agent
-# Description:       OpenStack Metadata services of instances
-#                    provisioned using OVN.
-### END INIT INFO
-
-# Author: James Page <james.page@ubuntu.com>
-
-PATH=/sbin:/usr/sbin:/bin:/usr/bin
-DESC="Networking OVN Metadata Agent"
-PROJECT_NAME=neutron
-NAME=networking-ovn-metadata-agent
-DAEMON_ARGS="--config-file=/etc/neutron/networking_ovn_metadata_agent.ini"
-# The metadata-agent runs as root as it needs to:
-# a) interact with openvswitch locally via a root owned unix socket
-# b) manage veth pairs for actual metadataproxy processes
-# the actual proxy between VM and Nova will not run as root
-SYSTEM_USER=root
-SYSTEM_GROUP=root
-#!/bin/sh
-# The content after this line comes from openstack-pkg-tools
-# and has been automatically added to a .init.in script, which
-# contains only the descriptive part for the daemon. Everything
-# else is standardized as a single unique script.
-
-# Author: Thomas Goirand <zigo@debian.org>
-# Author: Ondřej Nový <novy@ondrej.org>
-
-# PATH should only include /usr/* if it runs after the mountnfs.sh script
-PATH=/sbin:/usr/sbin:/bin:/usr/bin
-
-if [ -n "${UWSGI_PORT}" ] && [ -n "${UWSGI_INI_PATH}" ] && [ -n "${UWSGI_INI_APP}" ] ; then
-	if ! [ -f "${UWSGI_INI_APP}" ] ; then
-		exit 0
-	fi
-	if [ -d /etc/${PROJECT_NAME}/ssl/private ] ; then
-		KEY_FILE=$(find /etc/${PROJECT_NAME}/ssl/private -type f -iname '*.pem' 2>/dev/null | head -n 1)
-	fi
-
-	if [ -e /usr/local/share/ca-certificates/puppet_openstack.crt ] ; then
-		# This is needed for puppet...
-		CERT_FILE=/usr/local/share/ca-certificates/puppet_openstack.crt
-	else
-		if [ -d /etc/${PROJECT_NAME}/ssl/public ] ; then
-			CERT_FILE=$(find /etc/${PROJECT_NAME}/ssl/public -type f -iname '*.crt' 2>/dev/null | head -n 1)
-		fi
-	fi
-
-	# Sid doesn't have /usr/bin/uwsgi_python3, so we need
-	# to search for a more specific daemon name. For stretch
-	# /usr/bin/uwsgi_python3 is fine.
-	for i in 3 35 36 37 38 39 ; do
-		if [ -x /usr/bin/uwsgi_python${i} ] ; then
-			DAEMON=/usr/bin/uwsgi_python${i}
-		fi
-	done
-
-	if [ -n "${KEY_FILE}" ] && [ -n "${CERT_FILE}" ] ; then
-		DAEMON_ARGS="--https-socket [::]:${UWSGI_PORT},${CERT_FILE},${KEY_FILE}"
-	else
-		DAEMON_ARGS="--http-socket [::]:${UWSGI_PORT}"
-	fi
-
-	DAEMON_ARGS="${DAEMON_ARGS} --ini ${UWSGI_INI_PATH}"
-	NO_OPENSTACK_CONFIG_FILE_DAEMON_ARG=yes
-	NO_OPENSTACK_LOGFILE_DAEMON_ARG=yes
-fi
-
-if [ -z "${DAEMON}" ] ; then
-	DAEMON=/usr/bin/${NAME}
-fi
-PIDFILE=/var/run/${PROJECT_NAME}/${NAME}.pid
-if [ -z "${SCRIPTNAME}" ] ; then
-	SCRIPTNAME=/etc/init.d/${NAME}
-fi
-if [ -z "${SYSTEM_USER}" ] ; then
-	SYSTEM_USER=${PROJECT_NAME}
-fi
-if [ -z "${SYSTEM_GROUP}" ] ; then
-	SYSTEM_GROUP=${PROJECT_NAME}
-fi
-if [ "${SYSTEM_USER}" != "root" ] ; then
-	STARTDAEMON_CHUID="--chuid ${SYSTEM_USER}:${SYSTEM_GROUP}"
-fi
-if [ -z "${CONFIG_FILE}" ] ; then
-	CONFIG_FILE=/etc/${PROJECT_NAME}/${PROJECT_NAME}.conf
-fi
-LOGFILE=/var/log/${PROJECT_NAME}/${NAME}.log
-if [ -z "${NO_OPENSTACK_CONFIG_FILE_DAEMON_ARG}" ] ; then
-	DAEMON_ARGS="--config-file=${CONFIG_FILE} ${DAEMON_ARGS}"
-fi
-
-# Exit if the package is not installed
-[ -x $DAEMON ] || exit 0
-
-# If ran as root, create /var/lock/X, /var/run/X and /var/cache/X as needed
-if [ `whoami` = "root" ] ; then
-	for i in lock run cache ; do
-		mkdir -p /var/$i/${PROJECT_NAME}
-		chown ${SYSTEM_USER}:${SYSTEM_GROUP} /var/$i/${PROJECT_NAME}
-	done
-fi
-
-# This defines support functions which we use later on
-. /lib/lsb/init-functions
-
-RET=0
-
-# Manage log options: logfile and/or syslog, depending on user's choosing
-[ -r /etc/default/openstack ] && . /etc/default/openstack
-[ -r /etc/default/$NAME ] && . /etc/default/$NAME
-[ "x$USE_SYSLOG" = "xyes" ] && DAEMON_ARGS="$DAEMON_ARGS --use-syslog"
-if [ -z "${NO_OPENSTACK_LOGFILE_DAEMON_ARG}" ] ; then
-	[ "x$USE_LOGFILE" != "xno" ] && DAEMON_ARGS="$DAEMON_ARGS --log-file=$LOGFILE"
-fi
-
-do_start() {
-	start-stop-daemon \
-		--start \
-		--quiet \
-		--background ${STARTDAEMON_CHUID} \
-		--make-pidfile --pidfile ${PIDFILE} \
-		--chdir /var/lib/${PROJECT_NAME} \
-		--startas $DAEMON \
-		--test > /dev/null \
-		|| return 1
-	if [ -n "${PYARGV}" ] ; then
-		start-stop-daemon \
-			--start \
-			--quiet \
-			--background ${STARTDAEMON_CHUID} \
-			--make-pidfile --pidfile ${PIDFILE} \
-			--chdir /var/lib/${PROJECT_NAME} \
-			--startas $DAEMON \
-			-- $DAEMON_ARGS --pyargv "${PYARGV}" \
-			|| return 2
-	else
-		start-stop-daemon \
-			--start \
-			--quiet \
-			--background ${STARTDAEMON_CHUID} \
-			--make-pidfile --pidfile ${PIDFILE} \
-			--chdir /var/lib/${PROJECT_NAME} \
-			--startas $DAEMON \
-			-- $DAEMON_ARGS \
-			|| return 2
-	fi
-}
-
-do_stop() {
-	start-stop-daemon \
-		--stop \
-		--quiet \
-		--retry=TERM/30/KILL/5 \
-		--pidfile $PIDFILE
-	RETVAL=$?
-	rm -f $PIDFILE
-	return "$RETVAL"
-}
-
-do_systemd_start() {
-	if [ -n "${PYARGV}" ] ; then
-		exec $DAEMON $DAEMON_ARGS --pyargv "${PYARGV}"
-	else
-		exec $DAEMON $DAEMON_ARGS
-	fi
-}
-
-case "$1" in
-start)
-	log_daemon_msg "Starting $DESC" "$NAME"
-	do_start
-	case $? in
-		0|1) log_end_msg 0 ; RET=$? ;;
-		2)   log_end_msg 1 ; RET=$? ;;
-	esac
-;;
-stop)
-	log_daemon_msg "Stopping $DESC" "$NAME"
-	do_stop
-	case $? in
-		0|1) log_end_msg 0 ; RET=$? ;;
-		2)   log_end_msg 1 ; RET=$? ;;
-	esac
-;;
-status)
-	status_of_proc "$DAEMON" "$NAME"
-	RET=$?
-;;
-systemd-start)
-	do_systemd_start
-;;  
-show-args)
-	if [ -n "${PYARGV}" ] ; then
-		echo $DAEMON $DAEMON_ARGS --pyargv \"${PYARGV}\"
-	else
-		echo $DAEMON $DAEMON_ARGS
-	fi
-;;
-restart|force-reload)
-	log_daemon_msg "Restarting $DESC" "$NAME"
-	do_stop
-	case $? in
-		0|1)
-			do_start
-			case $? in
-				0) log_end_msg 0 ; RET=$? ;;
-				1) log_end_msg 1 ; RET=$? ;; # Old process is still running
-				*) log_end_msg 1 ; RET=$? ;; # Failed to start
-			esac
-		;;
-		*) log_end_msg 1 ; RET=$? ;; # Failed to stop
-	esac
-;;
-*)
-	echo "Usage: $SCRIPTNAME {start|stop|status|restart|force-reload|systemd-start}" >&2
-	RET=3
-;;
-esac
-
-exit $RET

src/templates/networking-ovn-metadata-agent.service

@@ -1,28 +0,0 @@
-###############################################################################
-# [ WARNING ]
-# Configuration file maintained by Juju. Local changes may be overwritten.
-# Configuration managed by neutron-openvswitch charm
-###############################################################################
-
-[Unit]
-Description=Networking OVN Metadata Agent
-
-
-
-
-[Service]
-User=root
-Group=root
-Type=simple
-WorkingDirectory=~
-RuntimeDirectory=neutron lock/neutron
-CacheDirectory=neutron
-ExecStart=/etc/init.d/networking-ovn-metadata-agent systemd-start
-Restart=on-failure
-LimitNOFILE=65535
-TimeoutStopSec=15
-
-
-[Install]
-WantedBy=multi-user.target
-

unit_tests/test_lib_charm_openstack_ovn_chassis.py

@@ -43,7 +43,14 @@ class Helper(test_utils.PatchHelper):
     def setUp(self):
         super().setUp()
         self.patch_release(ovn_chassis.OVNChassisCharm.release)
+        self.patch_object(ovn_chassis.reactive, 'is_flag_set',
+                          return_value=False)
         self.target = ovn_chassis.OVNChassisCharm()
+        # remove the 'is_flag_set' patch so the tests can use it
+        self._patches['is_flag_set'].stop()
+        setattr(self, 'is_flag_set', None)
+        del(self._patches['is_flag_set'])
+        del(self._patches_start['is_flag_set'])
 
     def patch_target(self, attr, return_value=None):
         mocked = mock.patch.object(self.target, attr)
@@ -56,19 +63,17 @@ class Helper(test_utils.PatchHelper):
 
 class TestOVNChassisCharm(Helper):
 
-    def test_disable_metadata(self):
-        self.patch_object(ovn_chassis.ch_core.unitdata, 'kv')
-        db = mock.MagicMock()
-        self.kv.return_value = db
-        self.target.disable_metadata()
-        db.unset.assert_called_once_with(self.target.metadata_kv_key)
-
-    def test_enable_metadata(self):
-        self.patch_object(ovn_chassis.ch_core.unitdata, 'kv')
-        db = mock.MagicMock()
-        self.kv.return_value = db
-        self.target.enable_metadata()
-        db.set.assert_called_once_with(self.target.metadata_kv_key, True)
+    def test_optional_openstack_metadata(self):
+        self.assertEquals(self.target.packages, ['ovn-host'])
+        self.assertEquals(self.target.services, ['ovn-host'])
+        self.patch_object(ovn_chassis.reactive, 'is_flag_set',
+                          return_value=True)
+        c = ovn_chassis.OVNChassisCharm()
+        self.assertEquals(c.packages, [
+            'ovn-host', 'networking-ovn-metadata-agent', 'haproxy'
+        ])
+        self.assertEquals(c.services, [
+            'ovn-host', 'networking-ovn-metadata-agent'])
 
     def test_run(self):
         self.patch_object(ovn_chassis.subprocess, 'run')

unit_tests/test_reactive_ovn_chassis_handlers.py

@@ -62,17 +62,20 @@ class TestOvnHandlers(test_utils.PatchHelper):
         self.provide_charm_instance().__exit__.return_value = None
 
     def test_disable_metadata(self):
+        self.patch_object(handlers.reactive, 'clear_flag')
         handlers.disable_metadata()
-        self.charm.disable_metadata.assert_called_once_with()
-        self.charm.assess_status.assert_called_once_with()
+        self.clear_flag.assert_called_once_with(
+            'charm.ovn-chassis.enable-openstack-metadata')
 
     def test_enable_metadata(self):
         self.patch_object(handlers.reactive, 'endpoint_from_flag')
+        self.patch_object(handlers.reactive, 'set_flag')
         nova_compute = mock.MagicMock()
         self.endpoint_from_flag.return_value = nova_compute
         handlers.enable_metadata()
+        self.set_flag.assert_called_once_with(
+            'charm.ovn-chassis.enable-openstack-metadata')
         nova_compute.publish_shared_secret.assert_called_once_with()
-        self.charm.enable_metadata.assert_called_once_with()
         self.charm.install.assert_called_once_with()
         self.charm.assess_status.assert_called_once_with()