Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support baremetal component #23

Merged
merged 1 commit into from Nov 6, 2019
Merged

Support baremetal component #23

merged 1 commit into from Nov 6, 2019

Conversation

ghost
Copy link

@ghost ghost commented Oct 24, 2019

Data sources:

  • availability zone
  • flavor
  • keypair

Resources:

  • keypair
  • server

ecl/resource_ecl_baremetal_server_v2_test.go Outdated Show resolved Hide resolved
The following attributes are exported:

* `name` - See Argument Reference above.
* `admin_pass` - See Argument Reference above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ECL provider we defined following rules

  • Argument is the section for the parameters which user can specify.
  • Attributes section is for the parameters which has Computed=true.

In current implementation of this resource, no parameters are appeared in attributes section.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it!

ecl/resource_ecl_baremetal_server_v2.go Outdated Show resolved Hide resolved

log.Printf("[DEBUG] Retrieved Server %s: %+v", d.Id(), server)

d.Set("name", server.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you only set name as state is, bare-metal API does not support Update. right?
But this may cause unexpected change in case updating configurations.
Under the condition that terraform only has server name as state, Terraform may not sense configuration change excepting name. (I can not say confidently)

To confirm it, could you please add ACC test to explicitly cause ForceNew change like compute server?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let me check it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed yesterday, I've removed state "name" because there is no update API:)

Data sources:

* availability zone
* flavor
* keypair

Resources:

* keypair
* server
@t-wata t-wata merged commit ccd4c8a into nttcom:master Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants