BIOS update is not idempotent

[brutus333]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform CLI and Terraform Redfish Provider Version

terraform -v
Terraform v0.14.7

Server(s) details and firmware version

PowerEdge R640
iDRAC 9 version 4.20.20.20

Affected Resource(s)

redfish_bios

Terraform Configuration Files

# Copy-paste your Terraform configurations here - for large Terraform configs,
# please use a service like Dropbox and share a link to the ZIP file.
resource "redfish_bios" "bios" {

  redfish_server {
    endpoint = "https://192.168.8.103:6443"
    ssl_insecure = true
  }
  attributes = {
    "SysProfile" = "PerfOptimized"
  }
  settings_apply_time = "OnReset"
  action_after_apply = "ForceRestart"
}

Debug Output

https://gist.github.com/brutus333/950a67e89c0a099356060af3a4ea4f2e

Panic Output

Expected Behavior

Once the configuration is updated TF should not try to change the resource again

Actual Behavior

Terraform tried to update the resource again (even if it was compliant with the definition), so the change is not idempotent as it should be.

Steps to Reproduce

1. terraform apply

Important Factoids

References

• #0000

[brutus333]

I have actually found a way to deal with this but not sure if this is the intended way to use the provider:

data "redfish_bios" "bios" {

  redfish_server {
    endpoint = "https://192.168.8.103:6443"
    ssl_insecure = true
  }
}

locals {
  changed_attrs = {
     SysProfile = "PerfOptimized"
  }
  attributes = merge(data.redfish_bios.bios.attributes, local.changed_attrs)
}


resource "redfish_bios" "bios" {

  redfish_server {
    endpoint = "https://192.168.8.103:6443"
    ssl_insecure = true
  }
  attributes = local.attributes
  settings_apply_time = "OnReset"
  action_after_apply = "ForceRestart"
}

[mikeletux]

Hi @brutus333 ,

Thank you very much for your feedback, really appreciate it.
I had that conversation with @anupamaloke some weeks ago. Also we escalated the way things are handle to Hashicorp to understand that better.

Currently Terraform has a limitation in terms of handling big maps on the state file. Not sure if you're familiar with Terraform development, but either way I'll explain.

When you create a BIOS resource, in the state file (if I don't remember wrong) we actually save the whole BIOS attributes. In your .TF file there are only defined the ones you wish to modify.
Turns out that Terraform does some internal validations when applying, and if the contents of the .TF file and state file does not match, it triggers an update. And that's what is happening. Since you only have one BIOS attribute to change (SysProfile), but in the state file are all of them (that update that you are seeing does not change anything, but I agree it is confusing).

We could change the behavior to make it moreless idempotent following this. Rather than saving the whole list of BIOS attributes, just save the ones you want to modify. That would make Terraform not to trigger an update every time. The thing is, as soon as you add or remove any attribute, an update will be triggered, since .TF file resource and state file one won't match.

What do you think?
Thanks!
/Miguel

When you create a resource, an entry for that resource is created in the state file.

[brutus333]

I looked a bit through the documentation and other providers code and it seems that customdiffs is the way to go:

https://www.terraform.io/docs/extend/resources/customizing-differences.html
https://github.com/terraform-provider-openstack/terraform-provider-openstack/blob/53f886ad12a1abcce778d535b101a91d06911a7c/openstack/networking_subnet_v2.go#L125

I'm sure that other providers are able to deal with partial configuration (I can't recall an example now) so it should be a way to fix this.

On the other hand, I have to say that I don't have experience with writing TF providers, but I can try to contribute to this one.

[mikeletux]

Hey @brutus333 ,

Let me take a look into that and I'll see what can be done there :).
I totally agree with you this needs to be re-thought.

I'll keep this thread open to update when a fix has been implemented. We are also going to reach out Hashicorp to see if we can overcome that with the info you provided.

About contributing, we are open to that, so you'll be more than welcome 😄
Thanks!
/Miguel

[anupamaloke]

I have actually found a way to deal with this but not sure if this is the intended way to use the provider:

data "redfish_bios" "bios" {

  redfish_server {
    endpoint = "https://192.168.8.103:6443"
    ssl_insecure = true
  }
}

locals {
  changed_attrs = {
     SysProfile = "PerfOptimized"
  }
  attributes = merge(data.redfish_bios.bios.attributes, local.changed_attrs)
}


resource "redfish_bios" "bios" {

  redfish_server {
    endpoint = "https://192.168.8.103:6443"
    ssl_insecure = true
  }
  attributes = local.attributes
  settings_apply_time = "OnReset"
  action_after_apply = "ForceRestart"
}

@brutus333, this is an excellent workaround. I am taking a look to see how the custom diff could be implemented for the Bios attributes. In the meantime, I will add your workaround to the example plans.

[grantcurell]

@anupamaloke heads up - I'm working on some optimizations for it. My in progress branch is here: https://github.com/grantcurell/terraform-provider-redfish/tree/18-wait-for-bios-update-job-to-finish

[mikeletux]

@grantcurell don't spend much time on this as I think Anupam had sorted it out already using a custom diff function. Let's sync up tomorrow!

[grantcurell]

@mikeletux it took me a bit to catch up but I just realized what @brutus333 was talking about and why it's happening. Agreed, yes, using a custom diff makes sense to me.