After foreman_host argument method became deprecated, I can't boot host like before

[mr-maxo]

Foreman version: 3.1.0 Provider: terraform-coop/foreman / 0.5.7

Warning:

       │ Warning: Argument is deprecated
       │ 
       │   with foreman_host.vm,
       │   on main.tf line 22, in resource "foreman_host" "vm":
       │   22:   method              = "image"
       │ 
       │ The argument is handled by build instead

Code that worked before deprecation:

data "foreman_computeprofile" "profile" {
  for_each = var.vms
  name     = each.value.profile
}

data "foreman_computeresource" "compute" {
  for_each = var.vms
  name     = each.value.host
}

data "foreman_hostgroup" "hostgroup" {
  for_each = var.vms
  title    = each.value.group
}

resource "foreman_host" "vm" {
  for_each            = var.vms
  name                = each.key
  compute_profile_id  = data.foreman_computeprofile.profile[each.key].id
  compute_resource_id = data.foreman_computeresource.compute[each.key].id
  hostgroup_id        = data.foreman_hostgroup.hostgroup[each.key].id
  method              = "image"
}

Can you help me to make the code work as before without using argument method? In my case, foreman create new host instance from Ubuntu cloud image.

Is there an option to return method argument to provider? AFAIK method still existing in foreman API documentation as provision_method Foreman API create host

Thank you, maxo

[agriffit79]

This is related to https://github.com/terraform-coop/terraform-provider-foreman/discussions/105 however I see you are not setting image_id so the fix proposed there would not resolve your issue. How does the platform know which image to use, based off compute_profile_id?

[agriffit79]

@lhw the more I look at this it doesn't make sense why method was deprecated in the first place. The build flag specifies if the host should be built, but method specifies how it should be built. If we re-introduce method then it should solve this issue and #105. We can default it to a value of "build" so it shouldn't be a breaking change.

[lhw]

I think I actually argued against the change as well but the discussion ended up the way it did. I don't really have an skin in the game at the moment so I am fine with a good solution that would fix the issue once and for all. If required we can create another schema and do a migration again, like I did with the first change.

[agriffit79]

My guess is the author of that pull request did not use the image build function, so perhaps didn't appreciate the implications of removing it. I wonder, would it be better to introduce a new parameter of provision_method rather than resurrect the deprecated one? This would match the parameter name in the underlying API.

[lhw]

Fine with me. As mentioned we can even do a schema migration that would automate the process for existing installations. btw if you want maintainer rights on the repository I can add you here. I do have a lot less time to spent on this as a hobby then i anticipated. And without a real installation to check errors against its also quite hard to so real world effects

[mr-maxo]

This is related to #105 however I see you are not setting image_id so the fix proposed there would not resolve your issue. How does the platform know which image to use, based off compute_profile_id?

Hi all,

Profile object looks like:

GET /api/compute_profiles/C2M4D20U22
{
  "created_at": "2022-07-19T12:24:53.770Z",
  "updated_at": "2022-07-19T12:24:53.770Z",
  "id": 81,
  "name": "C2M4D20U22",
  "compute_attributes": [
    {
      "id": 3301,
      "name": "2 CPUs and 4 GB memory",
      "compute_resource_id": 1,
      "compute_resource_name": "kvm01",
      "provider_friendly_name": "Libvirt",
      "compute_profile_id": 81,
      "compute_profile_name": "C2M4D20U22",
      "vm_attrs": {
        "cpus": "2",
        "cpu_mode": "default",
        "memory": "4294967296",
        "image_id": "/var/lib/libvirt/images/ubuntu-22.04-server-cloudimg-amd64.img",
        "nics_attributes": {
          "0": {
            "type": "bridge",
            "bridge": "br0",
            "model": "virtio"
          }
        },
        "volumes_attributes": {
          "0": {
            "pool_name": "kvm_images",
            "capacity": "20G",
            "allocation": "0G",
            "format_type": "qcow2"
          }
        }
      },
      "attributes": {
        "cpus": "2",
        "memory": "4294967296",
        "image_id": "/var/lib/libvirt/images/ubuntu-22.04-server-cloudimg-amd64.img",
        "image_name": "ubuntu-22.04-server-cloudimg-amd64.img",
        "volumes_attributes": {
          "0": {
            "capacity": "21474836480",
            "allocation": "0",
            "format_type": "qcow2",
            "pool": "kvm_images"
          }
        },
        "interfaces_attributes": {
          "0": {
            "type": "bridge",
            "model": "virtio",
            "bridge": "br0"
          }
        }
      }
    }
  ]
}

