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

[Merge pending] Add mirror gateway definition #1948

Closed
wants to merge 39 commits into from

Conversation

hlts2
Copy link
Collaborator

@hlts2 hlts2 commented Feb 9, 2023

Description:

WHAT

Added proto definition for mirror gateway.
The following RPCs are added.

  • Register RPC: register mirror servers.
  • Target RPC: get other mirror servers.

WHY

we need to support request mirroring for BCP

⚠️ FYI

Although there are many differences, most of them are due to automatic generation.
The following is the main addition.

Related PR:

#1792

Versions:

  • Go Version: 1.19.5
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 2.0.9

Checklist:

Special notes for your reviewer:

How to deploy

  1. Update helm repo for monitoring
helm repo update
  1. Create k3d cluster
make k3d/restart
  1. Deploy monitoring stack
 make k8s/external/cert-manager/deploy k8s/monitoring/deploy
  1. Deploy multiple vald clusters
 make k8s/multi/vald/deploy
  1. Deploy MirrorTarget resources
---
apiVersion: vald.vdaas.org/v1
kind: ValdMirrorTarget
metadata:
  name: mirror-target-01
  labels:
    group: mirror-group-01
spec:
  colocation: dc1
  target:
    host: vald-mirror-gateway.vald-01.svc.cluster.local
    port: 8081
---
apiVersion: vald.vdaas.org/v1
kind: ValdMirrorTarget
metadata:
  name: mirror-target-02
  labels:
    group: mirror-group-01
spec:
  colocation: dc1
  target:
    host: vald-mirror-gateway.vald-02.svc.cluster.local
    port: 8081
  1. (E2E test)

Please execute the following command if you want to perform the e2e test.

E2E_TARGET_NAME=vald-mirror-gateway E2E_TARGET_NAMESPACE=vald-03 make e2e

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Feb 9, 2023

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test


Copyright (C) 2019-2022 vdaas.org vald team <[email protected]>

Licensed under the Apache License, Version 2.0 (the "License");
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
Unpaired symbol: ‘)’ seems to be missing (EN_UNPAIRED_BRACKETS)
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-parentheses
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION

@hlts2 hlts2 changed the title [WIP] Add mirror gateway definition Add mirror gateway definition Feb 13, 2023
@hlts2 hlts2 force-pushed the feature/mirror-gateway-definition branch from 59a8a5f to 568db0d Compare March 3, 2023 02:47
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 3, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 08c44fb
Status: ✅  Deploy successful!
Preview URL: https://d80d7551.vald.pages.dev
Branch Preview URL: https://feature-mirror-gateway-defin.vald.pages.dev

View logs

@hlts2 hlts2 force-pushed the feature/mirror-gateway-definition branch 3 times, most recently from 8ece068 to 2909af0 Compare March 8, 2023 08:17
@hlts2 hlts2 force-pushed the feature/mirror-gateway-definition branch 2 times, most recently from 9fdc06b to 9c1ef3f Compare March 17, 2023 02:43
@hlts2 hlts2 force-pushed the feature/mirror-gateway-definition branch 3 times, most recently from bb05061 to 618cc97 Compare April 10, 2023 07:18
@hlts2 hlts2 force-pushed the feature/mirror-gateway-definition branch 4 times, most recently from 11eec16 to 3496ac1 Compare April 17, 2023 06:36
@hlts2 hlts2 force-pushed the feature/mirror-gateway-definition branch from 3496ac1 to a54e74e Compare April 25, 2023 07:43
@vdaas-ci
Copy link
Collaborator

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

ENV APP_NAME mirror

# skipcq: DOK-DL3008
RUN apt-get update && apt-get install -y --no-install-recommends \
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [hadolint] <DL3008> reported by reviewdog 🐶
Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

* fix: returns all connection information in the registration rpc

Signed-off-by: hlts2 <[email protected]>

* fix: lint warnning

Signed-off-by: hlts2 <[email protected]>

* refactor: deleted unnecessary parameter

Signed-off-by: hlts2 <[email protected]>

---------

Signed-off-by: hlts2 <[email protected]>
* feat: add new crud logic

