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 read replica rotator #2241

Merged
merged 32 commits into from
Nov 29, 2023
Merged

Add read replica rotator #2241

merged 32 commits into from
Nov 29, 2023

Conversation

ykadowak
Copy link
Contributor

@ykadowak ykadowak commented Nov 24, 2023

Description:

This PR implements read replica rotation algorithm. Tests will be added when job is implemented after this PR.

Related Issue:

Versions:

  • Go Version: 1.21.3
  • Docker Version: 20.10.8
  • Kubernetes Version: v1.28.2
  • NGT Version: 2.1.3

Checklist:

Special notes for your reviewer:

@vdaas-ci
Copy link
Collaborator

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

@ykadowak ykadowak changed the title [WIP] Reature/readreplica/rotator Reature/readreplica/rotator Nov 27, 2023
@ykadowak ykadowak changed the title Reature/readreplica/rotator Add read replica rotator Nov 27, 2023
@ykadowak ykadowak requested review from a team, kpango, datelier and vankichi and removed request for a team and kpango November 27, 2023 02:00
datelier
datelier previously approved these changes Nov 27, 2023
Observability *config.Observability `json:"observability" yaml:"observability"`

// TODO:
ReadreplicaRotate *config.ReadreplicaRotate `json:"rotator" yaml:"rotator"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ReadreplicaRotate *config.ReadreplicaRotate `json:"rotator" yaml:"rotator"`
ReadReplicaRotate *config.ReadreplicaRotate `json:"rotator" yaml:"rotator"`

🤔 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +56 to +59
if replicaID == "" {
return nil, fmt.Errorf("readreplica id is empty. it should be set via MY_TARGET_REPLICA_ID env var")
}
r.readReplicaID = replicaID
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to use functional options e.g. func() WithReplicaID(id string) error {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because replicaID is mandatory. Let's merge this first and discuss it later.

@datelier datelier merged commit 7821619 into main Nov 29, 2023
92 of 94 checks passed
@datelier datelier deleted the reature/readreplica/rotator branch November 29, 2023 04:47
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
@vankichi vankichi mentioned this pull request Dec 4, 2023
kmrmt pushed a commit that referenced this pull request Dec 12, 2023
* Implement sample read replica rotation logic

* Add external-snapshotter in go.mod

* Add readreplica initial implementation

* Add service implementation of readreplica rotator

* Add readreplica rotate config

* Refactor to read labels

* replace id to _MY_TARGET_REPLICA_ID_

* Add snapshot k8s client

* Format

* Fix snapshot client initialization

* Add Apache License to main.go

* Use GetConfigOrDie

* Use internal/k8s client

* Refactor

* Use controller-runtime for CRUD in readreplica

* Remove snapshot client and use controller-runtime

* Move LabelSelector into internal/k8s

* Report error to span

* Update go modules

* Remove old example

* Refactor

* Disable exhaustruct for now

* Fix predeclared

* Fix stylecheck

* nolint:gomnd

* Add test template

* Add test for getNewBaseName

* fix misspelling

* Apply format

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

Successfully merging this pull request may close these issues.

4 participants