so there is image_id. With method = "image" it was possible for me to easily create a machine (with only a few arguments :))

Thanks for trying to solve this issue, the suggested solutions seem ok to me...

Cheers.

[agriffit79]

@lhw please add me as maintainer. thx.

Regards schema migration, I am not familiar with this concept. Is there any documentation on it?

[lhw]

Yes. There is decent documentation on it here: https://developer.hashicorp.com/terraform/plugin/sdkv2/resources/state-migration You can see it in action here: https://github.com/terraform-coop/terraform-provider-foreman/blob/master/foreman/resource_foreman_host.go#L286 with the then deprecation of the keys mentioned in this issue. Which will now be somewhat reverted. The implied version of state schema is always 0. Thats why the file has a V0 and one without a version which is the current V1.

[thatsk]

@lhw is there any update are we gonna officially publish ?

[lhw]

I don't think anyone has worked on the issue yet. @agriffit79 you have your permissions btw

[thatsk]

As now @agriffit79 has permission he can officially publish new release with bugfiz he has done

[agriffit79]

Sorry, I'm busy on other projects at the moment. Hopefully get back to this in a few weeks.

@thatsk that "bugfix" was only a quick hack to test where the issue is. It's not suitable for production release, it needs a proper resolution (which I don't have time for right now). If you need something right now then I suggest you compile your own provider off that quick fix branch and run that locally.

[thatsk]

@agriffit79 have you got time to take a look? Thank in advance

[bitkeks]

Hi, I'm currently working on the provider to fix some issues. The most pressing issue with the build flag is the danger of resetting an existing host. This happens if Terraform sets the build flag in Foreman to true, after Foreman already built the host and set the build flag to false.

PR #122 switches back to provision_method (was method in older versions) and removes build completely.

We will have to find another way to support use cases where existing machines can be triggered to build = true.

[thatsk]

@agriffit79 pls comment and provide official provider

[agriffit79]

From a quick look the PR makes sense to me. I agree that explicitly setting build=true will cause problems.

However, as per https://github.com/terraform-coop/terraform-provider-foreman/pull/122#discussion_r1199028040 I think some more thought needs to go into when build is set on host creation.

[bitkeks]

Maybe we could collect some more use cases, where the build flag should be enforced in Foreman?

IIRC the flag is internally managed by Foreman on creation of a host, when provision_method is build. After successful installation, it is set to false and the buildStatus is set to 0/BUILT: https://github.com/terraform-coop/terraform-provider-foreman/blob/c66217cc973f5e0876406d2f580470ca72a0e497/foreman/api/host.go#L51-L59

Setting the flag with Terraform to true will re-install the host at the next boot. This of course would not be expected behaviour when using Foreman itself, except you explicitly tell it to rebuild.

[bitkeks]

FTR, as mentioned in https://github.com/terraform-coop/terraform-provider-foreman/discussions/105#discussioncomment-6231787, the original problem with "method = image" getting ignored was based on the fact, that the method argument was still accepted by Terraform (deprecated) but not passed on to Foreman at all.

[agriffit79]

I've started a discussion #125. Would be good if interested parties could comment so we can tie down all the scenarios the plugin should support. Hopefully we can then move forward with an agreed pattern that supports them all without the breakage we've experienced in the last year or so.

[agriffit79]

0.6.0 is released which should resolve these issues. Thanks @bitkeks

[mr-maxo]

In version 0.6.0 everything is functioning as expected. Thanks!