Signed-off-by: hlts2 <[email protected]>

* refactor code

Signed-off-by: hlts2 <[email protected]>

* refactor: error handling logic

Signed-off-by: hlts2 <[email protected]>

* fix: bugfix error handling for rpc

Signed-off-by: hlts2 <[email protected]>

* feat: add test for RemoveByTimestamp

Signed-off-by: hlts2 <[email protected]>

* fix: fails error test

Signed-off-by: hlts2 <[email protected]>

* refactor: test logic

Signed-off-by: hlts2 <[email protected]>

* fix: add new condition and add test case

Signed-off-by: hlts2 <[email protected]>

* refactor code

Signed-off-by: hlts2 <[email protected]>

* fix: error handling and comment

Signed-off-by: hlts2 <[email protected]>

* fix: test case name and test case order

Signed-off-by: hlts2 <[email protected]>

* fix: execute format

Signed-off-by: hlts2 <[email protected]>

* fix: add new comment

Signed-off-by: hlts2 <[email protected]>

---------

Signed-off-by: hlts2 <[email protected]>
@hlts2 hlts2 changed the title Add mirror gateway definition [Merge pending] Add mirror gateway definition Oct 12, 2023
hlts2 and others added 3 commits November 8, 2023 16:08
* fix: update mirror document

Signed-off-by: hlts2 <[email protected]>

* fix: format

Signed-off-by: hlts2 <[email protected]>

* feat: add removeByTimestamp section

Signed-off-by: hlts2 <[email protected]>

* fix: grammar warning

Signed-off-by: hlts2 <[email protected]>

* fix: grammar warning and refactor document

Signed-off-by: hlts2 <[email protected]>

* fix: grammar warning

Signed-off-by: hlts2 <[email protected]>

* feat: add mirror gateway troubleshooting docs

Signed-off-by: hlts2 <[email protected]>

* style: format code with Gofumpt and Prettier

This commit fixes the style issues introduced in 2c492f7 according to the output
from Gofumpt and Prettier.

Details: #2207

* feat: add contents of troubleshooting document

Signed-off-by: hlts2 <[email protected]>

* fix: grammar warning

Signed-off-by: hlts2 <[email protected]>

* fix: grammar

Signed-off-by: hlts2 <[email protected]>

* fix: link path

Signed-off-by: hlts2 <[email protected]>

* fix: invalid link path

Signed-off-by: hlts2 <[email protected]>

* fix: refactor docs

Signed-off-by: hlts2 <[email protected]>

* fix: deleted unnecessary contents

Signed-off-by: hlts2 <[email protected]>

* fix: status handling logic document

Signed-off-by: hlts2 <[email protected]>

* fix: document refactor

Signed-off-by: hlts2 <[email protected]>

* fix: bugfix status handling document

Signed-off-by: hlts2 <[email protected]>

* fix: refactor sentence

Signed-off-by: hlts2 <[email protected]>

---------

Signed-off-by: hlts2 <[email protected]>
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
# gRPC client configuration (overrides defaults.grpc.client)
client: {}
# The duration to register other Mirror Gateways.
register_duration: "1s"
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: Register
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

ctx, cancel := context.WithCancel(context.Background())
eg, egctx := errgroup.New(ctx)

uuid := "test"
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
string test has 26 occurrences, make it a constant (goconst)

// PodName represents the mirror gateway pod name.
PodName string `json:"pod_name" yaml:"pod_name"`
// RegisterDuration represents the duration to register Mirror Gateway.
RegisterDuration string `json:"register_duration" yaml:"register_duration"`
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
tag is not aligned , should be: json:"register_duration" yaml:"register_duration" (tagalign)

UnimplementedValdServerWithMirror vald.UnimplementedValdServerWithMirror
}
type want struct {
wantId *payload.Object_ID
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1003: struct field wantId should be wantID (stylecheck)

UnimplementedValdServerWithMirror: test.fields.UnimplementedValdServerWithMirror,
}

gotId, err := s.Exists(test.args.ctx, test.args.meta)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1003: var gotId should be gotID (stylecheck)

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.

2 participants