Skip to content

Commit 75e617d

Browse files
super-harshsuper-harsh
authored
Fix BackupVaults/BackupInstance extension to exit early on terminal states (#4180)
* Fix BackupVaults/BackupInstance extension to exit early on terminal states * Update test recording and minor changes * Update recording * Update comment with swagger link
1 parent a5a2d63 commit 75e617d

File tree

3 files changed

+3867
-1457
lines changed

3 files changed

+3867
-1457
lines changed

‎v2/api/dataprotection/customizations/backup_vaults_backup_instance_extensions.go

Lines changed: 66 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,30 @@ import (
1919
"github.com/Azure/azure-service-operator/v2/internal/genericarmclient"
2020
. "github.com/Azure/azure-service-operator/v2/internal/logging"
2121
"github.com/Azure/azure-service-operator/v2/internal/resolver"
22+
"github.com/Azure/azure-service-operator/v2/internal/set"
2223
"github.com/Azure/azure-service-operator/v2/internal/util/to"
2324
"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
2425
"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
2526
)
2627

2728
var _ extensions.PostReconciliationChecker = &BackupVaultsBackupInstanceExtension{}
2829

29-
var protectionError = "ProtectionError"
30+
// These are the states on which there's no future change possible, hence best to return PostReconcileCheck success on them.
31+
// Then further we let Operator decide what condition to put on the resource.
32+
// These terminal states are determined and confirmed by the dataprotection team from the swagger below:
33+
// https://github.com/Azure/azure-rest-api-specs/blob/a651ba25cda4eec698a3a4e35f867ecc2681d126/specification/dataprotection/resource-manager/Microsoft.DataProtection/stable/2023-11-01/dataprotection.json#L5014
34+
var terminalStates = set.Make(
35+
"configuringprotectionfailed",
36+
"invalid",
37+
"notprotected",
38+
"protectionconfigured",
39+
"softdeleted",
40+
"protectionstopped",
41+
"backupschedulessuspended",
42+
"retentionschedulessuspended",
43+
)
44+
45+
var protectionError = "protectionerror"
3046

3147
const (
3248
BackupInstancePollerResumeTokenAnnotation = "serviceoperator.azure.com/bi-poller-resume-token"
@@ -80,69 +96,73 @@ func (extension *BackupVaultsBackupInstanceExtension) PostReconcileCheck(
8096
}
8197

8298
protectionStatus := *backupInstance.Status.Properties.ProtectionStatus.Status
99+
protectionStatus = strings.ToLower(protectionStatus)
83100
log.V(Debug).Info(fmt.Sprintf("Protection Status is %q", protectionStatus))
84101

85-
// We only want to continue if protectionStatus == ProtectionError.
86-
if !strings.EqualFold(protectionStatus, protectionError) {
102+
// Return success if the status is in a terminal state
103+
if terminalStates.Contains(protectionStatus) {
87104
log.V(Debug).Info("Returning PostReconcileCheckResultSuccess")
88105
return next(ctx, obj, owner, resolver, armClient, log)
89106
}
90107

91-
// call sync api only when protection status is ProtectionError and error code is usererror
92-
var protectionStatusErrorCode string
93-
protectionStatusErrorCode = strings.ToLower(*backupInstance.Status.Properties.ProtectionStatus.ErrorDetails.Code)
94-
log.V(Debug).Info(fmt.Sprintf("Protection Error code is %q", protectionStatusErrorCode))
108+
if protectionStatus == protectionError {
109+
// call sync api only when protection status is ProtectionError and error code is usererror
110+
var protectionStatusErrorCode string
111+
protectionStatusErrorCode = strings.ToLower(*backupInstance.Status.Properties.ProtectionStatus.ErrorDetails.Code)
112+
log.V(Debug).Info(fmt.Sprintf("Protection Error code is %q", protectionStatusErrorCode))
95113

96-
if protectionStatusErrorCode != "" && strings.Contains(protectionStatusErrorCode, "usererror") {
97-
id, _ := genruntime.GetAndParseResourceID(backupInstance)
98-
subscription := id.SubscriptionID
99-
rg := id.ResourceGroupName
100-
vaultName := id.Parent.Name
114+
if strings.Contains(protectionStatusErrorCode, "usererror") {
115+
id, _ := genruntime.GetAndParseResourceID(backupInstance)
116+
subscription := id.SubscriptionID
117+
rg := id.ResourceGroupName
118+
vaultName := id.Parent.Name
101119

102-
clientFactory, err := armdataprotection.NewClientFactory(subscription, armClient.Creds(), armClient.ClientOptions())
103-
if err != nil {
104-
return extensions.PostReconcileCheckResultFailure("failed to create armdataprotection client"), err
105-
}
120+
clientFactory, err := armdataprotection.NewClientFactory(subscription, armClient.Creds(), armClient.ClientOptions())
121+
if err != nil {
122+
return extensions.PostReconcileCheckResultFailure("failed to create armdataprotection client"), err
123+
}
106124

107-
var parameters armdataprotection.SyncBackupInstanceRequest
108-
parameters.SyncType = to.Ptr(armdataprotection.SyncTypeDefault)
125+
var parameters armdataprotection.SyncBackupInstanceRequest
126+
parameters.SyncType = to.Ptr(armdataprotection.SyncTypeDefault)
109127

110-
// get the resume token from the resource
111-
pollerResumeToken, _ := GetPollerResumeToken(obj, log)
128+
// get the resume token from the resource
129+
pollerResumeToken, _ := GetPollerResumeToken(obj, log)
112130

113-
// BeginSyncBackupInstance is in-progress - poller resume token is available
114-
log.V(Debug).Info("Starting BeginSyncBackupInstance")
131+
// BeginSyncBackupInstance is in-progress - poller resume token is available
132+
log.V(Debug).Info("Starting BeginSyncBackupInstance")
115133

116-
poller, err := clientFactory.NewBackupInstancesClient().BeginSyncBackupInstance(ctx, rg, vaultName, backupInstance.AzureName(), parameters, &armdataprotection.BackupInstancesClientBeginSyncBackupInstanceOptions{
117-
ResumeToken: pollerResumeToken,
118-
})
119-
if err != nil {
120-
return extensions.PostReconcileCheckResultFailure("Failed Polling for BeginSyncBackupInstance to get the result"), err
121-
}
134+
poller, err := clientFactory.NewBackupInstancesClient().BeginSyncBackupInstance(ctx, rg, vaultName, backupInstance.AzureName(), parameters, &armdataprotection.BackupInstancesClientBeginSyncBackupInstanceOptions{
135+
ResumeToken: pollerResumeToken,
136+
})
137+
if err != nil {
138+
return extensions.PostReconcileCheckResultFailure("Failed Polling for BeginSyncBackupInstance to get the result"), err
139+
}
122140

123-
if pollerResumeToken == "" {
124-
resumeToken, resumeTokenErr := poller.ResumeToken()
125-
if resumeTokenErr != nil {
126-
return extensions.PostReconcileCheckResultFailure("couldn't create PUT resume token for resource"), resumeTokenErr
127-
} else {
128-
SetPollerResumeToken(obj, resumeToken, log)
141+
if pollerResumeToken == "" {
142+
resumeToken, resumeTokenErr := poller.ResumeToken()
143+
if resumeTokenErr != nil {
144+
return extensions.PostReconcileCheckResultFailure("couldn't create PUT resume token for resource"), resumeTokenErr
145+
} else {
146+
SetPollerResumeToken(obj, resumeToken, log)
147+
}
129148
}
130-
}
131149

132-
_, pollErr := poller.Poll(ctx)
133-
if pollErr != nil {
134-
return extensions.PostReconcileCheckResultFailure("couldn't create PUT resume token for resource"), pollErr
135-
}
150+
_, pollErr := poller.Poll(ctx)
151+
if pollErr != nil {
152+
return extensions.PostReconcileCheckResultFailure("couldn't create PUT resume token for resource"), pollErr
153+
}
136154

137-
if poller.Done() {
138-
log.V(Debug).Info("Polling is completed")
139-
ClearPollerResumeToken(obj, log)
140-
_, err := poller.Result(ctx)
141-
if err != nil {
142-
return extensions.PostReconcileCheckResultFailure("couldn't create PUT resume token for resource"), err
155+
if poller.Done() {
156+
log.V(Debug).Info("Polling is completed")
157+
ClearPollerResumeToken(obj, log)
158+
_, err := poller.Result(ctx)
159+
if err != nil {
160+
return extensions.PostReconcileCheckResultFailure("couldn't create PUT resume token for resource"), err
161+
}
143162
}
163+
log.V(Debug).Info("Polling is in-progress")
144164
}
145-
log.V(Debug).Info("Polling is in-progress")
146165
}
166+
147167
return extensions.PostReconcileCheckResultFailure("Backup Instance is in non terminal state"), nil
148168
}

‎v2/internal/controllers/crd_dataprotection_backupinstance_20231101_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func Test_DataProtection_BackupInstance_20231101_CRUD(t *testing.T) {
3131
// Create a test resource group and wait until the operation is completed, where the globalTestContext is a global object that provides the necessary context and utilities for testing.
3232
tc := globalTestContext.ForTest(t)
3333
rg := tc.CreateTestResourceGroupAndWait()
34-
contributorRoleId := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/17d1049b-9a84-46fb-8f53-869881c3d3ab", tc.AzureSubscription)
34+
contributorRoleId := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c", tc.AzureSubscription)
3535
readerRoleId := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/acdd72a7-3385-48ef-bd42-f606fba81ae7", tc.AzureSubscription)
3636
saContributorRoleId := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/17d1049b-9a84-46fb-8f53-869881c3d3ab", tc.AzureSubscription)
3737

@@ -55,11 +55,13 @@ func Test_DataProtection_BackupInstance_20231101_CRUD(t *testing.T) {
5555
},
5656
}
5757

58-
backupPolicy := newBackupPolicy20231101(tc, backupVault, "asotestbackuppolicy")
58+
backupPolicy := newBackupPolicy20231101(tc, backupVault, "asotestbp")
5959

6060
// create storage account and blob container
6161

6262
acct := newStorageAccount(tc, rg)
63+
// Not allowed by the policy anymore. Will need to update the storage account and respective tests
64+
acct.Spec.AllowBlobPublicAccess = to.Ptr(false)
6365

6466
blobService := &storage.StorageAccountsBlobService{
6567
ObjectMeta: tc.MakeObjectMeta("blobservice"),

0 commit comments

Comments
 (0)