Ingress removal storage svc #224
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xuluna
reviewed
Apr 5, 2023
| cmd.Execute() | ||
|
|
||
| if !gotCalled { | ||
| t.Error("expected DeleteTenant to be called, but it wasn't") |
There was a problem hiding this comment.
Is it DeleteTenant to be called? Also the gotCalled was set to true anyways from the beginning?
There was a problem hiding this comment.
fixed, also noticed this on other karavictl test and updated the error logs
alikdell
previously approved these changes
Apr 6, 2023
xuluna
reviewed
Apr 6, 2023
| @@ -0,0 +1,127 @@ | |||
| package middleware | |||
alikdell
previously approved these changes
Apr 7, 2023
cmd/storage-service/main.go
Outdated
| cfg Config | ||
| ) | ||
|
|
||
| // Config is the configuration details on the tenant-service |
There was a problem hiding this comment.
should this config say its for storage service?
| @@ -0,0 +1,79 @@ | |||
| // Copyright © 2021-2023 Dell Inc., or its subsidiaries. All Rights Reserved. | |||
There was a problem hiding this comment.
Don't need a range here if this file was just added
shaynafinocchiaro
previously approved these changes
Apr 7, 2023
alikdell
previously approved these changes
Apr 7, 2023
IsaiasA1
dismissed stale reviews from alikdell and shaynafinocchiaro
via
9a5ef59
xuluna
approved these changes
Apr 7, 2023
shaynafinocchiaro
approved these changes
Apr 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR removes the storage-service ingress from the RPM, adds storage-service handlers to the proxy-server, updates karavictl to connect to the proxy-server instead of the storage-service, and adds telemetry updates.
Proxy-Server changes:
New service handler at internal/proxy/service_handler.go to handle these paths for karavictl:
POST /proxy/storage/ (create storage, storage info is in request body)
PATCH /proxy/storage/ (update storage, storage info is in request body)
GET /proxy/storage/ (list or get storage, storage name is in query parameter)
PATCH /proxy/storage/ (storage info is in request body)
The handlers for each path will parse the request and make the appropriate grpc request to the storage service.
Adds grpc connection interceptors which will automatically create tracing spans for grpc client requests.
New http middleware called TelemetryMW. This will log the time taken for the next handler to complete and record the error in the span. The telemetry will happen only if the next handler is the type HandlerWithError. This handler returns an error specifically for recording in the span.
type HandlerWithError func(w http.ResponseWriter, r *http.Request) error
karavictl changes:
karavictl commands that previously connected to the storage-service via grpc now connect to the proxy-server via https
New file cmd/karavictl/cmd/api/api.go which is a generic http client inspired by https://github.com/dell/goscaleio/blob/main/api/api.go
karavictl will build this generic http client and execute the appropriate request
Storage-service changes:
Adds grpc server interceptors which will automatically create tracing spans for incoming requests.
Adds telemetry middleware at internal/storage-service/middleware/storage_telemetry.go to log the time taken for the request, log request info, and set the request info in the span
Most if not all code is similar to Tenant-service changes made by Aaron Tye
GitHub Issues
List the GitHub issues impacted by this PR:
Checklist:
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration
Powerflex E2E Test, and manual test (Containerized, RPM, and CSM-Operator)