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

New Resource: azurerm_recovery_services_protected_vm #1637

Merged
merged 9 commits into from
Oct 18, 2018

Conversation

katbyte
Copy link
Collaborator

@katbyte katbyte commented Jul 24, 2018

Blocked by #1978

@katbyte katbyte added this to the 1.11.0 milestone Jul 24, 2018
@katbyte katbyte requested a review from tombuildsstuff July 24, 2018 09:04
@katbyte katbyte force-pushed the r-recovery_services-backupvm branch 2 times, most recently from 6dc56d8 to a5af4ca Compare July 24, 2018 17:55
@katbyte katbyte modified the milestones: 1.11.0, 1.12.0 Jul 25, 2018
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

Left some comments in-line - mostly around how we can make the UX better here (using ID's to be able to infer the names) - but as is this going to have to be held until after the Backup Policy resource exists?

I'm assuming that this is also blocked on
Azure/azure-sdk-for-go#2284?

azurerm/resource_arm_recovery_services_protected_vm.go Outdated Show resolved Hide resolved
azurerm/resource_arm_recovery_services_protected_vm.go Outdated Show resolved Hide resolved
ImportState: true,
ImportStateVerify: true,
},
{ //vault cannot be deleted unless we unregister all backups
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be able to remove this step once the Policy resource exists?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so because as long as there is a VM backing up to the vault it cannot be deleted.

%s

resource "azurerm_recovery_services_protected_vm" "test" {
// location = "${azurerm_resource_group.test.location}"
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this field if we don't need it?

azurerm/resource_arm_recovery_services_protected_vm.go Outdated Show resolved Hide resolved
source = "modules/vm"

resource_group_name = "${azurerm_resource_group.rg.name}"
vm_size = "Standard_A0"
Copy link
Contributor

Choose a reason for hiding this comment

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

would suggest this is an F2 to make the example quicker

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

F2 didn't exist in the region i was testing in

website/azurerm.erb Show resolved Hide resolved
website/docs/r/recovery_services_protected_vm.markdown Outdated Show resolved Hide resolved
@metacpp metacpp requested a review from WodansSon August 3, 2018 00:03
@tombuildsstuff tombuildsstuff modified the milestones: 1.12.0, 1.13.0 Aug 3, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.13.0, 1.14.0 Aug 15, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.14.0, 1.15.0, 1.16.0 Sep 6, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.16.0, 1.17.0 Sep 25, 2018
@katbyte katbyte force-pushed the r-recovery_services-backupvm branch from a5af4ca to 553e600 Compare September 26, 2018 01:46
@ghost ghost added the size/XXL label Sep 26, 2018
@katbyte katbyte force-pushed the r-recovery_services-backupvm branch from 553e600 to 5df6877 Compare September 26, 2018 01:49
@ghost ghost added the size/XXL label Sep 26, 2018
@katbyte katbyte changed the title new resource: azurerm_recovery_services_protected_vm (WIP) New Resource: azurerm_recovery_services_protected_vm Sep 26, 2018
@ghost ghost added the size/XXL label Sep 26, 2018
@ghost ghost added size/XL and removed size/XXL labels Oct 18, 2018
@katbyte katbyte requested a review from a team October 18, 2018 17:07
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Left some comments and would love to see successful acceptance testing before approving as well. Thanks! 😄

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
"strconv"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: goimports 😉

@@ -20,6 +20,9 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

//tim4ezones:
// https://main.recoveryservices.ext.azure.com/api/timeZones
//{"resultArray":[{"displayName":"(UTC-12:00) International Date Line West","id":"Dateline Standard Time","offsetHours":-12,"offsetMinutes":0},{"displayName":"(UTC-11:00) Coordinated Universal Time-11","id":"UTC-11","offsetHours":-11,"offsetMinutes":0},{"displayName":"(UTC-10:00) Aleutian Islands","id":"Aleutian Standard Time","offsetHours":-10,"offsetMinutes":0},{"displayName":"(UTC-10:00) Hawaii","id":"Hawaiian Standard Time","offsetHours":-10,"offsetMinutes":0},{"displayName":"(UTC-09:30) Marquesas Islands","id":"Marquesas Standard Time","offsetHours":-9,"offsetMinutes":-30},{"displayName":"(UTC-09:00) Alaska","id":"Alaskan Standard Time","offsetHours":-9,"offsetMinutes":0},{"displayName":"(UTC-09:00) Coordinated Universal Time-09","id":"UTC-09","offsetHours":-9,"offsetMinutes":0},{"displayName":"(UTC-08:00) Baja California","id":"Pacific Standard Time (Mexico)","offsetHours":-8,"offsetMinutes":0},{"displayName":"(UTC-08:00) Coordinated Universal Time-08","id":"UTC-08","offsetHours":-8,"offsetMinutes":0},{"displayName":"(UTC-08:00) Pacific Time (US & Canada)","id":"Pacific Standard Time","offsetHours":-8,"offsetMinutes":0},{"displayName":"(UTC-07:00) Arizona","id":"US Mountain Standard Time","offsetHours":-7,"offsetMinutes":0},{"displayName":"(UTC-07:00) Chihuahua, La Paz, Mazatlan","id":"Mountain Standard Time (Mexico)","offsetHours":-7,"offsetMinutes":0},{"displayName":"(UTC-07:00) Mountain Time (US & Canada)","id":"Mountain Standard Time","offsetHours":-7,"offsetMinutes":0},{"displayName":"(UTC-06:00) Central America","id":"Central America Standard Time","offsetHours":-6,"offsetMinutes":0},{"displayName":"(UTC-06:00) Central Time (US & Canada)","id":"Central Standard Time","offsetHours":-6,"offsetMinutes":0},{"displayName":"(UTC-06:00) Easter Island","id":"Easter Island Standard Time","offsetHours":-6,"offsetMinutes":0},{"displayName":"(UTC-06:00) Guadalajara, Mexico City, Monterrey","id":"Central Standard Time (Mexico)","offsetHours":-6,"offsetMinutes":0},{"displayName":"(UTC-06:00) Saskatchewan","id":"Canada Central Standard Time","offsetHours":-6,"offsetMinutes":0},{"displayName":"(UTC-05:00) Bogota, Lima, Quito, Rio Branco","id":"SA Pacific Standard Time","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-05:00) Chetumal","id":"Eastern Standard Time (Mexico)","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-05:00) Eastern Time (US & Canada)","id":"Eastern Standard Time","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-05:00) Haiti","id":"Haiti Standard Time","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-05:00) Havana","id":"Cuba Standard Time","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-05:00) Indiana (East)","id":"US Eastern Standard Time","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-05:00) Turks and Caicos","id":"Turks And Caicos Standard Time","offsetHours":-5,"offsetMinutes":0},{"displayName":"(UTC-04:00) Asuncion","id":"Paraguay Standard Time","offsetHours":-4,"offsetMinutes":0},{"displayName":"(UTC-04:00) Atlantic Time (Canada)","id":"Atlantic Standard Time","offsetHours":-4,"offsetMinutes":0},{"displayName":"(UTC-04:00) Caracas","id":"Venezuela Standard Time","offsetHours":-4,"offsetMinutes":0},{"displayName":"(UTC-04:00) Cuiaba","id":"Central Brazilian Standard Time","offsetHours":-4,"offsetMinutes":0},{"displayName":"(UTC-04:00) Georgetown, La Paz, Manaus, San Juan","id":"SA Western Standard Time","offsetHours":-4,"offsetMinutes":0},{"displayName":"(UTC-04:00) Santiago","id":"Pacific SA Standard Time","offsetHours":-4,"offsetMinutes":0},{"displayName":"(UTC-03:30) Newfoundland","id":"Newfoundland Standard Time","offsetHours":-3,"offsetMinutes":-30},{"displayName":"(UTC-03:00) Araguaina","id":"Tocantins Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Brasilia","id":"E. South America Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Cayenne, Fortaleza","id":"SA Eastern Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) City of Buenos Aires","id":"Argentina Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Greenland","id":"Greenland Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Montevideo","id":"Montevideo Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Punta Arenas","id":"Magallanes Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Saint Pierre and Miquelon","id":"Saint Pierre Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-03:00) Salvador","id":"Bahia Standard Time","offsetHours":-3,"offsetMinutes":0},{"displayName":"(UTC-02:00) Coordinated Universal Time-02","id":"UTC-02","offsetHours":-2,"offsetMinutes":0},{"displayName":"(UTC-02:00) Mid-Atlantic - Old","id":"Mid-Atlantic Standard Time","offsetHours":-2,"offsetMinutes":0},{"displayName":"(UTC-01:00) Azores","id":"Azores Standard Time","offsetHours":-1,"offsetMinutes":0},{"displayName":"(UTC-01:00) Cabo Verde Is.","id":"Cape Verde Standard Time","offsetHours":-1,"offsetMinutes":0},{"displayName":"(UTC) Coordinated Universal Time","id":"UTC","offsetHours":0,"offsetMinutes":0},{"displayName":"(UTC+00:00) Casablanca","id":"Morocco Standard Time","offsetHours":0,"offsetMinutes":0},{"displayName":"(UTC+00:00) Dublin, Edinburgh, Lisbon, London","id":"GMT Standard Time","offsetHours":0,"offsetMinutes":0},{"displayName":"(UTC+00:00) Monrovia, Reykjavik","id":"Greenwich Standard Time","offsetHours":0,"offsetMinutes":0},{"displayName":"(UTC+01:00) Amsterdam, Berlin, Bern, Rome, Stockholm, Vienna","id":"W. Europe Standard Time","offsetHours":1,"offsetMinutes":0},{"displayName":"(UTC+01:00) Belgrade, Bratislava, Budapest, Ljubljana, Prague","id":"Central Europe Standard Time","offsetHours":1,"offsetMinutes":0},{"displayName":"(UTC+01:00) Brussels, Copenhagen, Madrid, Paris","id":"Romance Standard Time","offsetHours":1,"offsetMinutes":0},{"displayName":"(UTC+01:00) Sao Tome","id":"Sao Tome Standard Time","offsetHours":1,"offsetMinutes":0},{"displayName":"(UTC+01:00) Sarajevo, Skopje, Warsaw, Zagreb","id":"Central European Standard Time","offsetHours":1,"offsetMinutes":0},{"displayName":"(UTC+01:00) West Central Africa","id":"W. Central Africa Standard Time","offsetHours":1,"offsetMinutes":0},{"displayName":"(UTC+02:00) Amman","id":"Jordan Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Athens, Bucharest","id":"GTB Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Beirut","id":"Middle East Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Cairo","id":"Egypt Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Chisinau","id":"E. Europe Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Damascus","id":"Syria Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Gaza, Hebron","id":"West Bank Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Harare, Pretoria","id":"South Africa Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Helsinki, Kyiv, Riga, Sofia, Tallinn, Vilnius","id":"FLE Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Jerusalem","id":"Israel Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Kaliningrad","id":"Kaliningrad Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Khartoum","id":"Sudan Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Tripoli","id":"Libya Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+02:00) Windhoek","id":"Namibia Standard Time","offsetHours":2,"offsetMinutes":0},{"displayName":"(UTC+03:00) Baghdad","id":"Arabic Standard Time","offsetHours":3,"offsetMinutes":0},{"displayName":"(UTC+03:00) Istanbul","id":"Turkey Standard Time","offsetHours":3,"offsetMinutes":0},{"displayName":"(UTC+03:00) Kuwait, Riyadh","id":"Arab Standard Time","offsetHours":3,"offsetMinutes":0},{"displayName":"(UTC+03:00) Minsk","id":"Belarus Standard Time","offsetHours":3,"offsetMinutes":0},{"displayName":"(UTC+03:00) Moscow, St. Petersburg, Volgograd","id":"Russian Standard Time","offsetHours":3,"offsetMinutes":0},{"displayName":"(UTC+03:00) Nairobi","id":"E. Africa Standard Time","offsetHours":3,"offsetMinutes":0},{"displayName":"(UTC+03:30) Tehran","id":"Iran Standard Time","offsetHours":3,"offsetMinutes":30},{"displayName":"(UTC+04:00) Abu Dhabi, Muscat","id":"Arabian Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Astrakhan, Ulyanovsk","id":"Astrakhan Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Baku","id":"Azerbaijan Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Izhevsk, Samara","id":"Russia Time Zone 3","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Port Louis","id":"Mauritius Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Saratov","id":"Saratov Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Tbilisi","id":"Georgian Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:00) Yerevan","id":"Caucasus Standard Time","offsetHours":4,"offsetMinutes":0},{"displayName":"(UTC+04:30) Kabul","id":"Afghanistan Standard Time","offsetHours":4,"offsetMinutes":30},{"displayName":"(UTC+05:00) Ashgabat, Tashkent","id":"West Asia Standard Time","offsetHours":5,"offsetMinutes":0},{"displayName":"(UTC+05:00) Ekaterinburg","id":"Ekaterinburg Standard Time","offsetHours":5,"offsetMinutes":0},{"displayName":"(UTC+05:00) Islamabad, Karachi","id":"Pakistan Standard Time","offsetHours":5,"offsetMinutes":0},{"displayName":"(UTC+05:30) Chennai, Kolkata, Mumbai, New Delhi","id":"India Standard Time","offsetHours":5,"offsetMinutes":30},{"displayName":"(UTC+05:30) Sri Jayawardenepura","id":"Sri Lanka Standard Time","offsetHours":5,"offsetMinutes":30},{"displayName":"(UTC+05:45) Kathmandu","id":"Nepal Standard Time","offsetHours":5,"offsetMinutes":45},{"displayName":"(UTC+06:00) Astana","id":"Central Asia Standard Time","offsetHours":6,"offsetMinutes":0},{"displayName":"(UTC+06:00) Dhaka","id":"Bangladesh Standard Time","offsetHours":6,"offsetMinutes":0},{"displayName":"(UTC+06:00) Omsk","id":"Omsk Standard Time","offsetHours":6,"offsetMinutes":0},{"displayName":"(UTC+06:30) Yangon (Rangoon)","id":"Myanmar Standard Time","offsetHours":6,"offsetMinutes":30},{"displayName":"(UTC+07:00) Bangkok, Hanoi, Jakarta","id":"SE Asia Standard Time","offsetHours":7,"offsetMinutes":0},{"displayName":"(UTC+07:00) Barnaul, Gorno-Altaysk","id":"Altai Standard Time","offsetHours":7,"offsetMinutes":0},{"displayName":"(UTC+07:00) Hovd","id":"W. Mongolia Standard Time","offsetHours":7,"offsetMinutes":0},{"displayName":"(UTC+07:00) Krasnoyarsk","id":"North Asia Standard Time","offsetHours":7,"offsetMinutes":0},{"displayName":"(UTC+07:00) Novosibirsk","id":"N. Central Asia Standard Time","offsetHours":7,"offsetMinutes":0},{"displayName":"(UTC+07:00) Tomsk","id":"Tomsk Standard Time","offsetHours":7,"offsetMinutes":0},{"displayName":"(UTC+08:00) Beijing, Chongqing, Hong Kong, Urumqi","id":"China Standard Time","offsetHours":8,"offsetMinutes":0},{"displayName":"(UTC+08:00) Irkutsk","id":"North Asia East Standard Time","offsetHours":8,"offsetMinutes":0},{"displayName":"(UTC+08:00) Kuala Lumpur, Singapore","id":"Singapore Standard Time","offsetHours":8,"offsetMinutes":0},{"displayName":"(UTC+08:00) Perth","id":"W. Australia Standard Time","offsetHours":8,"offsetMinutes":0},{"displayName":"(UTC+08:00) Taipei","id":"Taipei Standard Time","offsetHours":8,"offsetMinutes":0},{"displayName":"(UTC+08:00) Ulaanbaatar","id":"Ulaanbaatar Standard Time","offsetHours":8,"offsetMinutes":0},{"displayName":"(UTC+08:30) Pyongyang","id":"North Korea Standard Time","offsetHours":8,"offsetMinutes":30},{"displayName":"(UTC+08:45) Eucla","id":"Aus Central W. Standard Time","offsetHours":8,"offsetMinutes":45},{"displayName":"(UTC+09:00) Chita","id":"Transbaikal Standard Time","offsetHours":9,"offsetMinutes":0},{"displayName":"(UTC+09:00) Osaka, Sapporo, Tokyo","id":"Tokyo Standard Time","offsetHours":9,"offsetMinutes":0},{"displayName":"(UTC+09:00) Seoul","id":"Korea Standard Time","offsetHours":9,"offsetMinutes":0},{"displayName":"(UTC+09:00) Yakutsk","id":"Yakutsk Standard Time","offsetHours":9,"offsetMinutes":0},{"displayName":"(UTC+09:30) Adelaide","id":"Cen. Australia Standard Time","offsetHours":9,"offsetMinutes":30},{"displayName":"(UTC+09:30) Darwin","id":"AUS Central Standard Time","offsetHours":9,"offsetMinutes":30},{"displayName":"(UTC+10:00) Brisbane","id":"E. Australia Standard Time","offsetHours":10,"offsetMinutes":0},{"displayName":"(UTC+10:00) Canberra, Melbourne, Sydney","id":"AUS Eastern Standard Time","offsetHours":10,"offsetMinutes":0},{"displayName":"(UTC+10:00) Guam, Port Moresby","id":"West Pacific Standard Time","offsetHours":10,"offsetMinutes":0},{"displayName":"(UTC+10:00) Hobart","id":"Tasmania Standard Time","offsetHours":10,"offsetMinutes":0},{"displayName":"(UTC+10:00) Vladivostok","id":"Vladivostok Standard Time","offsetHours":10,"offsetMinutes":0},{"displayName":"(UTC+10:30) Lord Howe Island","id":"Lord Howe Standard Time","offsetHours":10,"offsetMinutes":30},{"displayName":"(UTC+11:00) Bougainville Island","id":"Bougainville Standard Time","offsetHours":11,"offsetMinutes":0},{"displayName":"(UTC+11:00) Chokurdakh","id":"Russia Time Zone 10","offsetHours":11,"offsetMinutes":0},{"displayName":"(UTC+11:00) Magadan","id":"Magadan Standard Time","offsetHours":11,"offsetMinutes":0},{"displayName":"(UTC+11:00) Norfolk Island","id":"Norfolk Standard Time","offsetHours":11,"offsetMinutes":0},{"displayName":"(UTC+11:00) Sakhalin","id":"Sakhalin Standard Time","offsetHours":11,"offsetMinutes":0},{"displayName":"(UTC+11:00) Solomon Is., New Caledonia","id":"Central Pacific Standard Time","offsetHours":11,"offsetMinutes":0},{"displayName":"(UTC+12:00) Anadyr, Petropavlovsk-Kamchatsky","id":"Russia Time Zone 11","offsetHours":12,"offsetMinutes":0},{"displayName":"(UTC+12:00) Auckland, Wellington","id":"New Zealand Standard Time","offsetHours":12,"offsetMinutes":0},{"displayName":"(UTC+12:00) Coordinated Universal Time+12","id":"UTC+12","offsetHours":12,"offsetMinutes":0},{"displayName":"(UTC+12:00) Fiji","id":"Fiji Standard Time","offsetHours":12,"offsetMinutes":0},{"displayName":"(UTC+12:00) Petropavlovsk-Kamchatsky - Old","id":"Kamchatka Standard Time","offsetHours":12,"offsetMinutes":0},{"displayName":"(UTC+12:45) Chatham Islands","id":"Chatham Islands Standard Time","offsetHours":12,"offsetMinutes":45},{"displayName":"(UTC+13:00) Coordinated Universal Time+13","id":"UTC+13","offsetHours":13,"offsetMinutes":0},{"displayName":"(UTC+13:00) Nuku'alofa","id":"Tonga Standard Time","offsetHours":13,"offsetMinutes":0},{"displayName":"(UTC+13:00) Samoa","id":"Samoa Standard Time","offsetHours":13,"offsetMinutes":0},{"displayName":"(UTC+14:00) Kiritimati Island","id":"Line Islands Standard Time","offsetHours":14,"offsetMinutes":0}]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this comment still be here?

Check: checkAccAzureRMRecoveryServicesProtectionPolicyVm_basicWeekly(resourceName, ri),
},
{
/*{
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove this comment?

@@ -285,10 +285,10 @@
{
"checksumSHA1": "XiQW2U5GBOhOfPTwFlFeM2thDCs=",
"path": "github.com/Azure/azure-sdk-for-go/services/recoveryservices/mgmt/2016-06-01/backup",
"revision": "2935c0241c74bd8549b843978dd6fc1be6f48b4a",
"revisionTime": "2018-08-31T14:25:13Z",
"version": "=v20.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change expected?

<li<%= sidebar_current("docs-azurerm-recovery-services-vault") %>>
<a href="/docs/providers/azurerm/r/recovery_services_vault.html">azurerm_recovery_services_vault</a>
</li>
<li<%= sidebar_current("docs-azurerm-recovery-services-protection-policy") %>>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this change expected?

@ghost ghost added size/XXL and removed size/XL labels Oct 18, 2018
@ghost ghost added size/XL and removed size/XXL labels Oct 18, 2018
@katbyte katbyte changed the title (WIP) New Resource: azurerm_recovery_services_protected_vm New Resource: azurerm_recovery_services_protected_vm Oct 18, 2018
@katbyte
Copy link
Collaborator Author

katbyte commented Oct 18, 2018

Tests pass:
screen shot 2018-10-18 at 14 46 47
screen shot 2018-10-18 at 14 46 42

Aside from two known failures, these seem to be caused by an API issue

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@katbyte katbyte merged commit cf872ea into master Oct 18, 2018
@katbyte katbyte deleted the r-recovery_services-backupvm branch October 18, 2018 22:04
katbyte added a commit that referenced this pull request Oct 18, 2018
@brenak
Copy link

brenak commented Oct 31, 2018

The documentation for this is incorrect:

https://www.terraform.io/docs/providers/azurerm/r/recovery_services_protected_vm.html

The documentation states location, source_vm_name, and backup_policy_name are required. The code does not have location,source_vm_name or backup_policy_name. It does have backup_policy_id though. Code shows the following as required:

resource_group_name
recovery_vault_name (forces new)
source_vm_id (forces new)

The following as optional:
backup_policy_id (forces new)

@brenak
Copy link

brenak commented Oct 31, 2018

Attempting this with the fields the code currently requries nets an error:

Error creating/updating Recovery Service Protected VM "VM;iaasvmcontainerv2;my_rg;my_vm" (Resource Group "my_rg"): backup.ProtectedItemsClient#CreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="BMSUserErrorInvalidConfigureProtectionRequest" Message="Input for configure protection is not in proper format\r\nPlease ensure that container exists and other parameters are in valid format"

@tombuildsstuff
Copy link
Contributor

@brenak thanks for the feedback - since this PR has shipped, would you mind opening a separate issue for these issues so we can track them? Thanks!

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Oct 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants