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

Docs: Recipe Overview - details usage and S3 behavior #826

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tjanssen3
Copy link
Contributor

No description provided.

@tjanssen3 tjanssen3 marked this pull request as ready for review April 26, 2023 19:06
Copy link
Member

@ShyamsundarR ShyamsundarR left a comment

Choose a reason for hiding this comment

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

@tjanssen3 other than the comment regarding not needing recipe if app needs one big backup/restore of all its resources, this PR is good to be merged. If we can clear that out, we can merge it (and if you prefer the nits can be addressed)

* Recipe API managed in a [separate repo](https://github.com/RamenDR/recipe)
* Recipes utilize Velero under the hood
* Many fields are pass-through to create Velero Backup/Restore objects
* If your application can be backed up and restored as one big group, you don’t
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about this, if there is no recipe ref we do not protect any additional resources is what I see, so a recipe is required?


Where an application may have issues with Velero:

1. Velero works well when an application can be backed up and restored as a
Copy link
Member

Choose a reason for hiding this comment

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

(nit) link to CP4D product?

and Restore objects. Recipes have been omitted for clarity and brevity here.

```bash
tjanssen@eyewall4:~$ mc du -r minio-cluster1
Copy link
Member

Choose a reason for hiding this comment

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

(nit) remove tjanssen@eyewall4 (and below)

@tjanssen3 tjanssen3 force-pushed the recipe_documentation branch 2 times, most recently from 05cba77 to 0a8e473 Compare May 9, 2023 17:05
name to the `spec.kubeObjectProtection.recipeRef.name` field.

```yaml
# VRG metadata omitted
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend at least putting this line:

kind: VolumeReplicationGroup

the resources to restore to the appropriate type.

```yaml
# metadata omitted
Copy link
Collaborator

Choose a reason for hiding this comment

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

kind: Recipe

for PVC Selection, rather than what is defined on the VRG.

```yaml
# metadata omitted
Copy link
Collaborator

Choose a reason for hiding this comment

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

kind: Recipe

show the API only.

```yaml
# metadata omitted
Copy link
Collaborator

Choose a reason for hiding this comment

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

kind: Recipe

spec:
groups:
- name: instance-resources
backupRef: instance-resources
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is a restore group I think the backupRef should be omitted for clarity. This option would be removed if capture and recover groups were defined.

name: recipe-busybox
namespace: recipe-test
spec:
appType: busybox
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this appType field do? If nothing, then I suggest omitting. If something, then I suggest explanining it.


```yaml
# metadata omitted for example
# groups, hooks omitted for example
Copy link
Collaborator

Choose a reason for hiding this comment

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

kind: Recipe

- also updated docs/krp.md to reflect removal of CaptureOrder/RecoverOrder structs
- added krp_install.md, which details kubeObjectProtection/Recipe setup and configuration
- removed recipe.md, which contained old Recipe API, in favor of recipe_overview.md

Signed-off-by: Travis Janssen <[email protected]>
Copy link
Collaborator

@hatfieldbrian hatfieldbrian left a comment

Choose a reason for hiding this comment

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

Not finished reviewing, but posting some comments

Comment on lines +7 to +10
1. S3 store (e.g. Minio/Noobaa) installed and configured
1. S3 store credentials
1. Velero/OADP installed and configured
1. Velero credentials
Copy link
Collaborator

Choose a reason for hiding this comment

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

S3 store credentials and Velero credentials actually have to be the same, but their secret formats differ

Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) e.g., minio, noobaa i.e., , instead of /

s3Region: us-east-1
s3SecretRef:
name: minio-s3
VeleroNamespaceSecretKeyRef:
Copy link
Collaborator

Choose a reason for hiding this comment

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

lowercase v: veleroNamespaceSecretKeyRef

Comment on lines +35 to +36
For every s3 profile, two Secrets will need to be created: one with s3 credentials,
one with Velero/OADP credentials. This means for two S3 profiles, four Secrets are
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) I'd say credentials are the same, but secret formats differ

Comment on lines +8 to +13
By default, Kubernetes resources are Captured and Recovered as single, namespaced
groups. Some applications may require strict ordering by type of resource during
Capture or Recover to ensure a successful deployment. To provide this flexibility,
Recipes can be used to describe a Workflow used for a Capture or Recover action.
These Recipe Workflows can be referenced by the VolumeReplicationGroup (VRG), and
executed periodically.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) I recommend lowercase capture and recover

Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) fields names and kinds are good candidates for highlighting by surrounding in backticks. E.g., VolumeReplicationGroup

Comment on lines +20 to +24
1. What is a Recipe and why do we need it?
1. Recipe concepts
1. Sample application that uses a Recipe
1. Additional details on Recipe functionality
1. Where to go to learn more
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) If uppercase Recipe, then I recommend surrounding in backticks, else I recommend lowercase r

executed periodically.

Recipe source code and additional information can be found [here](https://github.com/RamenDR/recipe).
The current Recipe CRD can be found [here](https://github.com/RamenDR/recipe/blob/main/config/crd/bases/ramendr.openshift.io_recipes.yaml).
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) I recommend the raw link for easy downloading

Comment on lines +34 to +35
* If your application can be backed up and restored without specific sequencing,
a Recipe may not not be required.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) backed up and restored -> captured and recovered
(nit) drop period at end since bulleted list

Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend merging this krp.md file or at least linking it from there

Copy link
Collaborator

Choose a reason for hiding this comment

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

I recall a suggestion to store recipe documentation in the recipe repo. I understand recipe processing is done in the VRG controller which is in this repo, but the docs should probably just say what it should do based on the API not how it does it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants