-
Notifications
You must be signed in to change notification settings - Fork 667
feat(operator-grouping): add operator grouping in topology #4048
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
feat(operator-grouping): add operator grouping in topology #4048
Conversation
ffcffe1 to
b979071
Compare
605d71d to
bca5a1e
Compare
|
/retitle feat(operator-grouping): add operator grouping in topology |
bca5a1e to
ebfca66
Compare
|
/assign @christianvogt |
|
/kind feature |
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.
Use pf variables (https://www.patternfly.org/v4/documentation/overview/global-css-variables) wherever possible.
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.
Move it to the end of imports
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.
So we create operatorBackedServiceKindMap using the installedOperators, if operator is not present in installedOperators then it will not be present in operatorBackedServiceKindMap. Am I missing anything?
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.
we create operatorBackedServiceKindMap(CRD map) out of installedOperators's crds, which can be used to identify the resources created by the crd. But when you install an operator, the operator creates a resource (eg: postgres-operator) which will have ownerReference of the installed operator(csv). We need to group the nodes created by operator & its crds together.
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.
Based on a resource's owner reference, this will choose from installedOperators or its crds operatorBackedServiceKindMap
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.
Got it. Forgot about the case of the operator node :P Then it makes sense to look for the csv itself in the installed operators list.
ebfca66 to
2722570
Compare
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.
Doesn't appear there is any click nor drag action on service, so there shouldn't be a pointer cursor.
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.
The factory is not adding withDndDrop or withDragNode so a lot of these props will not be passed.
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.
removed it.
2722570 to
b156245
Compare
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.
@jeff-phillips-18 FYI, I have this use case where i have data:url from the csv instead of icon name. I believe you have the proper icon alignment in one of your PRs.
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.
thanks, I'll be sure to adjust.
|
I wanna say this is a flake... /test frontend |
eebf39d to
7646978
Compare
7646978 to
74b0a95
Compare
|
/lgtm |
74b0a95 to
9ae363b
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne, christianvogt, divyanshiGupta, jeff-phillips-18, karthikjeeyar, sahil143 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test analyze |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold All the checks are failing atm |
|
/hold cancel Looks like it's good to go again. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
This PR supports Operator backed services grouping in topology.
Todo
cc: @openshift/team-devconsole-ux @serenamarie125 @christianvogt
Fixes: https://issues.redhat.com/browse/ODC-2699