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

Update BRO readme #460

Merged
merged 7 commits into from
Jun 7, 2024

Conversation

mallardduck
Copy link
Member

This PR seeks to simplify the "above the fold" content of our readme. Essentially as a means to improve the experience for any Rancher end-users that end up on this repo. The goal is to optimize for that without removing any beneficial information for any power-users out there.

Change Highlights

  • Adjusts heading levels
  • Simplify Description (reused from Chart)
  • Restructures advanced user info under "More Info" section

@mallardduck mallardduck requested a review from ericpromislow as a code owner May 3, 2024 19:46
@mallardduck mallardduck marked this pull request as draft May 3, 2024 19:47
README.md Outdated Show resolved Hide resolved
@mallardduck mallardduck force-pushed the readme-enhancements branch from 62f18fa to d6b809e Compare May 3, 2024 19:56
@mallardduck mallardduck marked this pull request as ready for review June 7, 2024 15:22
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated

### Use-Cases
- Backing up Rancher before Rancher upgrades and restoring after a failed one.
- Restoring your *Rancher application* to a new cluster in a DR scenario.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need to highlight this

Copy link
Collaborator

Choose a reason for hiding this comment

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

And "DR" should prob be spelled out as it's not defined earlier

Copy link
Member Author

Choose a reason for hiding this comment

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

Will opt to expand DR and include this item - I know it's a topic that comes up in support world. So mainly defining that here for clarity on that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, we need to emphasize that it's the Rancher application getting backed up, and not user data

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
Our default chart is built for the use-case of backing up and restoring the Rancher application.
However, under the hood the Backup Restore Operator is rather flexible extension for backup and restore of Kubernetes resources.

* This operator provides ability to backup and restore Kubernetes applications (metadata) running on any cluster. It accepts a list of resources that need to be backed up for a particular application. It then gathers these resources by querying the Kubernetes API server, packages all the resources to create a tarball file and pushes it to the configured backup storage location. Since it gathers resources by querying the API server, it can back up applications from any type of Kubernetes cluster.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"provides the ability"

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what you mean "for a particular application". Isn't Rancher the the only application it backs up?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, I would insert an Oxford comma like so: packages all the resources to create a tarball file, and pushes it to. -- that's in the Microsoft style guide, so it's ok here

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you mean "for a particular application". Isn't Rancher the the only application it backs up?

Adjusting that to read as "backed up for the application" - since by default "the application" is Rancher. But this section is highlighting that with care and caution that BRO can backup other application data too. Technically I didn't write the wording here, just reorganizing it to a new section. But I like these improvements too!

README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

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

Just nits. No typos (that I found) and the content is an improvement).

@mallardduck mallardduck requested a review from ericpromislow June 7, 2024 21:05
Copy link
Collaborator

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

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

All good now. Thx for fixing this.

@ericpromislow ericpromislow merged commit a800c1e into rancher:release/v5.0 Jun 7, 2024
3 checks passed
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.

2 participants