Skip to content

Commit 2b445ce

Browse files
author
Emruz Hossain
authored
Fix hook defaulting issue
Signed-off-by: Emruz Hossain emruz@appscode.com
2 parents e2f4413 + 12436ad commit 2b445ce

33 files changed

+172
-120
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ require (
4141
kmodules.xyz/openshift v0.24.0
4242
kmodules.xyz/prober v0.24.0
4343
kmodules.xyz/webhook-runtime v0.24.0
44-
stash.appscode.dev/apimachinery v0.22.1-0.20220726122311-2e6ad371979a
44+
stash.appscode.dev/apimachinery v0.22.1-0.20220729111526-719487cd4f0b
4545
)
4646

4747
require (

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,5 +1641,5 @@ sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
16411641
sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
16421642
sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo=
16431643
sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8=
1644-
stash.appscode.dev/apimachinery v0.22.1-0.20220726122311-2e6ad371979a h1:76iDG86HWHZIXqnWJ3tN0PGIiZxkolfMnbMUPUdNfQs=
1645-
stash.appscode.dev/apimachinery v0.22.1-0.20220726122311-2e6ad371979a/go.mod h1:uzKMOt8OxE5YpSjRPR6DCkUA+Wn8McXQNPSaVARB0GM=
1644+
stash.appscode.dev/apimachinery v0.22.1-0.20220729111526-719487cd4f0b h1:v/kZlkOiK/9G/uxluUZ8N66bWEDMv7kj7dOO2CJyfU4=
1645+
stash.appscode.dev/apimachinery v0.22.1-0.20220729111526-719487cd4f0b/go.mod h1:uzKMOt8OxE5YpSjRPR6DCkUA+Wn8McXQNPSaVARB0GM=

‎pkg/backup/backupsession.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ func (c *BackupSessionController) backupHost(inv invoker.BackupInvoker, targetIn
226226
if targetInfo.Hooks != nil && targetInfo.Hooks.PreBackup != nil {
227227
err := c.executePreBackupHook(inv, targetInfo, backupSession)
228228
if err != nil {
229-
klog.Infof("failed to execute preBackup hook. Reason: ", err)
229+
klog.Infof("failed to execute preBackup hook. Reason: %s", err.Error())
230230
return nil
231231
}
232232
}
@@ -301,7 +301,7 @@ func (c *BackupSessionController) electLeaderPod(targetInfo invoker.BackupTarget
301301
rlc,
302302
)
303303
if err != nil {
304-
return fmt.Errorf("failed to create resource lock. Reason: %s", err)
304+
return fmt.Errorf("failed to create resource lock. Reason: %v", err)
305305
}
306306

307307
// use a Go context so we can tell the leader election code when we
@@ -360,7 +360,7 @@ func (c *BackupSessionController) electBackupLeader(backupSession *api_v1beta1.B
360360
rlc,
361361
)
362362
if err != nil {
363-
return fmt.Errorf("error during leader election: %s", err)
363+
return fmt.Errorf("error during leader election: %v", err)
364364
}
365365

366366
// use a Go context so we can tell the leader election code when we
@@ -378,7 +378,10 @@ func (c *BackupSessionController) electBackupLeader(backupSession *api_v1beta1.B
378378
OnStartedLeading: func(ctx context.Context) {
379379
klog.Infoln("Got leadership, preparing for backup")
380380
// run backup process
381-
_ = c.backupHost(inv, targetInfo, backupSession)
381+
err := c.backupHost(inv, targetInfo, backupSession)
382+
if err != nil {
383+
klog.Errorf("failed to backup host. Reason: %v", err)
384+
}
382385

383386
// backup process is complete. now, step down from leadership so that other replicas can start
384387
cancel()
@@ -448,10 +451,12 @@ func (c *BackupSessionController) handleBackupFailure(backupSession *api_v1beta1
448451
func (c *BackupSessionController) handleBackupCompletion(inv invoker.BackupInvoker, targetInfo invoker.BackupTargetInfo, backupSession *api_v1beta1.BackupSession, backupOutput *restic.BackupOutput) error {
449452
// execute hooks at the end of backup completion. no matter if the backup succeed or fail.
450453
defer func() {
451-
if targetInfo.Hooks != nil && targetInfo.Hooks.PostBackup.Handler != nil {
454+
if targetInfo.Hooks != nil &&
455+
targetInfo.Hooks.PostBackup != nil &&
456+
targetInfo.Hooks.PostBackup.Handler != nil {
452457
hookErr := c.executePostBackupHook(inv, targetInfo, backupSession)
453458
if hookErr != nil {
454-
klog.Infof("failed to execute postBackup hook. Reason: ", hookErr)
459+
klog.Infof("failed to execute postBackup hook. Reason: %v", hookErr)
455460
}
456461
}
457462
}()

‎pkg/cmds/create_volumesnapshot.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,9 @@ func (opt *VSoption) createVolumeSnapshot(bsMeta metav1.ObjectMeta, inv invoker.
211211
}
212212

213213
// If postBackup hook is specified, then execute those hooks after backup
214-
if targetInfo.Hooks != nil && targetInfo.Hooks.PostBackup.Handler != nil {
214+
if targetInfo.Hooks != nil &&
215+
targetInfo.Hooks.PostBackup != nil &&
216+
targetInfo.Hooks.PostBackup.Handler != nil {
215217
klog.Infoln("Executing postBackup hooks........")
216218
podName := meta.PodName()
217219
if podName == "" {

‎pkg/cmds/restore_volumesnapshot.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,9 @@ func (opt *VSoption) restoreVolumeSnapshot(targetInfo invoker.RestoreTargetInfo)
240240
})
241241
}
242242
// If postRestore hook is specified, then execute those hooks after restore
243-
if targetInfo.Hooks != nil && targetInfo.Hooks.PostRestore.Handler != nil {
243+
if targetInfo.Hooks != nil &&
244+
targetInfo.Hooks.PostRestore != nil &&
245+
targetInfo.Hooks.PostRestore.Handler != nil {
244246
klog.Infoln("Executing postRestore hooks........")
245247
podName := meta.PodName()
246248
if podName == "" {

‎pkg/cmds/update_status.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
"k8s.io/client-go/kubernetes"
3232
"k8s.io/client-go/tools/clientcmd"
33+
"k8s.io/klog/v2"
3334
)
3435

3536
func NewCmdUpdateStatus() *cobra.Command {
@@ -69,11 +70,11 @@ func NewCmdUpdateStatus() *cobra.Command {
6970
return err
7071
}
7172

72-
if opt.BackupSession != "" {
73-
return opt.UpdateBackupStatusFromFile()
74-
} else {
75-
return opt.UpdateRestoreStatusFromFile()
73+
err = updateStatus(opt)
74+
if err != nil {
75+
klog.Errorln(err)
7676
}
77+
return nil
7778
},
7879
}
7980

@@ -104,3 +105,11 @@ func NewCmdUpdateStatus() *cobra.Command {
104105

105106
return cmd
106107
}
108+
109+
func updateStatus(opt status.UpdateStatusOptions) error {
110+
if opt.BackupSession != "" {
111+
return opt.UpdateBackupStatusFromFile()
112+
} else {
113+
return opt.UpdateRestoreStatusFromFile()
114+
}
115+
}

‎pkg/controller/backup_session.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,6 @@ func (c *StashController) applyBackupSessionReconciliationLogic(session *invoker
155155
return nil
156156
}
157157

158-
if !backupMetricPushed(session.GetConditions()) {
159-
160-
err = c.sendBackupMetrics(inv, session)
161-
if err != nil {
162-
condErr := conditions.SetBackupMetricsPushedConditionToFalse(session, err)
163-
if condErr != nil {
164-
return condErr
165-
}
166-
}
167-
}
168-
169158
// Cleanup old BackupSession according to backupHistoryLimit
170159
if !backupHistoryCleaned(session.GetConditions()) {
171160
err = c.cleanupBackupHistory(session, inv.GetBackupHistoryLimit())
@@ -187,6 +176,16 @@ func (c *StashController) applyBackupSessionReconciliationLogic(session *invoker
187176
}
188177
}
189178

179+
if !backupMetricPushed(session.GetConditions()) {
180+
err = c.sendBackupMetrics(inv, session)
181+
if err != nil {
182+
condErr := conditions.SetBackupMetricsPushedConditionToFalse(session, err)
183+
if condErr != nil {
184+
return condErr
185+
}
186+
}
187+
}
188+
190189
klog.Infof("Skipping processing BackupSession %s/%s. Reason: phase is %q.",
191190
session.GetObjectMeta().Namespace,
192191
session.GetObjectMeta().Name,
@@ -436,7 +435,9 @@ func (c *StashController) ensureBackupJob(inv invoker.BackupInvoker, targetInfo
436435
taskResolver.PreTaskHookInput = make(map[string]string)
437436
taskResolver.PreTaskHookInput[apis.HookType] = apis.PreBackupHook
438437
}
439-
if targetInfo.Hooks != nil && targetInfo.Hooks.PostBackup.Handler != nil {
438+
if targetInfo.Hooks != nil &&
439+
targetInfo.Hooks.PostBackup != nil &&
440+
targetInfo.Hooks.PostBackup.Handler != nil {
440441
taskResolver.PostTaskHookInput = make(map[string]string)
441442
taskResolver.PostTaskHookInput[apis.HookType] = apis.PostBackupHook
442443
}
@@ -690,7 +691,7 @@ func backupExecutor(inv invoker.BackupInvoker, targetInfo invoker.BackupTargetIn
690691

691692
func postBackupHooksExecuted(targetInfo []invoker.BackupTargetInfo, targetStatus []api_v1beta1.BackupTargetStatus) bool {
692693
for _, target := range targetInfo {
693-
if target.Hooks != nil {
694+
if target.Hooks != nil && target.Hooks.PostBackup != nil {
694695
if !postBackupHookExecutedForTarget(target, targetStatus) {
695696
return false
696697
}
@@ -716,7 +717,9 @@ func postBackupHookExecutedForTarget(targetInfo invoker.BackupTargetInfo, target
716717

717718
func globalPostBackupHookExecuted(inv invoker.BackupInvoker, session *invoker.BackupSessionHandler) bool {
718719
backupHooks := inv.GetGlobalHooks()
719-
if backupHooks == nil || backupHooks.PostBackup.Handler == nil {
720+
if backupHooks == nil ||
721+
backupHooks.PostBackup == nil ||
722+
backupHooks.PostBackup.Handler == nil {
720723
return true
721724
}
722725
return kmapi.HasCondition(session.GetConditions(), api_v1beta1.GlobalPostBackupHookSucceeded) &&

‎pkg/controller/restore_session.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -219,20 +219,20 @@ func (c *StashController) applyRestoreInvokerReconciliationLogic(inv invoker.Res
219219
return nil
220220
}
221221

222-
if !restoreMetricsPushed(status.Conditions) {
223-
err = c.sendRestoreMetrics(inv)
222+
if !globalPostRestoreHookExecuted(inv) {
223+
err = c.executeGlobalPostRestoreHook(inv)
224224
if err != nil {
225-
condErr := conditions.SetRestoreMetricsPushedConditionToFalse(inv, err)
225+
condErr := conditions.SetGlobalPostRestoreHookSucceededConditionToFalse(inv, err)
226226
if condErr != nil {
227227
return condErr
228228
}
229229
}
230230
}
231231

232-
if !globalPostRestoreHookExecuted(inv) {
233-
err = c.executeGlobalPostRestoreHook(inv)
232+
if !restoreMetricsPushed(status.Conditions) {
233+
err = c.sendRestoreMetrics(inv)
234234
if err != nil {
235-
condErr := conditions.SetGlobalPostRestoreHookSucceededConditionToFalse(inv, err)
235+
condErr := conditions.SetRestoreMetricsPushedConditionToFalse(inv, err)
236236
if condErr != nil {
237237
return condErr
238238
}
@@ -599,7 +599,9 @@ func (c *StashController) resolveRestoreTask(inv invoker.RestoreInvoker, reposit
599599
taskResolver.PreTaskHookInput = make(map[string]string)
600600
taskResolver.PreTaskHookInput[apis.HookType] = apis.PreRestoreHook
601601
}
602-
if targetInfo.Hooks != nil && targetInfo.Hooks.PostRestore.Handler != nil {
602+
if targetInfo.Hooks != nil &&
603+
targetInfo.Hooks.PostRestore != nil &&
604+
targetInfo.Hooks.PostRestore.Handler != nil {
603605
taskResolver.PostTaskHookInput = make(map[string]string)
604606
taskResolver.PostTaskHookInput[apis.HookType] = apis.PostRestoreHook
605607
}
@@ -734,7 +736,7 @@ func (c *StashController) requeueRestoreInvoker(inv invoker.RestoreInvoker, key
734736

735737
func postRestoreHooksExecuted(inv invoker.RestoreInvoker) bool {
736738
for _, targetInfo := range inv.GetTargetInfo() {
737-
if targetInfo.Hooks != nil {
739+
if targetInfo.Hooks != nil && targetInfo.Hooks.PostRestore != nil {
738740
if !postRestoreHookExecutedForTarget(inv, targetInfo) {
739741
return false
740742
}
@@ -760,7 +762,9 @@ func postRestoreHookExecutedForTarget(inv invoker.RestoreInvoker, targetInfo inv
760762
}
761763

762764
func globalPostRestoreHookExecuted(inv invoker.RestoreInvoker) bool {
763-
if inv.GetGlobalHooks() == nil || inv.GetGlobalHooks().PostRestore.Handler == nil {
765+
if inv.GetGlobalHooks() == nil ||
766+
inv.GetGlobalHooks().PostRestore == nil ||
767+
inv.GetGlobalHooks().PostRestore.Handler == nil {
764768
return true
765769
}
766770
return kmapi.HasCondition(inv.GetStatus().Conditions, api_v1beta1.GlobalPostRestoreHookSucceeded) &&

‎pkg/restore/restore.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,9 @@ func (opt *Options) electRestoreLeader(inv invoker.RestoreInvoker, targetInfo in
136136
func (opt *Options) restoreHost(inv invoker.RestoreInvoker, targetInfo invoker.RestoreTargetInfo) error {
137137
// execute at the end of restore. no matter if the restore succeed or fail.
138138
defer func() {
139-
if targetInfo.Hooks != nil && targetInfo.Hooks.PostRestore.Handler != nil {
139+
if targetInfo.Hooks != nil &&
140+
targetInfo.Hooks.PostRestore != nil &&
141+
targetInfo.Hooks.PostRestore.Handler != nil {
140142
err := opt.executePostRestoreHook(inv, targetInfo)
141143
if err != nil {
142144
klog.Infoln("failed to execute postRestore hook. Reason: ", err)

‎pkg/status/status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (o UpdateStatusOptions) UpdatePostBackupStatus(backupOutput *restic.BackupO
143143
statusErr = errors.NewAggregate([]error{statusErr, err})
144144
}
145145

146-
// create status against the BackupSession for each hosts
146+
// create event against the BackupSession for each hosts
147147
for _, hostStats := range backupOutput.BackupTargetStatus.Stats {
148148
var eventType, eventReason, eventMessage string
149149
if hostStats.Error != "" {

0 commit comments

Comments
 (0)