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

Additional TieringPolicy and ProtectedItemExtendedInfo details for Microsoft.RecoveryServices version 2021-12-01 #17517

Merged
merged 10 commits into from
Mar 11, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -5454,6 +5454,21 @@
"description": "The oldest backup copy available for this backup item.",
Copy link
Member

Choose a reason for hiding this comment

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

item.

across all tiers?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Member

Choose a reason for hiding this comment

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

consider clarifying that in the description

"type": "string"
},
"oldestRecoveryPointInVault": {
Copy link
Member Author

@anjorsh anjorsh Feb 14, 2022

Choose a reason for hiding this comment

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

Replied above #Closed

"format": "date-time",
"description": "The oldest backup copy available for this backup item in vault tier",
"type": "string"
},
"oldestRecoveryPointInArchive": {
"format": "date-time",
"description": "The oldest backup copy available for this backup item in archive tier",
"type": "string"
},
"newestRecoveryPointInArchive": {
"format": "date-time",
"description": "The latest backup copy available for this backup item in archive tier",
"type": "string"
},
"recoveryPointCount": {
"format": "int32",
"description": "Number of backup copies available for this backup item.",
Expand Down Expand Up @@ -5485,6 +5500,13 @@
"$ref": "#/definitions/RetentionPolicy",
"description": "Retention policy with the details on backup copy retention ranges."
},
"tieringPolicy": {
"description": "Tiering policy to automatically move RPs to another tier\r\nKey is Target Tier, defined in RecoveryPointTierType enum.\r\nTiering policy specifies the criteria to move RP to the target tier.",
Copy link
Member

@rkmanda rkmanda Feb 14, 2022

Choose a reason for hiding this comment

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

Key is Target Tier

If a target tier is not in the dictionary, what does its policy default to? #Closed

Copy link
Member Author

@anjorsh anjorsh Feb 14, 2022

Choose a reason for hiding this comment

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

In a GET call, all supported tiers will be populated by the service in the response. In a CreateOrUpdate call, any target tiers omitted by user in the request will result in service retaining the existing policy for that target tier.

Copy link
Member

Choose a reason for hiding this comment

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

Please call out the default behavior in description

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

Copy link
Member

@rkmanda rkmanda Feb 14, 2022

Choose a reason for hiding this comment

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

Tiering policy specifies the criteria t

Are there any combination of source and target tiers that are not allowed by the service during the move? If so how does the end user discover such combinations? #Closed

Copy link
Member Author

@anjorsh anjorsh Feb 14, 2022

Choose a reason for hiding this comment

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

There is only one source tier and one target tier supported as of now for the move, which will be mentioned in the feature documentation

Copy link
Member

Choose a reason for hiding this comment

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

Please specify the conditions inline or add a link to the documentation. Also when you add support for more combinations, it will be considered a breaking change and you will have to release that as a part of a new API version.

Copy link
Member Author

Choose a reason for hiding this comment

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

The documentation is not available yet. I'll add the conditions inline.

Yes, new combinations will be added in a new API version.

"type": "object",
"additionalProperties": {
Copy link
Member

@mentat9 mentat9 Jan 28, 2022

Choose a reason for hiding this comment

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

additionalProperties

Why is this modeled as additionalProperties? ARM doesn't allow additionalProperties unless the members are unknown, dynamic or user defined. Ref: https://armwiki.azurewebsites.net/api_contracts/guidelines/openapi.html#oapi032-only-use-additionalproperties-when-the-object-properties-are-dynamic-unknown-or-user-defined #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

The tieringPolicy field is a dictionary. As per the OpenAPI specification, dictionaries need to be defined with the additionalProperties keyword.
Ref: https://swagger.io/docs/specification/data-models/dictionaries/

Copy link
Member

Choose a reason for hiding this comment

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

Is the size of the dictionary dynamic? Meaning can the user only specify one element out of all the possible elements?

Copy link
Member Author

@anjorsh anjorsh Feb 14, 2022

Choose a reason for hiding this comment

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

Yes

"$ref": "#/definitions/TieringPolicy"
}
},
"instantRpRetentionRangeInDays": {
"format": "int32",
"description": "Instant RP retention policy range in days",
Expand Down Expand Up @@ -5807,6 +5829,21 @@
"description": "The oldest backup copy available for this backup item.",
"type": "string"
},
"oldestRecoveryPointInVault": {
"format": "date-time",
"description": "The oldest backup copy available for this backup item in vault tier",
"type": "string"
},
"oldestRecoveryPointInArchive": {
"format": "date-time",
"description": "The oldest backup copy available for this backup item in archive tier",
"type": "string"
},
"newestRecoveryPointInArchive": {
"format": "date-time",
"description": "The latest backup copy available for this backup item in archive tier",
"type": "string"
},
"recoveryPointCount": {
"format": "int32",
"description": "Number of backup copies available for this backup item.",
Expand Down Expand Up @@ -9001,6 +9038,13 @@
"retentionPolicy": {
"$ref": "#/definitions/RetentionPolicy",
"description": "Retention policy with the details on backup copy retention ranges."
},
"tieringPolicy": {
Copy link
Member Author

@anjorsh anjorsh Feb 14, 2022

Choose a reason for hiding this comment

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

As mentioned above, 2021-12-01 is a new API version and these changes were missed in an earlier PR for this version. There are currently no consumers for this API version.

Copy link
Member

Choose a reason for hiding this comment

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

You still have to get an approval from the breaking change review board as the API version is exposed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got approval from breaking change review board (attached on email thread)

"description": "Tiering policy to automatically move RPs to another tier.\r\nKey is Target Tier, defined in RecoveryPointTierType enum.\r\nTiering policy specifies the criteria to move RP to the target tier.",
"type": "object",
"additionalProperties": {
"$ref": "#/definitions/TieringPolicy"
}
}
}
},
Expand Down Expand Up @@ -12434,6 +12478,46 @@
}
}
}
},
"TieringPolicy": {
"description": "Tiering Policy for a target tier.\r\nIf the policy is not specified for a given target tier, service retains the existing configured tiering policy for that tier",
"type": "object",
"properties": {
"tieringMode": {
"description": "Tiering Mode to control automatic tiering of recovery points. Supported values are:\r\n1. TierRecommended: Tier all recovery points recommended to be tiered\r\n2. TierAfter: Tier all recovery points after a fixed period, as specified in duration + durationType below.\r\n3. DoNotTier: Do not tier any recovery points",
"enum": [
"Invalid",
Copy link
Member

@rkmanda rkmanda Feb 14, 2022

Choose a reason for hiding this comment

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

Invalid

When would a user specify Invalid? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the default value of the enum and not intended to be explicitly specified by user. Any string value specified by user that is not part of the enum gets translated into "invalid" on service side.

"TierRecommended",
anjorsh marked this conversation as resolved.
Show resolved Hide resolved
"TierAfter",
"DoNotTier"
],
"type": "string",
"x-ms-enum": {
"name": "TieringMode",
"modelAsString": true
}
},
"duration": {
"format": "int32",
Copy link
Member

Choose a reason for hiding this comment

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

int32

Whats the acceptable range for this field?

Copy link
Member Author

Choose a reason for hiding this comment

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

Greater than or equal to zero

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to place a restriction on the max value to catch accidental updates with unintended large values?

Copy link
Member Author

Choose a reason for hiding this comment

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

The service does validate the number before accepting the update, and returns an error code if the check fails. The actual permissible range is dynamically calculated by the service during the update call and can vary based on a number of factors.

Copy link
Member

Choose a reason for hiding this comment

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

If you have external documentation on the rules it will be great to add a link to it in the description

"description": "Number of days/weeks/months/years to retain backups in current tier before tiering.\r\nUsed only if TieringMode is set to TierAfter",
"type": "integer"
},
"durationType": {
"description": "Retention duration type: days/weeks/months/years\r\nUsed only if TieringMode is set to TierAfter",
"enum": [
"Invalid",
anjorsh marked this conversation as resolved.
Show resolved Hide resolved
"Days",
"Weeks",
"Months",
"Years"
],
"type": "string",
"x-ms-enum": {
"name": "RetentionDurationType",
"modelAsString": true
}
}
}
}
},
"parameters": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@
}
}
},
"tieringPolicy": {
"ArchivedRP": {
"tieringMode": "TierRecommended"
}
},
"protectedItemsCount": 0
}
},
Expand Down Expand Up @@ -65,6 +70,13 @@
}
}
},
"tieringPolicy": {
"ArchivedRP": {
"tieringMode": "TierAfter",
"duration": 12,
"durationType": "Months"
}
},
"timeZone": "Pacific Standard Time",
"protectedItemsCount": 0
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@
"containerName": "iaasvmcontainer;iaasvm-rg;iaasvm-1",
"sourceResourceId": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/providers/Microsoft.ClassicCompute/virtualMachines/iaasvm-1",
"policyId": "/Subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/SwaggerTestRg/providers/Microsoft.RecoveryServices/vaults/NetSDKTestRsVault/backupPolicies/testPolicy1",
"lastRecoveryPoint": "2017-11-22T12:25:32.048723Z"
"lastRecoveryPoint": "2017-11-22T12:25:32.048723Z",
"extendedInfo": {
"oldestRecoveryPoint": "2021-01-30T00:09:30.8639311Z",
"oldestRecoveryPointInVault": "2021-01-30T00:09:30.8639311Z",
"oldestRecoveryPointInArchive": "2021-01-30T00:09:30.8639311Z",
"newestRecoveryPointInArchive": "2021-01-30T00:09:30.8639311Z",
"recoveryPointCount": 14,
"policyInconsistent": false
}
}
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@
"durationType": "Days"
}
}
},
"tieringPolicy": {
"ArchivedRP": {
"tieringMode": "TierAfter",
"duration": 60,
"durationType": "Days"
}
}
},
{
Expand Down