CP-53859 Implement a new TF resource xenserver_vm_import for VHD #95
Conversation
Signed-off-by: Fei Su <[email protected]>
acefei
force-pushed
the
private/feis/CP-53859
branch
from
647654a to
55ae1e7
Compare
| @@ -0,0 +1,47 @@ | |||
| locals { | |||
There was a problem hiding this comment.
This a good example file , but seems this file type is not right. And more import, customer can't see this example on web documentation.
I suggest we can move this example to examples/resources/xenserver_import_raw_vdi/resource.tf or xamples/resources/xenserver_vm/resource.tf , and give more messages about what this resource used for.
| @@ -0,0 +1,47 @@ | |||
| locals { | |||
There was a problem hiding this comment.
This a good example file , but seems this file type is not right. And more import, customer can't see this example on web documentation.
I suggest we can move this example to examples/resources/xenserver_import_raw_vdi/resource.tf or xamples/resources/xenserver_vm/resource.tf , and give more messages about what this resource used for.
|
|
||
| func (r *importRawVdiResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { | ||
| resp.Schema = schema.Schema{ | ||
| MarkdownDescription: "import Raw/VHD vdi resource.", |
There was a problem hiding this comment.
"Provides a virtual disk image resource which can be imported from Raw/VHD" ?
and does this mean this resource also support .raw type?
| return | ||
| } | ||
|
|
||
| defaultSr, err := getDefaultSR(r.session) |
There was a problem hiding this comment.
Could we allow customer to choose SR like xenserver_vdi resource? add a parameter "SR" in this resource?
| func importRawVdiSchema() map[string]schema.Attribute { | ||
| return map[string]schema.Attribute{ | ||
| "raw_vdi_path": schema.StringAttribute{ | ||
| Description: "The path to the raw VDI file.", |
There was a problem hiding this comment.
Does this path only support .vhd? suggest we can add a support type list here.
|
|
||
| // Configure HTTP client with appropriate timeouts and TLS settings | ||
| // #nosec G402 - InsecureSkipVerify is required for self-signed certificates | ||
| transport := &http.Transport{ |
There was a problem hiding this comment.
It looks good , can we move the client call to a common function? in case we have other task in future.
| } | ||
|
|
||
| // Wait for task completion - remove the unnecessary Sleep | ||
| timeout := 60 * 60 // 10 minutes in seconds |
| if err != nil { | ||
| return errors.New("task failed but couldn't get error info: " + err.Error()) | ||
| } | ||
| return fmt.Errorf("import task failed: %s", errorInfo) |
There was a problem hiding this comment.
the "errorInfo" is a string set, can it be used like this ?
| @@ -611,3 +611,15 @@ func smbResourceModelUpdate(session *xenapi.Session, ref xenapi.SRRef, data smbR | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func getDefaultSR(session *xenapi.Session) (xenapi.SRRef, error) { | |||
There was a problem hiding this comment.
same used in "snapshot_resource"
| return vdiRef, nil | ||
| } | ||
|
|
||
| func removeVDI(session *xenapi.Session, vdiRef xenapi.VDIRef) error { |
There was a problem hiding this comment.
why add this? we already have "cleanupVDIResource"， may be it can be combined?
| @@ -207,3 +209,40 @@ func cleanupVDIResource(session *xenapi.Session, ref xenapi.VDIRef) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| func createVDI(session *xenapi.Session, nameLabel string, fileSize int, srRef xenapi.SRRef) (xenapi.VDIRef, error) { | |||
There was a problem hiding this comment.
this is not used in xenserver/vdi_resource.go, we can move it to xenserver/import_raw_vdi_utils.go
| // check if host, username, password are non-empty | ||
| if host == "" || username == "" || password == "" { | ||
| return nil, errors.New("host, username, password cannot be empty") | ||
| return nil, "", errors.New("host, username, password cannot be empty") |
There was a problem hiding this comment.
the type here may not be correct, you can add
var sessionRef xenapi.SessionRef
add return
return nil, sessionRef, errors.New("host, username, password cannot be empty")
same for below.
| // download the raw vdi file to the local machine | ||
| // and set the VDI_PATH environment variable to its location | ||
| // Example: os.Setenv("VDI_PATH", "/path/to/your/raw.vdi") | ||
| vdi_path := os.Getenv("VDI_PATH") |
There was a problem hiding this comment.
add in README file as you add a new env value.
|
|
||
| func (r *importRawVdiResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { | ||
| resp.Diagnostics.AddError( | ||
| "Update Not Supported", |
There was a problem hiding this comment.
we should also add this "not support update" message to parameter Description. like:
"\n\n-> **Note:** raw_vdi_path is not allowed to be updated.",
| } | ||
|
|
||
| func importRawVdiSchema() map[string]schema.Attribute { | ||
| return map[string]schema.Attribute{ |
There was a problem hiding this comment.
Simply like vdi resource, this resource is also create a vdi in final, should we add some other parameter like “NameLabel ", "NameDescription" .etc?
I‘m thinking , actually this resource is also create a VDI and follow a import task, can we move the "raw_vdi_path" as a option filed in xenserver_vdi resource, and do import when this filed is not empty?
| tflog.Debug(ctx, "Failed to destroy VDI for UUID: "+data.UUID.ValueString()) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Is that possible we can record the raw_vdi_path in vdi other-config , so we can do read and import ?
216508 pass