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

Add geo-replications to svc and ocp ACRs #995

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Add geo-replications to svc and ocp ACRs #995

merged 1 commit into from
Jan 6, 2025

Conversation

jfchevrette
Copy link
Collaborator

@jfchevrette jfchevrette commented Dec 17, 2024

What this PR does

  • Add an acr-replication bicep module used to set up a replication
  • Use the acr-replication module in region.bicep to deploy ACR replications for both the svc and ocp ACRs
  • An ACR replication cannot exist in the same region as the parent ACR and the module usage definitions checks for that

Jira: https://issues.redhat.com/browse/ARO-13411
Link to demo recording:

Special notes for your reviewer

@jfchevrette
Copy link
Collaborator Author

@microsoft-github-policy-service agree company="Red Hat"

@jfchevrette jfchevrette force-pushed the acr-replica branch 2 times, most recently from f9ad4d0 to 27b48f4 Compare December 17, 2024 01:17
@jfchevrette jfchevrette changed the title Add ACR replica in global scope WIP: Add ACR replica in global scope Dec 17, 2024
@jfchevrette jfchevrette marked this pull request as draft December 17, 2024 16:46
@jfchevrette jfchevrette changed the title WIP: Add ACR replica in global scope WIP: Add geo-replication to svc and ocp ACRs Dec 18, 2024
@jfchevrette jfchevrette changed the title WIP: Add geo-replication to svc and ocp ACRs WIP: Add geo-replications to svc and ocp ACRs Dec 18, 2024
@jfchevrette jfchevrette changed the title WIP: Add geo-replications to svc and ocp ACRs Add geo-replications to svc and ocp ACRs Dec 19, 2024
@jfchevrette jfchevrette marked this pull request as ready for review December 19, 2024 21:12
Copy link
Collaborator

@SudoBrendan SudoBrendan left a comment

Choose a reason for hiding this comment

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

I think this work is well done, thanks! I don't have questions on the code, only on Acceptance Criteria. The jira is a bit vague so everything here fits well within that, but I imagine we will eventually want replication to every Azure region we support, meaning we'd need an array of locations, which I think would come from our global config file Gerd has stood up, if I'm understanding all of this correctly 😅

At minimum I imagine we'd want that for the OCP ACR so that cluster installs aren't pulling images over an ocean and affecting install/repair times of clusters, right? SVC might be more lenient... but I could also see replication playing a role in our services' resiliency (e.g. we don't want a regional outage in data center A affecting data center B's resiliency - there might actually be MSFT requirements around this (?))

So I guess all that to say - this PR looks great, but what are we trying to build? What do we need to be successful as a service with our replication? Has a DDR been made (do we even need one, or just make that choice here)?

Thanks again for the hard work 👍

@tony-schndr
Copy link
Collaborator

@SudoBrendan the region template contains the replication resource that references the ACRs (ocp/svc) in the global resource group. Anytime we deploy a region we will get replication without having to maintain a separate list. No reason to maintain a separate list with it implemented this way. There is a DDR btw, SD-DDR-0044, not much is said about global replication besides the ACRs should be replicated in every region the service is provided in.

Copy link
Collaborator

@tony-schndr tony-schndr left a comment

Choose a reason for hiding this comment

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

Nice work @jfchevrette, lgtm

@jfchevrette
Copy link
Collaborator Author

Thank you both for the reviews!

@jfchevrette jfchevrette merged commit 853772c into main Jan 6, 2025
19 checks passed
@jfchevrette jfchevrette deleted the acr-replica branch January 6, 2025 13:49
@geoberle
Copy link
Collaborator

geoberle commented Jan 7, 2025

well done. thank you

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.

4 participants