Snapshot and restore volumes by default

[jrnt30]

• Adjusts defaults for backup Creation and Restoration

NOTE: Additionally changes default for replicas on the PV example so that it will bootstrap properly.

[heptibot]

Can one of the admins verify this patch?

[ncdc]

We "cheated" a bit in here to label one node with app=nginx to ensure pods from the deployment all land on the same node. @abiogenesis-now I think it's cleaner this way, and then we can remove the node labeling bit from the docs. WDYT?

[jrnt30]

Landing on the same node wasn’t the problem for me. With the PVC in Minikibe the two replicas are contending for ownership since it is not a storage class that allows of multiple attachments Via the ReadWriteOnce

[ncdc]

Ah, interesting! I think we'll probably go ahead and remove the node labeling bit from that doc, but let's wait for @abiogenesis-now to chime in before doing so.

[jesscodez]

Yeah, I think that makes sense, and it'll simplify the docs a lot. @ncdc @jrnt30 We also do need to make docs changes to cloud-provider-specifics.md:

• Remove the first node-labeling example from the snapshot example instructions (This relates to discussion above)

• Update the basic (no-PVs) example command to include --snapshot-volumes=false and --restore-volumes=false

• It might be clearer to also update the snapshot example section to either specify --snapshot-volumes=true and --restore-volumes=true, or remove the flags entirely since its true by default

[ncdc]

It might be clearer to also update the snapshot example section to either specify --snapshot-volumes=true and --restore-volumes=true, or remove the flags entirely since its true by default

Let's remove them.

[jrnt30]

I have removed the additional notes, thanks!

My preference would be to leave the --snapshot-volumes=false for the basic example to not muddle the waters as Ark works just fine if you don't have PVs but the setting is enabled. If you would still like that change, I can do so.

[ncdc]

Sorry, I was referring to removing the =true ones!

[ncdc]

@jrnt30 thanks for this. Once we resolve the replicas discussion, could you please amend your commits so they're signed off (git rebase --signoff ...... if you have git 2.13+; otherwise you could try something like this (note, I haven't tested it) or do it by hand)?

[ncdc]

ok to test

[ncdc]

LGTM. @skriss PTAL

[jesscodez]

@jrnt30 I think you might have missed adding --snapshot-volumes=false and --restore-volumes=false in lines 289-304 of cloud-provider-specifics.md?

[jesscodez]

Oh @jrnt30 misread your comment, you are intentionally leaving out those false flags to avoid confusing people from previous versions of Ark?

misread earlier comment

[jrnt30]

I left it out intentionally because it isn't necessary, yes. If I were to do a create/restore with the flags set to true but without any PVs, it still "just works". To me, introducing the flag would just muddy the goal of the doc which is a quickstart guide.

That being said, I can add those in explicitly if you think it is adding value or I'm overlooking the importance of said feature.

[ncdc]

I'm fine not including the flags

[skriss]

Probably good to remove the flags from the 2 command snippets here: use cases

@abiogenesis-now also has an updated architecture diagram that removes the flag.

Otherwise LGTM!

[skriss]

Thanks for making the docs changes @jrnt30, LGTM. Before merging, it looks like a download backup commit made its way into this branch that should be pulled out, and you need to rebase.

[jrnt30]

@skriss I do not see the file you are referring to, can you be more specific perhaps?

[jrnt30]

Oh...right, wrong branch... Yes, I will correct this

[ncdc]

@jrnt30 please rebase

[ncdc]

LGTM. @abiogenesis-now & @skriss, please take 1 more look. Thanks!

[skriss]

LGTM.