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: document gcp backup #127

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

docs: document gcp backup #127

wants to merge 11 commits into from

Conversation

marcoieni
Copy link
Member

@marcoieni marcoieni commented Aug 19, 2024

Related to #122
This PR documents the GCP backup I will implement (the document assumes that the feature is already implemented).

This document is taken from the hackmd document shared in zulip.
I slightly edited it to:

  • remove the parts that don't fit for docs, such as the execution plan and the two remaining unresolved questions and pricing calculation. I copied the plan and questions in Back up Rust releases and crates #122 (comment).
  • address some comments (e.g. the region is now in europe)
  • adapt the style (from RFC to docs).

@marcoieni marcoieni force-pushed the docs-document-gcp-backup branch from 3b600ca to a1657f1 Compare August 20, 2024 09:03
@marcoieni marcoieni marked this pull request as ready for review August 20, 2024 10:16
@marcoieni marcoieni force-pushed the docs-document-gcp-backup branch from c577a9b to b16ab33 Compare August 20, 2024 10:17
Copy link
Member

@jdno jdno left a comment

Choose a reason for hiding this comment

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

This is a great starting point and I'm happy to see that we're slowly adding documentation for our services! 🤩

@@ -0,0 +1,132 @@
# GCP backup
Copy link
Member

Choose a reason for hiding this comment

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

I think GCP is an implementation detail and would therefore rename this piece of our infrastructure (i.e. the document). Internally, we've talked about this in terms of "backup of critical (Rust) assets". How about we use a similar name for this "service"?

Copy link
Member Author

Choose a reason for hiding this comment

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

what do you think of aebc9e6 ?

service-catalog/gcp-backup/README.md Outdated Show resolved Hide resolved
service-catalog/gcp-backup/README.md Outdated Show resolved Hide resolved
service-catalog/gcp-backup/README.md Outdated Show resolved Hide resolved
service-catalog/gcp-backup/README.md Outdated Show resolved Hide resolved
service-catalog/gcp-backup/README.md Outdated Show resolved Hide resolved
- In case our data in AWS is deleted, the `infra-admin` team can restore it by:
- copying the data from GCP to AWS using the GCP read-only access.
- restoring the `crates-io-index` bucket from the `db-dump` stored in the `crates-io` bucket. Use [this](https://github.com/rust-lang/crates.io/blob/e0bb0049daa12f5362def463b04febd6c036d315/src/worker/jobs/git.rs#L19-L129) code.
- If the GCP synchronization mechanism breaks, the Infra team can raise a PR to fix the terraform configuration and a GCP admin can apply it.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'd either call the the "(I|i)nfrastructure team" (without a strong preference for capitalization) or the "infra-team", with a preference for the first. We're optimizing for readability here, and imo abbreviations make it more difficult to read long documents. But that's totally a personal opinion of mine...

Copy link
Member Author

Choose a reason for hiding this comment

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

82e3135 👍

- In case our data in AWS is deleted, the `infra-admin` team can restore it by:
- copying the data from GCP to AWS using the GCP read-only access.
- restoring the `crates-io-index` bucket from the `db-dump` stored in the `crates-io` bucket. Use [this](https://github.com/rust-lang/crates.io/blob/e0bb0049daa12f5362def463b04febd6c036d315/src/worker/jobs/git.rs#L19-L129) code.
- If the GCP synchronization mechanism breaks, the Infra team can raise a PR to fix the terraform configuration and a GCP admin can apply it.
Copy link
Member

Choose a reason for hiding this comment

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

Nit (another personal opinion): When talking about Terraform, the product, we should capitalize its name. When talking about terraform, the CLI, we should wrap it in a code "block". (This goes for all product names, e.g. CloudFront further down.)

Copy link
Member Author

Choose a reason for hiding this comment

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

1000b25 👍

@jdno
Copy link
Member

jdno commented Sep 25, 2024

A question that I had while reading the document is what its purpose and target audience are. Using the Diataxis framework as a reference, I feel like the current document mixes a project plan, explanations, points that we should probably expand into how-to guides, and tasks.

Thinking long-term, I think the README.md should probably give a high-level overview of the service, what it tries to achieve, and in only a few sentences how it does that. And then we link to individual documents that provide more information and how-to guides for specific tasks. I think that'll provide a more scalable and easier-to-manage structure, even though the initial complexity will be higher.

What do you think?

@marcoieni
Copy link
Member Author

The original format of the doc reflected the rust rfc process, but I guess that's used for discussions, while here we should adopt the diataxis style. 👍

I extracted FAQ and maintenance in separate docs.

Let me know if you think readme is still longer then it should be.

@marcoieni marcoieni requested a review from jdno September 26, 2024 14:00
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