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

Timezone parameter missing on azurerm_recovery_services_protection_policy_vm #2147

Closed
AndreasMWalter opened this issue Oct 24, 2018 · 8 comments · Fixed by #2404
Closed

Timezone parameter missing on azurerm_recovery_services_protection_policy_vm #2147

AndreasMWalter opened this issue Oct 24, 2018 · 8 comments · Fixed by #2404

Comments

@AndreasMWalter
Copy link

First of all, thank you for adding the resource!

As I have already posted, Azure requires the timezone to be specified, it would help alot if it can be implemented in the resource.

image

Originally posted by @AndreasMWalter in #1978 (comment)

@katbyte
Copy link
Collaborator

katbyte commented Oct 25, 2018

Hi @AndreasMWalter,

The resource will work without the timezone specified, all the portal does is change the time provided to UTC based on the zone.

The documentation needs to be updated and we could add a timezone parameter to the resource, however its not required so is not a high priority at the moment.

@AndreasMWalter
Copy link
Author

I would expect it to do more than just change the time provided, e.g. respect DST changes (Expect: Since they have implemented this for Azure automation), we shall know on monday since DST change will be on Sunday in Europe. However a policy that was configured for 9:30 CET UTC+1 just successfully started a backup at 9:30 CEST UTC+2

image

Even though I do despise people who still have not committed to use UTC, some customers will still want this feature.

Also when I say "we shall know", I mean a colleague of mine since I am switching employers end of this month and will not be in the office on monday, and my Azure account should be blocked then...

@katbyte
Copy link
Collaborator

katbyte commented Nov 9, 2018

@AndreasMWalter,

I would expect it to do more to, however the backend API does not have a timezone option, and the portal simply converts it into UTC and sends that off to the API.

Here is what is exposed to us via the SDK/API 🙁

type SimpleSchedulePolicy struct {
	// ScheduleRunFrequency - Defines the frequency interval (daily or weekly) for the schedule policy. Possible values include: 'ScheduleRunTypeInvalid', 'ScheduleRunTypeDaily', 'ScheduleRunTypeWeekly'
	ScheduleRunFrequency ScheduleRunType `json:"scheduleRunFrequency,omitempty"`
	// ScheduleRunDays - This list is the days of the week when the schedule runs.
	ScheduleRunDays *[]DayOfWeek `json:"scheduleRunDays,omitempty"`
	// ScheduleRunTimes - List of times, during a day, when the schedule runs.
	ScheduleRunTimes *[]date.Time `json:"scheduleRunTimes,omitempty"`
	// ScheduleWeeklyFrequency - The number of times per week the schedule runs.
	ScheduleWeeklyFrequency *int32 `json:"scheduleWeeklyFrequency,omitempty"`
	// SchedulePolicyType - Possible values include: 'SchedulePolicyTypeSchedulePolicy', 'SchedulePolicyTypeLongTermSchedulePolicy', 'SchedulePolicyTypeSimpleSchedulePolicy'
	SchedulePolicyType SchedulePolicyType `json:"schedulePolicyType,omitempty"`
}

@AndreasMWalter
Copy link
Author

AndreasMWalter commented Nov 10, 2018

@katbyte Sorry I am no Go expert, but since I found it in the REST / .NET API/SDK I thought it might be this one?

type AzureIaaSVMProtectionPolicy struct {
    // SchedulePolicy - Backup schedule specified as part of backup policy.
    SchedulePolicy BasicSchedulePolicy `json:"schedulePolicy,omitempty"`
    // RetentionPolicy - Retention policy with the details on backup copy retention ranges.
    RetentionPolicy BasicRetentionPolicy `json:"retentionPolicy,omitempty"`
    // InstantRpRetentionRangeInDays - Instant RP retention policy range in days
    InstantRpRetentionRangeInDays *int32 `json:"instantRpRetentionRangeInDays,omitempty"`
    // TimeZone - TimeZone optional input as string. For example: TimeZone = "Pacific Standard Time".
    TimeZone *string `json:"timeZone,omitempty"`
    // ProtectedItemsCount - Number of items associated with this policy.
    ProtectedItemsCount *int32 `json:"protectedItemsCount,omitempty"`
    // BackupManagementType - Possible values include: 'BackupManagementTypeProtectionPolicy', 'BackupManagementTypeAzureStorage', 'BackupManagementTypeAzureIaasVM', 'BackupManagementTypeAzureSQL', 'BackupManagementTypeAzureWorkload', 'BackupManagementTypeGenericProtectionPolicy', 'BackupManagementTypeMAB'
    BackupManagementType ManagementTypeBasicProtectionPolicy `json:"backupManagementType,omitempty"`
}

https://godoc.org/github.com/Azure/azure-sdk-for-go/services/recoveryservices/mgmt/2017-07-01/backup

@katbyte
Copy link
Collaborator

katbyte commented Nov 12, 2018

Thank you @AndreasMWalter!

Good find, we are currently using the 2016-06-1 whitch doesn't have that property:

// AzureIaaSVMProtectionPolicy azure VM (also known as IaaS VM) workload-specific backup policy.
type AzureIaaSVMProtectionPolicy struct {
	// SchedulePolicy - The backup schedule specified as part of backup policy.
	SchedulePolicy BasicSchedulePolicy `json:"schedulePolicy,omitempty"`
	// RetentionPolicy - The retention policy with the details on backup copy retention ranges.
	RetentionPolicy BasicRetentionPolicy `json:"retentionPolicy,omitempty"`
	// ProtectedItemsCount - The number of items associated with this policy.
	ProtectedItemsCount *int32 `json:"protectedItemsCount,omitempty"`
	// BackupManagementType - Possible values include: 'BackupManagementTypeProtectionPolicy', 'BackupManagementTypeAzureIaasVM', 'BackupManagementTypeMAB', 'BackupManagementTypeAzureSQL'
	BackupManagementType ManagementTypeBasicProtectionPolicy `json:"backupManagementType,omitempty"`
}

I'll give upgrading the API version we are using a go and see if it works 🙂

@katbyte
Copy link
Collaborator

katbyte commented Nov 28, 2018

@AndreasMWalter,

I have added the timezone parameter in #2404 🙂

@AndreasMWalter
Copy link
Author

@AndreasMWalter,

I have added the timezone parameter in #2404 🙂

Cheers! I will inform my ex-colleagues, so they can continue their testing.

@ghost
Copy link

ghost commented Mar 5, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants