-
Notifications
You must be signed in to change notification settings - Fork 15
[wip] SmartState analysis POC #119
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
base: main
Are you sure you want to change the base?
Conversation
41c138c to
fcbc9c1
Compare
the container exposes a service that recieves the cluster credentials, runs the smartstate and will return the results as a bulk when done. Signed-off-by: borod108 <[email protected]>
fcbc9c1 to
f668069
Compare
| VERBOSE ?= false | ||
| MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/kubev2v/migration-planner-agent | ||
| MIGRATION_PLANNER_API_IMAGE ?= quay.io/kubev2v/migration-planner-api | ||
| MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/bodnopoz/migration-planner-agent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please keep the defaul kubev2v
| MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/kubev2v/migration-planner-agent | ||
| MIGRATION_PLANNER_API_IMAGE ?= quay.io/kubev2v/migration-planner-api | ||
| MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/bodnopoz/migration-planner-agent | ||
| MIGRATION_PLANNER_API_IMAGE ?= quay.io/bodnopoz/migration-planner-api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
| id: id, | ||
| jwt: jwt, | ||
| config: config, | ||
| healthCheckStopCh: make(chan chan any), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make a separate PR for this.
| credUrl string | ||
| id uuid.UUID | ||
| jwt string | ||
| config *config.Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the file formatted? gopls maybe?
| require 'json' | ||
| require 'manageiq-smartstate' | ||
| # boot.rb | ||
| $LOAD_PATH << File.expand_path("/home/bodnopoz/work/manageiq-gems-pending/lib", __FILE__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, this needs to be fixed?
| continue | ||
| } | ||
|
|
||
| nsc := service.NewSmartStateClient() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be moved outside the loop.
| } | ||
|
|
||
| nsc := service.NewSmartStateClient() | ||
| zap.S().Debug("trying to get smart state results") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will pollute the log with an entry that does not add value to the logs.
|
|
||
| type SmartStateOption func(client *SmartStateClient) | ||
|
|
||
| func NewSmartStateClient(opts ...SmartStateOption) SmartStateClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should return a pointer here. the methods are attached to a pointer to the type.
| } | ||
| } | ||
|
|
||
| func RemoveProtocolAndRoute(rawURL string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be private
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode >= 200 && resp.StatusCode < 300 { | ||
| fmt.Println("Request succeeded with status:", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the logger
| return nil | ||
| } | ||
|
|
||
| return fmt.Errorf("request failed with status: %d", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a suggestion. It is common for the last return to be successful.
| return nil, fmt.Errorf("failed to decode JSON response: %w", err) | ||
| } | ||
|
|
||
| fmt.Println("Request succeeded with status:", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the logger
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode == 202 { | ||
| fmt.Println("Request in progress. Status:", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the logger
|
@tupyy what should we do with that PR? |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Add collection of additional data about the cluster using ManageIQ Smart State tool. This information is very invasive, it includes accounts, installed software etc. So this is just a POC which will be modified later to avoid gathering information we do not need. Also it takes a lot of time, about 30 seconds per VM on average.