Hexagonal architecture

ARCH.md

Towards a cleaner architecture for the provider

Ideas

Make naming consistent

Bad naming conventions make hard to read code and figure out layers when refactoring.

• Today

func resourceLibvirtNetworkCreate(d *schema.ResourceData, meta interface{}) error {
        virConn := meta.(*Client).libvirt
        // ...
  	networkDef := newNetworkDef()
        // ...
	networkDef.Name = d.Get("name").(string)
        // ...
	networkDef.Domain = getDomainFromResource(d) // sometimes virConn neeeds to be passed here
	// ...
	data, err := xmlMarshallIndented(networkDef)
	// ...
	return virConn.NetworkDefineXML(data)
}

Proposal

func resourceLibvirtNetworkCreate(d *schema.ResourceData, meta interface{}) error {
        virConn := meta.(*Client).libvirt
        // ...
  	networkDef := networkDefinitionDefault()
        // ...
	networkDef.Name = d.Get("name").(string)
        // ...
	networkDef.Domain = networkDefinitionDomain(d, meta) // sometimes virConn neeeds to be passed here
	// ...
	data, err := xmlMarshallIndented(networkDef)
	// ...
	return virConn.NetworkDefineXML(data)
}

For a create function resourceLibvirtNetworkCreate(d *schema.ResourceData, meta interface{}) error, we follow the following principles:

• We pass d *schema.ResourceData, meta interface{} down in every helper. Always as first parameters
• The top level helper creates an empty network definition with defaults, returning libvirtxml.Network, error eg. networkDefinitionDefault()
• We get virConn := meta.(*Client).libvirt explicitly in every helper
• Every helper gets the same naming convention eg. networkDef.Domain = getDomainFromResource(d) would be networkDef.Domain = networkDefinitionDomain(d, meta) and returning libvirtxml.Network.Domain, error
• We only do I/O at the end of resourceLibvirtNetworkCreate (calling NetworkCreate), except for the helpers that need to pull virConn out of meta in order to read something.

For a read function resourceLibvirtNetworkRead(d *schema.ResourceData, meta interface{}) error

• We retrieve the XML at the beginning of the function
• We call helpers with name convention networkResource(d, meta, network) and networkResourceDomain(d, meta, network.Domain) passing the original resource data, meta, and the XML definition to use when updating the resource data.

For an update function resourceLibvirtNetworkUpdate(d *schema.ResourceData, meta interface{}) error

• We retrieve the libvirt network handler at the beginning of the function
• We call helpers with name convention networUpdate(d, meta) and networkUpdateDomain(d, meta, network.Domain) passing the original resource data, meta, and the handler to use when issuing libvirt calls to live update.
• I/O at the end of the main function

How to avoid the update helper to issue I/O?

Other notes

func newDomainDefForConnection(virConn *libvirtc.Connect, rd *schema.ResourceData) (libvirtxml.Domain, error) {
	d := newDomainDef()

	if arch, ok := rd.GetOk("arch"); ok {
		d.OS.Type.Arch = arch.(string)
	} else {
		arch, err := getHostArchitecture(virConn)
		if err != nil {
			return d, err
		}
		d.OS.Type.Arch = arch
	}

Which take the libvirt connection, and therefore do I/O. They then call more functions, passing the connection. Note It is not even clear why getHostArchitecture is called there, or why the function below does not reuse getCapabilities, or reuse the libvirtxml struct. It is not clear why this functions are called "get", and there is a miriad of files related to domain, which do not express clear their purpose.

func getHostArchitecture(virConn *libvirtc.Connect) (string, error) {
	type HostCapabilities struct {
		XMLName xml.Name `xml:"capabilities"`
		Host    struct {
			XMLName xml.Name `xml:"host"`
			CPU     struct {
				XMLName xml.Name `xml:"cpu"`
				Arch    string   `xml:"arch"`
			}
		}
	}

	info, err := virConn.GetCapabilities()
	if err != nil {
		return "", err
	}

	capabilities := HostCapabilities{}
	xml.Unmarshal([]byte(info), &capabilities)

	return capabilities.Host.CPU.Arch, nil
}

The proposal is to move to an Hexagonal architecture.

• Terraform skeleton code
• Code that creates default XML structs, and converts from terraform schema ResourceData to XML structs
• thin layer on the libvirt code that allows to inject XML structs and do I/O

Suggestion: add a thin layer on libvirt access API and xml access API. This can be done by just augmenting the types with some methods by using type alias, embedding, or wrapping them completely.

package def

import (
	libvirtxml "github.com/libvirt/libvirt-go-xml"
)

type Domain libvirtxml.Domain

// fill defaults
func NewDefaultDomain()
func FromResourceData(data ResourceData) (Domain, error)

Alternatively, without type alias, and just passing the original object as 1st argument

• why is func updateOrAddHost(n *libvirtc.Network, ip, mac, name string) error { in network_def?