Remove build flag and re-introduce provision method attribute (#122)

* Remove build flag and re-introduce provision method attribute

The build flag was introduced as the replacement for the "method"
attribute. Earlier, the method could be set to "build" or "image", which
is then used by Foreman. The initial creation of a host also needed the
build flag to be set in the API request to signal Foreman that it should
create the machine.

This lead to the problem that Terraform configs containing "build = true"
would reset the build flag in Foreman even for existing, already built
machines, resetting them at the next boot. This is dangerous and should
be avoided.

The commit removes the build flag completely from Terraform and uses the
"provision_method" attribute from the Foreman API (again).

We will have to find an additional attribute to support the use case of
"enforced rebuild" that originally lead to the switch.

The "managed" flag is not changed.

Refs #40
Refs #68
Refs #106

* Add checks for image_id in compute_attributes in image-based provisioning

See examples/host/vmware_imagebased_host.tf for a VMware-backed image VM
Refs #103

Refs #102
Refs #105

* Fix test with failing provision_method Mock attribute

* Remove VMware-specific checks in ProvisionMethod==image in host creation

Removes an experimental check of the compute_attributes when using "image"-based
host creation. The backend providers need different values.

VMware for example needs the image_id as UUID, see the example file
in examples/host/vmware_imagebased_host.tf

Refs #125

Author: bitkeks

examples/host/vmware_imagebased_host.tf

@@ -0,0 +1,33 @@
+data "foreman_image" "ubuntu" {
+  name ="ubuntu-22.04"
+  compute_resource_id = data.foreman_computeresource.my_vmware_cluster.id
+}
+
+
+resource "foreman_host" "image_based_machine" {
+  count      = var.hosts.count
+  name       = format("my-image-based-machine-%d", count.index + 1)
+  provision_method     = "image"
+
+  # This is the Foreman-internal image ID, integer
+  # TODO: Do we need two image IDs, int and UUID?
+  image_id   = data.foreman_image.ubuntu.id
+
+  # The image_id JSON attribute is required in image-based setups on VMware! It must contain the VMware-internal UUID
+  compute_attributes = format(<<EOF
+{
+    "image_id": "%s"
+}
+EOF
+, data.foreman_image.ubuntu.uuid)
+
+  hostgroup_id        = foreman_hostgroup.ubuntu.id
+  compute_resource_id = data.foreman_computeresource.my_vmware_cluster.id
+}
+
+
+resource "foreman_hostgroup" "ubuntu" {
+  name          = var.group
+
+  # Not complete, fill with your own data
+}

foreman/api/host.go

@@ -50,8 +50,13 @@ type ForemanHost struct {
 
 	// Whether or not to rebuild the host on reboot
 	Build bool `json:"build"`
+	// Current build status
+	// From Foreman: BUILT = 0, PENDING = 1, TOKEN_EXPIRED = 2, BUILD_FAILED = 3
+	BuildStatus int `json:"build_status"`
+	// Current build status label
+	BuildStatusLabel string `json:"build_status_label"`
 	// Describes the way this host will be provisioned by Foreman
-	Method string `json:"provision_method,omitempty"`
+	ProvisionMethod string `json:"provision_method,omitempty"`
 	// ID of the domain to assign the host
 	DomainId *int `json:"domain_id,omitempty"`
 	// Name of the Domain. To substract from the Machine name
@@ -101,6 +106,10 @@ type ForemanHost struct {
 	PuppetAttributes PuppetAttribute `json:"puppet_attributes"`
 }
 
+func (fh *ForemanHost) isBuilt() bool {
+	return fh.BuildStatus == 0
+}
+
 // ForemanInterfacesAttribute representing a hosts defined network interfaces
 type ForemanInterfacesAttribute struct {
 	Id         int    `json:"id,omitempty"`

foreman/resource_foreman_host.go

@@ -83,6 +83,7 @@ func resourceForemanHostV0() *schema.Resource {
 					"Note: Changes to this attribute will trigger a host rebuild.",
 				),
 			},
+
 			"parameters": {
 				Type:     schema.TypeMap,
 				ForceNew: false,
@@ -342,17 +343,16 @@ func resourceForemanHost() *schema.Resource {
 				},
 			},
 
-			// -- Optional --
-
-			"method": {
-				Type:       schema.TypeString,
-				Optional:   true,
-				Deprecated: "The argument is handled by build instead",
+			"provision_method": {
+				Type:     schema.TypeString,
+				ForceNew: true,
+				Optional: true,
+				Default:  "build",
 				ValidateFunc: validation.StringInSlice([]string{
-					"build",
-					"image",
+					"build", // build = Network Based
+					"image", // image = Image Based
 				}, false),
-				Description: "REMOVED - use build argument instead to manage build flag of host.",
+				Description: "Sets the provision method in Foreman for this host: either network-based ('build') or image-based ('image')",
 			},
 
 			"comment": {
@@ -365,6 +365,7 @@ func resourceForemanHost() *schema.Resource {
 					"Note: Changes to this attribute will trigger a host rebuild.",
 				),
 			},
+
 			"parameters": {
 				Type:     schema.TypeMap,
 				ForceNew: false,
@@ -386,34 +387,21 @@ func resourceForemanHost() *schema.Resource {
 					"boot to PXE and power on. Defaults to `false`.",
 			},
 
-			"manage_build": {
-				Type:       schema.TypeBool,
-				Optional:   true,
-				Deprecated: "The feature was merged into the new key managed",
-				Description: "REMOVED, please use the new 'managed' key instead." +
-					" Create host only, don't set build status or manage power states",
-			},
-
 			"managed": {
 				Type:     schema.TypeBool,
 				Optional: true,
 				Default:  true,
 				Description: "Whether or not this host is managed by Foreman." +
 					" Create host only, don't set build status or manage power states.",
 			},
-			"build": {
-				Type:     schema.TypeBool,
-				Optional: true,
-				Default:  true,
-				Description: "Whether or not this host's build flag will be enabled in Foreman. Default is true, " +
-					"which means host will be built at next boot.",
-			},
+
 			"manage_power_operations": {
 				Type:        schema.TypeBool,
 				Optional:    true,
 				Default:     true,
 				Description: "Manage power operations, e.g. power on, if host's build flag will be enabled.",
 			},
+
 			"retry_count": {
 				Type:         schema.TypeInt,
 				Optional:     true,
@@ -732,8 +720,9 @@ func buildForemanHost(d *schema.ResourceData) *api.ForemanHost {
 	host.Comment = d.Get("comment").(string)
 	host.OwnerType = d.Get("owner_type").(string)
 	host.DomainName = d.Get("domain_name").(string)
+
+	host.ProvisionMethod = d.Get("provision_method").(string)
 	host.Managed = d.Get("managed").(bool)
-	host.Build = d.Get("build").(bool)
 	host.Token = d.Get("token").(string)
 
 	ownerId := d.Get("owner_id").(int)
@@ -760,10 +749,14 @@ func buildForemanHost(d *schema.ResourceData) *api.ForemanHost {
 	if mediumId != 0 {
 		host.MediumId = &mediumId
 	}
+
+	// TODO: How is this parameter used?
+	// VMware-backed providers need the UUID instead of the Foreman-internal ID
 	imageId := d.Get("image_id").(int)
 	if imageId != 0 {
 		host.ImageId = &imageId
 	}
+
 	modelId := d.Get("model_id").(int)
 	if modelId != 0 {
 		host.ModelId = &modelId
@@ -953,6 +946,10 @@ func setResourceDataFromForemanHost(d *schema.ResourceData, fh *api.ForemanHost)
 		log.Printf("[WARN] error setting compute attributes: %s", err)
 	}
 
+	// See issue #115 for "Build" attribute
+	d.Set("managed", fh.Managed)
+	d.Set("provision_method", fh.ProvisionMethod)
+
 	d.Set("domain_id", fh.DomainId)
 	d.Set("domain_name", fh.DomainName)
 	d.Set("environment_id", fh.EnvironmentId)
@@ -1046,11 +1043,19 @@ func resourceForemanHostCreate(ctx context.Context, d *schema.ResourceData, meta
 	client := meta.(*api.Client)
 	h := buildForemanHost(d)
 
-	managed := d.Get("managed").(bool)
+	// NOTE(ALL): Set the build flag to true on host create
+	if h.ProvisionMethod == "build" && h.Managed {
+		h.Build = true
+	}
 
 	log.Debugf("ForemanHost: [%+v]", h)
 	hostRetryCount := d.Get("retry_count").(int)
 
+	// See commit ad2b5890f09645513b520f12291546f26b812c96 for an experimental implementation
+	// for checks of the "computeAttributes" field, when using ProvisionMethod=image.
+	// The feature was removed because it was VMware-specific and the test on the backend provider
+	// could not yet be implemented (via client.ReadComputeResource -> computeResource.Provider)
+
 	createdHost, createErr := client.CreateHost(ctx, h, hostRetryCount)
 	if createErr != nil {
 		return diag.FromErr(createErr)
@@ -1065,15 +1070,14 @@ func resourceForemanHostCreate(ctx context.Context, d *schema.ResourceData, meta
 
 	setResourceDataFromForemanHost(d, createdHost)
 
-	enablebmc := d.Get("enable_bmc").(bool)
 	ManagePowerOperations := d.Get("manage_power_operations").(bool)
 
 	// Manage power operations only if needed, default is true
 	if ManagePowerOperations {
 		var powerCmds []interface{}
 		// If enable_bmc is true, perform required power off, pxe boot and power on BMC functions
 		// Don't modify power state at all if we're not managing the build
-		if enablebmc {
+		if h.EnableBMC {
 			log.Debugf("Calling BMC Reboot/PXE Functions")
 			// List of BMC Actions to perform
 			powerCmds = []interface{}{
@@ -1084,7 +1088,7 @@ func resourceForemanHostCreate(ctx context.Context, d *schema.ResourceData, meta
 					PowerAction: api.PowerCycle,
 				},
 			}
-		} else if managed {
+		} else if h.Managed {
 			log.Debugf("Using default Foreman behaviour for startup")
 			powerCmds = []interface{}{
 				api.Power{

foreman/resource_foreman_host_test.go

@@ -37,6 +37,8 @@ func ForemanHostToInstanceState(obj api.ForemanHost) *terraform.InstanceState {
 	}
 	attr["domain_name"] = obj.DomainName
 	attr["build"] = strconv.FormatBool(obj.Build)
+	attr["provision_method"] = obj.ProvisionMethod
+
 	if obj.EnvironmentId != nil {
 		attr["environment_id"] = strconv.Itoa(*obj.EnvironmentId)
 	}