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 crds for continuous benchmark tools #1779

Conversation

vankichi
Copy link
Contributor

@vankichi vankichi commented Aug 25, 2022

Signed-off-by: vankichi kyukawa315@gmail.com

Description:

I have created the crd and rbac for the continuous benchmark operator.

Related Issue:

How Has This Been Tested?:

Environment:

  • Go Version: 1.18.3
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 1.14.7

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

@vdaas-ci
Copy link
Collaborator

[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

@vankichi vankichi force-pushed the feature/pkg-apis-internal/create-conn-bench-operator-crd branch from 396d183 to b5e87e9 Compare August 25, 2022 06:00
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 25, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: efbfd13
Status: ✅  Deploy successful!
Preview URL: https://be41f9a1.vald.pages.dev
Branch Preview URL: https://feature-pkg-apis-internal-cr-u856.vald.pages.dev

View logs

@vankichi vankichi force-pushed the feature/pkg-apis-internal/create-conn-bench-operator-crd branch 3 times, most recently from 0d347c2 to 031b997 Compare August 29, 2022 03:09
@vankichi vankichi marked this pull request as ready for review August 29, 2022 03:22
@vankichi vankichi changed the title [WIP] Add crds for continuous benchmark tools Add crds for continuous benchmark tools Aug 29, 2022
Signed-off-by: vankichi <kyukawa315@gmail.com>
@vankichi vankichi force-pushed the feature/pkg-apis-internal/create-conn-bench-operator-crd branch from 031b997 to 326e699 Compare August 30, 2022 04:20
Signed-off-by: vankichi <kyukawa315@gmail.com>
@codecov
Copy link

codecov bot commented Aug 30, 2022

Codecov Report

Base: 31.62% // Head: 31.67% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (efbfd13) compared to base (f7ccb94).
Patch has no changes to coverable lines.

Additional details and impacted files
@@                         Coverage Diff                          @@
##           feature/continous-benchmark-tool    #1779      +/-   ##
====================================================================
+ Coverage                             31.62%   31.67%   +0.04%     
====================================================================
  Files                                   377      377              
  Lines                                 32243    32243              
====================================================================
+ Hits                                  10197    10213      +16     
+ Misses                                21650    21638      -12     
+ Partials                                396      392       -4     
Impacted Files Coverage Δ
internal/worker/queue.go 98.73% <0.00%> (-1.27%) ⬇️
internal/worker/worker.go 85.15% <0.00%> (+1.56%) ⬆️
...nt/core/ngt/service/vqueue/uninserted_index_map.go 73.22% <0.00%> (+2.73%) ⬆️
...ent/core/ngt/service/vqueue/undeleted_index_map.go 73.88% <0.00%> (+5.55%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

internal/k8s/benchmark/job/job.go Outdated Show resolved Hide resolved
internal/k8s/benchmark/job/job.go Outdated Show resolved Hide resolved
internal/k8s/benchmark/job/job.go Show resolved Hide resolved
internal/k8s/benchmark/operator/operator.go Show resolved Hide resolved
internal/k8s/benchmark/job/job.go Outdated Show resolved Hide resolved
internal/k8s/benchmark/job/option.go Outdated Show resolved Hide resolved
Signed-off-by: vankichi <kyukawa315@gmail.com>
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

Sorry I couldn't find struct which implements k8s.io/apimachinery/pkg/runtime.Object

"k8s.io/apimachinery/pkg/runtime"
)

type BenchmarkJobSpec struct {
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 🐶
fieldalignment: struct with 104 pointer bytes could be 56 (govet)

Port int
}

type BenchmarkDatasetSpec struct {
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 🐶
fieldalignment: struct with 48 pointer bytes could be 32 (govet)

Type string
}

type BenchmarkJob struct {
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 🐶
fieldalignment: struct with 408 pointer bytes could be 400 (govet)

BenchmarkOperatorHealthy = BenchmarkOperatorStatus("Healthy")
)

type BenchmarkOperator struct {
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 🐶
fieldalignment: struct with 328 pointer bytes could be 320 (govet)

@vankichi vankichi force-pushed the feature/pkg-apis-internal/create-conn-bench-operator-crd branch from 42dce0d to e128f80 Compare September 7, 2022 02:09
Port int
}

type BenchmarkDataset struct {
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 🐶
fieldalignment: struct with 48 pointer bytes could be 32 (govet)

Signed-off-by: vankichi <kyukawa315@gmail.com>
@vankichi vankichi force-pushed the feature/pkg-apis-internal/create-conn-bench-operator-crd branch from e128f80 to 9747e30 Compare September 7, 2022 08:33
@vankichi vankichi requested a review from kpango September 8, 2022 06:43
Co-authored-by: Yusuke Kato <kpango@vdaas.org>
func New(opts ...Option) BenchmarkJobWatcher {
r := new(reconciler)
for _, opt := range append(defaultOpts, opts...) {
opt(r)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add error handling for applying option? 🙏

func New(opts ...Option) BenchmarkOperatorWatcher {
r := new(reconciler)
for _, opt := range append(defaultOpts, opts...) {
opt(r)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add error handling for applying option? 🙏

Comment on lines 34 to 36
BenchmarkOperatorNotReady = BenchmarkOperatorStatus("NotReady")
BenchmarkOperatorAvailable = BenchmarkOperatorStatus("Available")
BenchmarkOperatorHealthy = BenchmarkOperatorStatus("Healthy")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
BenchmarkOperatorNotReady = BenchmarkOperatorStatus("NotReady")
BenchmarkOperatorAvailable = BenchmarkOperatorStatus("Available")
BenchmarkOperatorHealthy = BenchmarkOperatorStatus("Healthy")
BenchmarkOperatorNotReady BenchmarkOperatorStatus = "NotReady"
BenchmarkOperatorAvailable BenchmarkOperatorStatus = "Available"
BenchmarkOperatorHealthy BenchmarkOperatorStatus = "Healthy"

Signed-off-by: vankichi <kyukawa315@gmail.com>

func (r *reconciler) AddListOpts(opt client.ListOption) {}

func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, err error) {
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 🐶
named return "res" with type "reconcile.Result" found (nonamedreturns)


func (r *reconciler) AddListOpts(opt client.ListOption) {}

func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, err error) {
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 🐶
named return "res" with type "reconcile.Result" found (nonamedreturns)

@kpango kpango merged commit 1681a74 into feature/continous-benchmark-tool Sep 12, 2022
@kpango kpango deleted the feature/pkg-apis-internal/create-conn-bench-operator-crd branch September 12, 2022 04:50
@vankichi vankichi restored the feature/pkg-apis-internal/create-conn-bench-operator-crd branch September 17, 2022 08:30
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.

5 participants