-
Notifications
You must be signed in to change notification settings - Fork 231
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
Added summary command to kompanion #3180
Added summary command to kompanion #3180
Conversation
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.
Overall LGTM! Just some questions.
/lgtm
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 like this! Can we create a pkg/discovery
instead of utils?
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 could but unless we anticipate adding a lot more than just 1 method for discovery this doesn't seem like its own package. Might make sense to move it from utils.go to discovery.go.
return val, nil | ||
} | ||
|
||
func shouldExclude(name string, excludes []string, includes []string) bool { |
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.
hehe we can probably factor some of these out but non blocking
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.
+1
return cmd | ||
} | ||
|
||
type Result struct { |
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 think this is fine for now but adding some types around the Map[gk, Map[status, count]]] might make this easier to read.
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 it possible to represent this as a 3-uple instead? [<NS,GK,status> => count] ? would this help simplify increment?
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.
Its possible but that then makes other things harder. If you want to count objects in a namespace you now have to walk the entire thrupple map and filter then accrue, rather than just walking the given namespace map and accruing.
const ( | ||
// flag names. | ||
kubeconfigFlag = "kubeconfig" | ||
reportNamePrefixFlag = "report-prefix" | ||
|
||
targetNamespacesFlag = "target-namespaces" | ||
ignoreNamespacesFlag = "exclude-namespaces" | ||
|
||
targetObjectsFlag = "target-objects" | ||
|
||
workerRoutinesFlag = "worker-routines" | ||
) |
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.
might be nice to make some of these common options; can be added as a follow on!
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.
+1
if err != nil { | ||
log.Printf("Got error retrieving status of type gvr %s, %v", gvr, err) | ||
} | ||
gk := gvr.Resource + "." + gvr.Group |
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.
wait I am confused -- is this actually the group kind then ?
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.
Group and Kind should uniquely identifies a type of resource. Version identifies which (of several) access mechanisms you want to use to get that resource. If we include the gvr we will can end up multi-counting the resource. We should probably actually add code in utils.GetResources() to defend against that. I don't believe we are seeing instances of that today because we generally remove the v1alphaX when we add the v1betaY. Also we don't currently have any V1s. However we probably should add some protections to GetResources().
gkMap[status] = count + 1 | ||
} | ||
|
||
func (r *Result) Print() { |
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 think this format is fine but it may be nice to support tabulation, json or other formats in the future.
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.
+1. Even just a CSV format. See if its popular and if it is how users would like to consumer the results.
kompanion summary walks the relevant cluster inspecting cnrm resources. It will sum the count of resources by namespace, type and status. Fix date on the copyright.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm Cool. Thanks! |
c86a410
into
GoogleCloudPlatform:master
kompanion summary walks the relevant cluster inspecting cnrm resources. It will sum the count of resources by namespace, type and status.
Change description
Sample usage
$ ./kompanion summary
2024/11/14 18:51:25 Running kompanion summary with kubeconfig:
2024/11/14 18:51:25 Starting worker threads to process Config Connector objects
2024/11/14 18:51:25 Dumping Config Connector summaries
2024/11/14 18:51:33 Namespace: e2e-655cda
2024/11/14 18:51:33 - GroupKind: iamserviceaccounts.iam.cnrm.cloud.google.com (True: 50)
2024/11/14 18:51:33 - GroupKind: storagebuckets.storage.cnrm.cloud.google.com (True: 100)
2024/11/14 18:51:33 - GroupKind: iampartialpolicies.iam.cnrm.cloud.google.com (True: 152)
2024/11/14 18:51:33 - GroupKind: configconnectorcontexts.core.cnrm.cloud.google.com (no status: 1)
2024/11/14 18:51:33 - GroupKind: containerclusters.container.cnrm.cloud.google.com (True: 1)
2024/11/14 18:51:33 - GroupKind: sqldatabases.sql.cnrm.cloud.google.com (True: 45)
2024/11/14 18:51:33 - GroupKind: sqlusers.sql.cnrm.cloud.google.com (True: 45)
2024/11/14 18:51:33 - GroupKind: containernodepools.container.cnrm.cloud.google.com (True: 4)
2024/11/14 18:51:33 - GroupKind: sqlinstances.sql.cnrm.cloud.google.com (True: 9)
...
2024/11/14 18:51:33 Namespace: e2e-0704d0
2024/11/14 18:51:33 - GroupKind: configconnectorcontexts.core.cnrm.cloud.google.com (no status: 1)
2024/11/14 18:51:33 - GroupKind: containerclusters.container.cnrm.cloud.google.com (True: 1)
2024/11/14 18:51:33 - GroupKind: containernodepools.container.cnrm.cloud.google.com (True: 4)
2024/11/14 18:51:33 - GroupKind: storagebuckets.storage.cnrm.cloud.google.com (True: 100)
2024/11/14 18:51:33 - GroupKind: iampartialpolicies.iam.cnrm.cloud.google.com (True: 152)
2024/11/14 18:51:33 - GroupKind: sqlinstances.sql.cnrm.cloud.google.com (True: 9)
2024/11/14 18:51:33 - GroupKind: iamserviceaccounts.iam.cnrm.cloud.google.com (True: 50)
2024/11/14 18:51:33 - GroupKind: sqldatabases.sql.cnrm.cloud.google.com (True: 45)
2024/11/14 18:51:33 - GroupKind: sqlusers.sql.cnrm.cloud.google.com (True: 45)
2024/11/14 18:51:33 Totals:
2024/11/14 18:51:33 - GroupKind: storagebuckets.storage.cnrm.cloud.google.com (True: 2200)
2024/11/14 18:51:33 - GroupKind: containerclusters.container.cnrm.cloud.google.com (False: 26, True: 23)
2024/11/14 18:51:33 - GroupKind: iampolicymembers.iam.cnrm.cloud.google.com (True: 69)
2024/11/14 18:51:33 - GroupKind: sqlusers.sql.cnrm.cloud.google.com (False: 80, True: 990)
2024/11/14 18:51:33 - GroupKind: iamserviceaccounts.iam.cnrm.cloud.google.com (True: 1123)
2024/11/14 18:51:33 - GroupKind: sqldatabases.sql.cnrm.cloud.google.com (False: 49, True: 1021)
2024/11/14 18:51:33 - GroupKind: iampartialpolicies.iam.cnrm.cloud.google.com (True: 3480, False: 58)
2024/11/14 18:51:33 - GroupKind: configconnectorcontexts.core.cnrm.cloud.google.com (no status: 50)
2024/11/14 18:51:33 - GroupKind: containernodepools.container.cnrm.cloud.google.com (True: 92, False: 104)
2024/11/14 18:51:33 - GroupKind: sqlinstances.sql.cnrm.cloud.google.com (False: 16, True: 198)
Fixes #
Tests you have done
make ready-pr
to ensure this PR is ready for review.