register vmware source functionality #2
Conversation
|
LGTM |
|
@ggCohesity can you review this please ? |
|
I can, It will take some time, As I am still on a learning curve for GoLang.
I will be able to complete all basics by 23(Monday)
Is that ok?
Thanks,
Gaurav
…On Tue, Dec 17, 2019 at 11:47 PM Ashish Bhat ***@***.***> wrote:
@ggCohesity <https://github.com/ggCohesity> can you review this please ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2?email_source=notifications&email_token=AK4RRN3YYNAJE62XITXJJ2TQZEJSDA5CNFSM4JWF6542YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHDOXAQ#issuecomment-566684546>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AK4RRN2N46BCWXBQUDY6IDTQZEJSDANCNFSM4JWF654Q>
.
|
| return nil | ||
| } | ||
|
|
||
| func resourceCohesityJobRunUpdate(resourceData *schema.ResourceData, configMetaData interface{}) error { |
There was a problem hiding this comment.
Update only means increasing snapshot retention right ?
There was a problem hiding this comment.
no, for now we are supporting only start and stop the run
| return nil | ||
| } | ||
|
|
||
| func resourceCohesityJobRunDelete(resourceData *schema.ResourceData, configMetaData interface{}) error { |
There was a problem hiding this comment.
Can this be cancel job run ?
There was a problem hiding this comment.
the cancel is based on the state user provides, its is done in either create/update function, the delete or terraform destroy wouldn't delete/stop the run
| Description: `Specifies the time to wait in minutes for the protection job run | ||
| to complete the run or stop the run`, | ||
| }, | ||
| "timestamp": { |
There was a problem hiding this comment.
Where are we populating this ?
There was a problem hiding this comment.
this is given by the user, in the configuration the user has to use an existing terraform function timestamp()
| } | ||
| //start or stop the job run and wait for completion or operation timeout | ||
| if resourceData.Get("state").(string) == "start" { | ||
| if len(response) == 0 || (response[0].BackupRun.Status != models.StatusBackupRun_KACCEPTED && response[0].BackupRun.Status != models.StatusBackupRun_KRUNNING) { |
There was a problem hiding this comment.
reduce line length. Go doesn't have any ?
There was a problem hiding this comment.
changed this in the later commit
| requestParams.RunType = models.RunTypeRunProtectionJobParam_KSYSTEM | ||
| } | ||
| err = client.ProtectionJobs().CreateRunProtectionJob(jobID, &requestParams) | ||
| time.Sleep(30 * time.Second) |
There was a problem hiding this comment.
here is the situation I faced, for a newly created protection job without any runs, when we trigger a run and try to get the runs for that job with get runs call, this return empty. It takes time, (race condition). I will reduce the wait time to 2-5 secs
| } | ||
| } | ||
| for timeout > 0 { | ||
| log.Printf("[INFO] Wait for protection job (%s) run completion", jobName) |
There was a problem hiding this comment.
Can we make this a global/user configurable ?
There was a problem hiding this comment.
its user configurable, the operation_timeout argument can be used to configure this
| } | ||
| } | ||
|
|
||
| func jobStartStopUtil(resourceData *schema.ResourceData, configMetaData interface{}) (*int64, error) { |
There was a problem hiding this comment.
Where are we stopping the run ?
There was a problem hiding this comment.
in create and update functions, based on the state the user provides, we start or stop the run
| return nil, errors.New("Failed to authenticate with Cohesity") | ||
| } | ||
|
|
||
| timeout := resourceData.Get("operation_timeout").(int) * WaitTimeToSeconds |
There was a problem hiding this comment.
WaitTimeToSeconds is there in config.go
| Description: `Specifies the time of the protection job run. | ||
| Should be in the format YYYY-MM-DD HH:MM Area/Location`, | ||
| }, | ||
| "vm_names": { |
There was a problem hiding this comment.
Just make sure we have handled duplicate names scenario.
| ForceNew: true, | ||
| Description: "Specifies the environment where the protected source exists", | ||
| }, | ||
| "operation_timeout": { |
There was a problem hiding this comment.
This is with assumption that restore would complete within 2 hours ? can you check with Magneto team once ?
| case "VMware": | ||
| environment = models.EnvironmentGetRestoreTasks_KVMWARE | ||
| case "HyperV": | ||
| environment = models.EnvironmentGetRestoreTasks_KHYPERV |
There was a problem hiding this comment.
Its better to have separate restores for VMWare and HyperV ? since we'll have different parameter set for each.
| []int64{protectionJobID}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) | ||
| if err != nil { | ||
| log.Printf(err.Error()) | ||
| return nil, fmt.Errorf("Error in getting the snapshot object for vm %s", vmName) |
There was a problem hiding this comment.
If Getting snapshot information for 1 VM fails, then all the VMs fail? In actual product I think 1 VM is skipped no ?
| for _, snapshotObjectVersion := range snapshotObject.Versions { | ||
| backupDateTime := strings.Split(time.Unix((*snapshotObjectVersion. | ||
| StartedTimeUsecs)/epochTimestampToSeconds, 0).In(location).String(), " ") | ||
| if backupDateTime[0]+" "+backupDateTime[1][:5] == userDateTime[0]+" "+userDateTime[1] { |
There was a problem hiding this comment.
What about scenario where some vms dont have a snapshot in the backuptimestamp, and some do?
There was a problem hiding this comment.
we fail the complete restore operation
No description provided.