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

Implement offline backup #229

Merged
merged 26 commits into from Dec 4, 2017

Conversation

@diptadas
Copy link
Contributor

commented Nov 16, 2017

Fixes #225 #216

Changes:

  • Rename package schedule to backup and add offline flag
  • Use init container for offline backup instead of sidecar
  • Add check command
  • Create check job from init container
  • Create cronjob from restic watcher to delete workload pods, set restic as cronjob's owner ref, set label app=stash, set annotation restic-name, stash-operation
  • Update cronjob accordingly when restic updated
  • Watch job with label app=stash, delete completed jobs/pods for backup/check/recovery

Issues:

  • Event writing
  • Wait for workload ready in kutil

@diptadas diptadas requested a review from tamalsaha Nov 16, 2017

@diptadas diptadas force-pushed the offline-backup branch from c50f67c to dd44975 Nov 27, 2017

main.go Outdated
@@ -16,7 +17,9 @@ func main() {
defer logs.FlushLogs()

if err := cmds.NewCmdStash(Version).Execute(); err != nil {
log.Infoln("Error in Stash Main:", err)

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

Use log.Fatalln() instead of 2 commands

type Controller struct {
k8sClient kubernetes.Interface
stashClient cs.StashV1alpha1Interface
namespace string

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

Create an Options struct for the string variables

return
}

defer func() {

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

Where is the delay?

ctrl.ElectLeader(stopBackup)
default:
ctrl.SetupAndRun(stopBackup)
if opt.RunOffline {

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

Use an option called runOnce

func (c *StashController) initJobWatcher() {
lw := &cache.ListWatch{ // TODO @ Dipta: only watch stash jobs
ListFunc: func(options metav1.ListOptions) (rt.Object, error) {
return c.k8sClient.BatchV1().Jobs(core.NamespaceAll).List(options)

This comment has been minimized.

job := obj.(*batch.Job)
fmt.Printf("Sync/Add/Update for Job %s\n", job.GetName())

if job.Labels["app"] == util.AppLabelStash && job.Status.Succeeded > 0 {

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

These should be annotations

@@ -165,6 +165,27 @@ func (c *StashController) runResticInjector(key string) error {
d := obj.(*api.Restic)
fmt.Printf("Sync/Add/Update for Restic %s\n", d.GetName())

if d.Spec.Type == api.BackupOffline {

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

I think this should be done at the start of operator and we should check that the image exists and exit if the image is missing. To check image existence use,
https://github.com/k8sdb/apimachinery/blob/3f92c150ed3bedd0d2d19bcc3856360937d06033/pkg/docker/checks.go#L13

if c.Name == StashContainer {
found = true
break
if offline {

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

Instead of offline bool, I think we should pass the backupType

}
return container
}

func CreateSidecarContainer(r *api.Restic, tag string, workload api.LocalTypedReference) core.Container {

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

Where is serviceAccountName created

This comment has been minimized.

Copy link
@diptadas

diptadas Nov 29, 2017

Author Contributor

InitContainer and Sidecar use the same service account as the workload?


err = c.resticCLI.Check()
if err != nil {
c.recorder.Eventf(resource.ObjectReference(), core.EventTypeWarning, eventer.EventReasonFailedToCheck, "Repository check failed for workload %s %s/%s. Reason: %v", c.opt.Workload.Kind, c.opt.Namespace, c.opt.Workload.Name, err)

This comment has been minimized.

Copy link
@tamalsaha

tamalsaha Nov 29, 2017

Contributor

When the line is this long, move each parameter to a separate line. Makes it more readable.

@diptadas diptadas force-pushed the offline-backup branch from df998bd to 3e6c15f Nov 29, 2017

@diptadas diptadas force-pushed the offline-backup branch from 3e6c15f to 71d5be9 Nov 29, 2017

diptadas added 6 commits Nov 29, 2017
Dipta Das

@tamalsaha tamalsaha merged commit 7241c6d into master Dec 4, 2017

@tamalsaha tamalsaha deleted the offline-backup branch Dec 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.