lang: core: sys, engine: resource: Update hostname functionality

We didn't have a solid resource and sys.hostname() didn't have events!

engine/resources/hostname.go

@@ -39,6 +39,7 @@ import (
 	engineUtil "github.com/purpleidea/mgmt/engine/util"
 	"github.com/purpleidea/mgmt/util"
 	"github.com/purpleidea/mgmt/util/errwrap"
+	"github.com/purpleidea/mgmt/util/recwatch"
 
 	"github.com/godbus/dbus/v5"
 )
@@ -57,7 +58,10 @@ const (
 // resource is insufficient for the resource to do any useful work.
 var ErrResourceInsufficientParameters = errors.New("insufficient parameters for this resource")
 
-// HostnameRes is a resource that allows setting and watching the hostname.
+// HostnameRes is a resource that allows setting and watching the hostname. If
+// you don't specify any parameters, the Name is used. The Hostname field is
+// used if none of the other parameters are used. If the parameters are set to
+// the empty string, then those variants are not managed by the resource.
 type HostnameRes struct {
 	traits.Base // add the base methods without re-implementation
 
@@ -72,22 +76,54 @@ type HostnameRes struct {
 
 	// PrettyHostname is a free-form UTF8 host name for presentation to the
 	// user.
-	PrettyHostname string `lang:"pretty_hostname" yaml:"pretty_hostname"`
+	PrettyHostname *string `lang:"pretty_hostname" yaml:"pretty_hostname"`
 
 	// StaticHostname is the one configured in /etc/hostname or a similar
 	// file. It is chosen by the local user. It is not always in sync with
 	// the current host name as returned by the gethostname() system call.
-	StaticHostname string `lang:"static_hostname" yaml:"static_hostname"`
+	StaticHostname *string `lang:"static_hostname" yaml:"static_hostname"`
 
 	// TransientHostname is the one configured via the kernel's
 	// sethostbyname(). It can be different from the static hostname in case
 	// DHCP or mDNS have been configured to change the name based on network
 	// information.
-	TransientHostname string `lang:"transient_hostname" yaml:"transient_hostname"`
+	TransientHostname *string `lang:"transient_hostname" yaml:"transient_hostname"`
 
 	conn *dbus.Conn
 }
 
+func (obj *HostnameRes) getHostname() string {
+	if obj.Hostname != "" {
+		return obj.Hostname
+	}
+
+	return obj.Name()
+}
+
+func (obj *HostnameRes) getPrettyHostname() string {
+	if obj.PrettyHostname != nil {
+		return *obj.PrettyHostname // this may be empty!
+	}
+
+	return obj.getHostname()
+}
+
+func (obj *HostnameRes) getStaticHostname() string {
+	if obj.StaticHostname != nil {
+		return *obj.StaticHostname // this may be empty!
+	}
+
+	return obj.getHostname()
+}
+
+func (obj *HostnameRes) getTransientHostname() string {
+	if obj.TransientHostname != nil {
+		return *obj.TransientHostname // this may be empty!
+	}
+
+	return obj.getHostname()
+}
+
 // Default returns some sensible defaults for this resource.
 func (obj *HostnameRes) Default() engine.Res {
 	return &HostnameRes{}
@@ -95,7 +131,10 @@ func (obj *HostnameRes) Default() engine.Res {
 
 // Validate if the params passed in are valid data.
 func (obj *HostnameRes) Validate() error {
-	if obj.PrettyHostname == "" && obj.StaticHostname == "" && obj.TransientHostname == "" {
+	a := obj.getPrettyHostname() == ""
+	b := obj.getStaticHostname() == ""
+	c := obj.getTransientHostname() == ""
+	if a && b && c && obj.getHostname() == "" {
 		return ErrResourceInsufficientParameters
 	}
 	return nil
@@ -105,15 +144,6 @@ func (obj *HostnameRes) Validate() error {
 func (obj *HostnameRes) Init(init *engine.Init) error {
 	obj.init = init // save for later
 
-	if obj.PrettyHostname == "" {
-		obj.PrettyHostname = obj.Hostname
-	}
-	if obj.StaticHostname == "" {
-		obj.StaticHostname = obj.Hostname
-	}
-	if obj.TransientHostname == "" {
-		obj.TransientHostname = obj.Hostname
-	}
 	return nil
 }
 
@@ -124,6 +154,13 @@ func (obj *HostnameRes) Cleanup() error {
 
 // Watch is the primary listener for this resource and it outputs events.
 func (obj *HostnameRes) Watch(ctx context.Context) error {
+	recurse := false // single file
+	recWatcher, err := recwatch.NewRecWatcher("/etc/hostname", recurse)
+	if err != nil {
+		return err
+	}
+	defer recWatcher.Close()
+
 	// if we share the bus with others, we will get each others messages!!
 	bus, err := util.SystemBusPrivateUsable() // don't share the bus connection!
 	if err != nil {
@@ -149,7 +186,23 @@ func (obj *HostnameRes) Watch(ctx context.Context) error {
 	var send = false // send event?
 	for {
 		select {
-		case <-signals:
+		case _, ok := <-signals:
+			if !ok { // channel shutdown
+				return fmt.Errorf("unexpected close")
+			}
+			//signals = nil
+			send = true
+
+		case event, ok := <-recWatcher.Events():
+			if !ok { // channel shutdown
+				return fmt.Errorf("unexpected close")
+			}
+			if err := event.Error; err != nil {
+				return err
+			}
+			if obj.init.Debug { // don't access event.Body if event.Error isn't nil
+				obj.init.Logf("event(%s): %v", event.Body.Name, event.Body.Op)
+			}
 			send = true
 
 		case <-ctx.Done(): // closed by the engine to signal shutdown
@@ -209,22 +262,22 @@ func (obj *HostnameRes) CheckApply(ctx context.Context, apply bool) (bool, error
 	hostnameObject := conn.Object(hostname1Iface, hostname1Path)
 
 	checkOK := true
-	if obj.PrettyHostname != "" {
+	if h := obj.getPrettyHostname(); h != "" {
-		propertyCheckOK, err := obj.updateHostnameProperty(hostnameObject, obj.PrettyHostname, "PrettyHostname", "SetPrettyHostname", apply)
+		propertyCheckOK, err := obj.updateHostnameProperty(hostnameObject, h, "PrettyHostname", "SetPrettyHostname", apply)
 		if err != nil {
 			return false, err
 		}
 		checkOK = checkOK && propertyCheckOK
 	}
-	if obj.StaticHostname != "" {
+	if h := obj.getStaticHostname(); h != "" {
-		propertyCheckOK, err := obj.updateHostnameProperty(hostnameObject, obj.StaticHostname, "StaticHostname", "SetStaticHostname", apply)
+		propertyCheckOK, err := obj.updateHostnameProperty(hostnameObject, h, "StaticHostname", "SetStaticHostname", apply)
 		if err != nil {
 			return false, err
 		}
 		checkOK = checkOK && propertyCheckOK
 	}
-	if obj.TransientHostname != "" {
+	if h := obj.getTransientHostname(); h != "" {
-		propertyCheckOK, err := obj.updateHostnameProperty(hostnameObject, obj.TransientHostname, "Hostname", "SetHostname", apply)
+		propertyCheckOK, err := obj.updateHostnameProperty(hostnameObject, h, "Hostname", "SetHostname", apply)
 		if err != nil {
 			return false, err
 		}
@@ -242,13 +295,13 @@ func (obj *HostnameRes) Cmp(r engine.Res) error {
 		return fmt.Errorf("not a %s", obj.Kind())
 	}
 
-	if obj.PrettyHostname != res.PrettyHostname {
+	if engineUtil.StrPtrCmp(obj.PrettyHostname, res.PrettyHostname) != nil {
 		return fmt.Errorf("the PrettyHostname differs")
 	}
-	if obj.StaticHostname != res.StaticHostname {
+	if engineUtil.StrPtrCmp(obj.StaticHostname, res.StaticHostname) != nil {
 		return fmt.Errorf("the StaticHostname differs")
 	}
-	if obj.TransientHostname != res.TransientHostname {
+	if engineUtil.StrPtrCmp(obj.TransientHostname, res.TransientHostname) != nil {
 		return fmt.Errorf("the TransientHostname differs")
 	}
 
@@ -271,9 +324,9 @@ func (obj *HostnameRes) UIDs() []engine.ResUID {
 	x := &HostnameUID{
 		BaseUID:           engine.BaseUID{Name: obj.Name(), Kind: obj.Kind()},
 		name:              obj.Name(),
-		prettyHostname:    obj.PrettyHostname,
+		prettyHostname:    obj.getPrettyHostname(),
-		staticHostname:    obj.StaticHostname,
+		staticHostname:    obj.getStaticHostname(),
-		transientHostname: obj.TransientHostname,
+		transientHostname: obj.getTransientHostname(),
 	}
 	return []engine.ResUID{x}
 }

examples/lang/hostname_mapper.mcl

@@ -1,3 +1,4 @@
+import "sys"
 import "util"
 
 $m = {
@@ -7,6 +8,12 @@ $m = {
 	"aa:bb:cc:dd:ee:ff" => "hostname2",
 }
 
-print "mapper" {
+$h = util.hostname_mapper($m)
-	msg => util.hostname_mapper($m),
+
+print "hostname_mapper" {
+	msg => $h,
+}
+
+if $h != "" and sys.hostname() != $h {
+	hostname "${h}" {} # set it correctly!
 }

integration/basic_test.go

@@ -179,7 +179,7 @@ func TestCluster1(t *testing.T) {
 		code := util.Code(`
 		import "sys"
 		$root = sys.getenv("MGMT_TEST_ROOT")
-		$host = sys.hostname()
+		$host = $hostname #sys.hostname()
 
 		file "${root}/mgmt-hostname" {
 			content => "i am ${host}\n",
@@ -206,7 +206,7 @@ func TestCluster1(t *testing.T) {
 		code := util.Code(`
 		import "sys"
 		$root = sys.getenv("MGMT_TEST_ROOT")
-		$host = sys.hostname()
+		$host = $hostname #sys.hostname()
 
 		file "${root}/mgmt-hostname" {
 			content => "i am ${host}\n",

lang/core/sys/hostname_fact.go

@@ -31,15 +31,26 @@ package coresys
 
 import (
 	"context"
+	"fmt"
 
+	engineUtil "github.com/purpleidea/mgmt/engine/util"
 	"github.com/purpleidea/mgmt/lang/funcs/facts"
 	"github.com/purpleidea/mgmt/lang/types"
+	"github.com/purpleidea/mgmt/util"
+	"github.com/purpleidea/mgmt/util/errwrap"
+	"github.com/purpleidea/mgmt/util/recwatch"
+
+	"github.com/godbus/dbus/v5"
 )
 
 const (
 	// HostnameFuncName is the name this fact is registered as. It's still a
 	// Func Name because this is the name space the fact is actually using.
 	HostnameFuncName = "hostname"
+
+	hostname1Path       = "/org/freedesktop/hostname1"
+	hostname1Iface      = "org.freedesktop.hostname1"
+	dbusPropertiesIface = "org.freedesktop.DBus.Properties"
 )
 
 func init() {
@@ -79,14 +90,113 @@ func (obj *HostnameFact) Init(init *facts.Init) error {
 
 // Stream returns the single value that this fact has, and then closes.
 func (obj *HostnameFact) Stream(ctx context.Context) error {
+	defer close(obj.init.Output) // signal that we're done sending
+
+	recurse := false // single file
+	recWatcher, err := recwatch.NewRecWatcher("/etc/hostname", recurse)
+	if err != nil {
+		return err
+	}
+	defer recWatcher.Close()
+
+	// if we share the bus with others, we will get each others messages!!
+	bus, err := util.SystemBusPrivateUsable() // don't share the bus connection!
+	if err != nil {
+		return errwrap.Wrapf(err, "failed to connect to bus")
+	}
+	defer bus.Close()
+	// watch the PropertiesChanged signal on the hostname1 dbus path
+	args := fmt.Sprintf(
+		"type='signal', path='%s', interface='%s', member='PropertiesChanged'",
+		hostname1Path,
+		dbusPropertiesIface,
+	)
+	if call := bus.BusObject().Call(engineUtil.DBusAddMatch, 0, args); call.Err != nil {
+		return errwrap.Wrapf(call.Err, "failed to subscribe to DBus events for hostname1")
+	}
+	defer bus.BusObject().Call(engineUtil.DBusRemoveMatch, 0, args) // ignore the error
+
+	signals := make(chan *dbus.Signal, 10) // closed by dbus package
+	bus.Signal(signals)
+
+	// streams must generate an initial event on startup
+	startChan := make(chan struct{}) // start signal
+	close(startChan)                 // kick it off!
+
+	for {
 		select {
-	case obj.init.Output <- &types.StrValue{
+		case <-startChan: // kick the loop once at start
-		V: obj.init.Hostname,
+			startChan = nil // disable
-	}:
+
+		case _, ok := <-signals:
+			if !ok { // channel shutdown
+				return fmt.Errorf("unexpected close")
+			}
+
+		case event, ok := <-recWatcher.Events():
+			if !ok { // channel shutdown
+				return fmt.Errorf("unexpected close")
+			}
+			if err := event.Error; err != nil {
+				return err
+			}
+			if obj.init.Debug { // don't access event.Body if event.Error isn't nil
+				obj.init.Logf("event(%s): %v", event.Body.Name, event.Body.Op)
+			}
+
+		case <-ctx.Done(): // closed by the engine to signal shutdown
+			return nil
+		}
+
+		// NOTE: We ask the actual machine instead of using obj.init.Hostname
+		value, err := obj.Call(ctx)
+		if err != nil {
+			return err
+		}
+
+		select {
+		case obj.init.Output <- value:
 			// pass
 		case <-ctx.Done():
 			return nil
 		}
-	close(obj.init.Output) // signal that we're done sending
+	}
-	return nil
+}
+
+// Call returns the result of this function.
+func (obj *HostnameFact) Call(ctx context.Context) (types.Value, error) {
+	conn, err := util.SystemBusPrivateUsable()
+	if err != nil {
+		return nil, errwrap.Wrapf(err, "failed to connect to the private system bus")
+	}
+	defer conn.Close()
+
+	hostnameObject := conn.Object(hostname1Iface, hostname1Path)
+
+	h, err := obj.getHostnameProperty(hostnameObject, "Hostname")
+	if err != nil {
+		return nil, err
+	}
+
+	return &types.StrValue{
+		V: h,
+	}, nil
+}
+
+func (obj *HostnameFact) getHostnameProperty(object dbus.BusObject, property string) (string, error) {
+	propertyObject, err := object.GetProperty("org.freedesktop.hostname1." + property)
+	if err != nil {
+		return "", errwrap.Wrapf(err, "failed to get org.freedesktop.hostname1.%s", property)
+	}
+	if propertyObject.Value() == nil {
+		return "", fmt.Errorf("unexpected nil value received when reading property %s", property)
+	}
+
+	propertyValue, ok := propertyObject.Value().(string)
+	if !ok {
+		return "", fmt.Errorf("received unexpected type as %s value, expected string got '%T'", property, propertyValue)
+	}
+
+	// expected value and actual value match => checkOk
+	return propertyValue, nil
 }