-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added changes in volume object. #228
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,8 @@ const ( | |
vCreatedSite = "created_site" | ||
vReplicationEnabled = "replication_enabled" | ||
vExportCount = "export_count" | ||
vDataCenterName = "data_center_name" | ||
vStoragePoolName = "storage_pool_name" | ||
|
||
// volume Info constants. | ||
vID = "id" | ||
|
@@ -211,6 +213,22 @@ func volumeSchema() map[string]*schema.Schema { | |
Computed: true, | ||
Description: "The number of active exports for this volume", | ||
}, | ||
|
||
vDataCenterName: { | ||
Type: schema.TypeString, | ||
Required: false, | ||
Optional: false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't one of these be true? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will remove this Optional parameter as it is a read only field. |
||
Computed: true, | ||
Description: "Name of the data center where the volume is created on the storage array.", | ||
}, | ||
|
||
vStoragePoolName: { | ||
Type: schema.TypeString, | ||
Required: false, | ||
Optional: false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't one of these be true? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is already a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like vStoragePool is not being populated (in the translation layer). Wonder how it's being used by the client (BMaaS UI). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @karthik-radakrishnan and @dbozzato81, I realized that vStoragePool is getting populated using /available-resources API. And if I remove StoragePoolName then we are back to the same problem where vStoragePool can have a specific data to a particular pod location. And same is true with DataCanterName. I will keep both of them as these are read only parameters. Let me know if there are any concerns. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the vStoragePool definition and looking at the code, it seams it is only used for Volume create i.e. it is not getting populated by the provider by it is given by the user in case vStoragePoolID is not. vStoragePool: {
Type: schema.TypeString,
Required: false,
Optional: true,
Description: "The storage pool of the volume to be created.",
}, So why we can't use it like this?... vStoragePool: {
Type: schema.TypeString,
Optional: true,
Computed: true,
Description: "The storage pool of the volume to be created.",
}, Another question, are we sure that vStoragePoolID is computed and returned correctly for external volumes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @dbozzato81, vStoragePoolID is assigned to externally managed volumes when we create them in the DB from DCC. This poolID is assigned based on the array volume was created on. I have explained above for the vStoragePool - This is fetched based on the vStoragePoolID and available-resources response. And then it is used to create Volume. vStoragePoolName and vDataCenterName are read only fields and if we don't want them to have in terraform then we can discard this PR and make vStoragePool Computed as true |
||
Computed: true, | ||
Description: "Name of the storage pool from where the volume is allocated.", | ||
}, | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
1.3.62 | ||
1.3.63 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this dependency has changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbozzato81, I didn't do anything except the change at line 9 and "go mod tidy".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was part of the new metal-client release: