feat: Allow to collect only selected CF apps metadata through yaml; Support query depth parameter and spaceguid as filters #1026

Akash-Nayak · 2023-04-12T18:27:03Z

Currently move2kube collect fetches the metadata of all the apps that are running on Cloud Foundry instance. With this PR, we provide the user the flexibility to collect the metadata of a single app, or multiple apps, or all the apps that is/are running on Cloud Foundry.

To collect metadata of all the apps there is no change required while running move2kube collect.

$ move2kube collect -a cf

If the user wants to collect metadata of only specific application(s), then the user needs to create a simple yaml file and provide Move2Kube the path of the folder containing this yaml file while running the collect phase.

$ move2kube collect -a cf -s cfcollect

$ ls cfcollect
cf_target_apps.yaml

Here is a sample cf_target_apps.yaml file. Currently, it is optional to include guid, spaceguid, and organizationguid.

apiVersion: move2kube.konveyor.io/v1alpha1
kind: CfCollectApps
spec:
  applications:
    - name: cfnodejsapp
      guid: b48f67fb
      spaceguid: 74faba82
      organizationguid: 11387077-d81c
    - name: customers
      guid: 445b24f7
      spaceguid: 74faba82
      organizationguid: 11387077-d81c
    - name: inventory
      guid: 445b24f7
      spaceguid: 74faba82
      organizationguid: 11387077-d81c

Also, I have introduced depth constant that can be changed when required, to change the inline-relations-depth query parameter.

Signed-off-by: Akash Nayak akash19nayak@gmail.com

github-actions · 2023-04-12T18:27:19Z

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

codecov · 2023-04-12T18:39:45Z

Codecov Report

Patch coverage: 2.89% and project coverage change: -1.25 ⚠️

Comparison is base (f3b48c3) 15.90% compared to head (b77a7a3) 14.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
- Coverage   15.90%   14.66%   -1.25%     
==========================================
  Files          81       86       +5     
  Lines        7405     8035     +630     
==========================================
  Hits         1178     1178              
- Misses       5922     6552     +630     
  Partials      305      305

Impacted Files	Coverage Δ
collector/cfappscollector.go	`0.00% <0.00%> (ø)`
common/utils.go	`0.99% <0.00%> (-0.02%)`	⬇️
transformer/cloudfoundrytransformer.go	`0.00% <0.00%> (ø)`
collector/cfutils.go	`12.19% <35.71%> (ø)`

... and 4 files with indirect coverage changes

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

ashokponkumar

Can we allow for a hybrid approach:

By default we get all info, but provide a way to specify depth and get the level of info we need.
Also, we can allow users to filter based on space, org, etc..
if users specify a list of apps we get the info for them, which could be in the similar format as the output of above, so that users can edit the above output and use it as input.

ashokponkumar · 2023-04-13T07:21:12Z

collector/cfappscollector.go

+
+// CfCollectApp defines CfCollectApp information
+type CfCollectApp struct {
+	Name             string `yaml:"name"`


Can't we make name also optional. Say somebody wants to specify just the guids.

Yes, I have made name optional and the user can specify just the guids.

ashokponkumar · 2023-05-25T04:55:44Z

@seshapad can we review and merge this soon?

Akash-Nayak · 2023-05-25T05:07:14Z

Can we allow for a hybrid approach:

1. By default we get all info, but provide a way to specify depth and get the level of info we need.

2. Also, we can allow users to filter based on space, org, etc..

3. if users specify a list of apps we get the info for them, which could be in the similar format as the output of above, so that users can edit the above output and use it as input.

This is the new yaml format that we support now with move2kube collect.

apiVersion: move2kube.konveyor.io/v1alpha1
kind: CfCollectApps
spec:
  filters:
    query_depth: "0"
    spaceguid: 74faba82-c6db-40af-9179-4f4c288efdde
  applications:
    - application:
        # guid: b3ed607d-17df-4b74-a4b3-ddb88d7d1965
        name: inventory
    - application:
        # name: gateway
        guid: 445b24f7-0877-4447-ab01-e96af6741a91
    - application:
        name: cfnodejsapp
        guid: b48f67fb-f046-4700-bd65-e43170d802aa

This would collect the metadata of inventory, 445b24f7-0877-4447-ab01-e96af6741a91 (gateway), cfnodejsapp with query depth "0".

apiVersion: move2kube.konveyor.io/v1alpha1
kind: CfCollectApps
spec:
  filters:
    spaceguid: 74faba82-c6db-40af-9179-4f4c288efdde

The above yaml can be used to fetch all apps from the given spaceguid.

apiVersion: move2kube.konveyor.io/v1alpha1
kind: CfCollectApps
spec:
  filters:
    query_depth: "1"

With the above yaml, move2kube collect will collect info of all the apps for the given query_depth.

(1 & 2) It allows to filter based on query depth and spaceguid.
(3) move2kube collect output can now be fed as input to move2kube collect by changing the kind from CfApps to CfCollectApps. The query_depth/spaceguid filters can be added to the cfapps.yaml and apps can be added/removed from the yaml to only fetch the metadata of specific apps.

seshapad · 2023-05-28T14:10:29Z

collector/cfappscollector.go

+
+// CfCollectFilters stores the spaceguid and querydepth to be used to filter while collecting metadata
+type CfCollectFilters struct {
+	SpaceGuid  string `yaml:"spaceguid,omitempty"`


Why is spaceguid used in filter? What is the difference between guid and spaceguid?

A guid is for an app, and there can be multiple apps deployed to a CF space, and a CF org can contain multiple spaces which will have different spaceguids. So, we also want to support filtering apps based on spaceguid to narrow down the search results.

https://docs.cloudfoundry.org/concepts/roles.html

A space provides users with access to a shared location for app development, deployment, and maintenance. An org can contain multiple spaces. Every app, service, and route is scoped to a space.

why not also do the same for orgs?
https://cli.cloudfoundry.org/en-US/v6/org.html

We are not using orgs as filter because we cannot fetch the app data using q query parameter through /v2/organizations/, whereas /v2/spaces/:spaceguid supports fetching single/all app(s) data using query parameter (for ex: cf curl /v2/spaces/74faba82-c6db-40af-9179-4f4c288efdde/apps\?q=name%3Aorders).

seshapad · 2023-05-28T14:13:57Z

collector/cfappscollector.go

+func listAppsBySpaceGuid(client *cfclient.Client, spaceGuid string, queryDepth string, collectApps []App, numCfCollectApps int) (string, []App) {
+	query := setQueryDepth(queryDepth)
+	path := ""
+	if spaceGuid != "" {


Should we have shorter return from conditions? I.e. test for spaceGuid == ""?

Yes sure. Done.

seshapad · 2023-05-28T14:18:11Z

collector/cfappscollector.go

+func getAppByGuid(client *cfclient.Client, guid string, queryDepth string) (App, error) {
+	var appResource AppResource
+	query := setQueryDepth(queryDepth)
+	requestUrl := getRequestUrl("/v2/apps/"+guid, query) // /v2/apps/:appGuid fetches the app with given guid


Could you have used listAppPaths constant? If this is used only here, could you do away with constant?

I have updated the requestUrl to use the listAppsPaths constant. We are also using the listAppPaths in two other places too.

seshapad · 2023-05-28T14:24:51Z

collector/cfappscollector.go

+			path = listAppsPath
+		}
+		appSpec := cfCollectApp.AppSpec
+		if appSpec.Guid != "" {


Better to use shorter if returns. Please see previous comment on this.

I have update the code. First we check if app Guid is given, if the app data is collected successfully for the app Guid, then we continue the for loop for the next app. Otherwise, we check if app Name is given and try to collect the data of the app.

Also, I have made changes in the code to use shorter return in some other places too.

seshapad · 2023-05-28T14:28:54Z

collector/cfappscollector.go

+func setQueryDepth(queryDepth string) url.Values {
+	query := url.Values{}
+	if queryDepth != "" {
+		query.Set(inlineDepthRelations, queryDepth)


Would it not have been better to have a single if block to assign depth (initialized to default from global depth value) query_depth if not empty?

The set function could have used this resultant depth value.

Do you see a need for a if-else block with two different invocations of set function?

Yes sure. Done.

…meter as variable Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…th parameter as variable Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…ry depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…ort query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…nd support query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…adata and support query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

seshapad · 2023-05-30T07:45:58Z

collector/cfappscollector.go

+	return mergeAppResource(client, appResource), nil
+}
+
+func getAppsByNameOrGuid(client *cfclient.Client, path string, query url.Values, collectApps []App, appName string, appGuid string) []App {


what is the appName and appGuid used for? Is it only for error messages? Are these values already captured in the query? If so, why not remove them and print these values at the calling site of this function?

I have updated the function and also renamed it, and I have removed the appGuid which is not needed now in this function. The appName value is captured in the query but it is in form of a string like name:inventory (query- map[inline-relations-depth:[0] q:[name:inventory]]). The appName can be extracted using query.Get("q")[5:]. So, please suggest if I should pass the appName as parameter or should I use query.Get("q")[5:] inside the function?

@Akash-Nayak appName is only used for error logging right? if so, better not pass it as parameter. Also, dont extract it from the query either. You could always log the appName if getAppsByNameOrGuid() returns an error (append or wrap it with another error message with appName in it).

seshapad · 2023-05-30T07:58:40Z

collector/cfutils.go

+}
+
+// stringifyMap stringifies the map values
+func stringifyMap(inputMap map[string]interface{}) map[string]interface{} {


Two comments here:

This function is not just stringifying the map, isnt it? It is using the json format, right? If so, please name the function to reflect this function.

Should the function be moved to common utils package? @ashokponkumar what do you think?

If it a function that is reusable and not specific to this one flow, then move it to commons.utils. else it's fine to have it here.

Two comments here:

1. This function is not just stringifying the map, isnt it? It is using the json format, right? If so, please name the function to reflect this function.

I have renamed the function.

2. Should the function be moved to `common utils` package? @ashokponkumar what do you think?

I have moved the function to common utils package.

seshapad · 2023-05-30T08:03:42Z

collector/cfappscollector.go

+
+// CfCollectFilters stores the spaceguid and querydepth to be used to filter while collecting metadata
+type CfCollectFilters struct {
+	SpaceGuid  string `yaml:"spaceguid,omitempty"`


why not also do the same for orgs?
https://cli.cloudfoundry.org/en-US/v6/org.html

…pps metadata and support query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

github-actions bot added the feat label Apr 12, 2023

ashokponkumar reviewed Apr 13, 2023

View reviewed changes

Akash-Nayak force-pushed the cf-collect-per-app branch 2 times, most recently from 85029df to 980c202 Compare May 25, 2023 04:50

Akash-Nayak changed the title ~~feat: Allow to collect only selected CF apps metadata; Use depth parameter as variable~~ feat: Allow to collect only selected CF apps metadata through yaml; Support query depth parameter and spaceguid as filters May 25, 2023

Akash-Nayak mentioned this pull request May 25, 2023

feat: Optimize move2kube collect performance using ListAppsByQuery with depth 0 #1018

Closed

seshapad reviewed May 29, 2023

View reviewed changes

Akash-Nayak added 5 commits May 30, 2023 11:08

feat: Allow to collect only selected CF apps metadata; Use depth para…

3ac2695

…meter as variable Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fixup! feat: Allow to collect only selected CF apps metadata; Use dep…

a1707b8

…th parameter as variable Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

feat: Allow to collect only selected CF apps metadata and support que…

10d3f74

…ry depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fixup! feat: Allow to collect only selected CF apps metadata and supp…

14d6d8f

…ort query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fixup! fixup! feat: Allow to collect only selected CF apps metadata a…

8d09039

…nd support query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

Akash-Nayak force-pushed the cf-collect-per-app branch from 77f11bd to 8d09039 Compare May 30, 2023 05:43

fixup! fixup! fixup! feat: Allow to collect only selected CF apps met…

a3b497c

…adata and support query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

Akash-Nayak requested a review from seshapad May 30, 2023 07:16

seshapad reviewed May 30, 2023

View reviewed changes

Akash-Nayak added 2 commits May 30, 2023 18:14

fixup! fixup! fixup! fixup! feat: Allow to collect only selected CF a…

06b8854

…pps metadata and support query depth and space guid filters Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fix: Update the getAppsByName function

b77a7a3

Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

Akash-Nayak requested a review from seshapad May 31, 2023 03:21

seshapad approved these changes May 31, 2023

View reviewed changes

seshapad merged commit 5113d4c into konveyor:main May 31, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Allow to collect only selected CF apps metadata through yaml; Support query depth parameter and spaceguid as filters #1026

feat: Allow to collect only selected CF apps metadata through yaml; Support query depth parameter and spaceguid as filters #1026

Akash-Nayak commented Apr 12, 2023

github-actions bot commented Apr 12, 2023

codecov bot commented Apr 12, 2023 •

edited

Loading

ashokponkumar left a comment •

edited

Loading

ashokponkumar Apr 13, 2023

Akash-Nayak May 25, 2023

ashokponkumar commented May 25, 2023

Akash-Nayak commented May 25, 2023 •

edited

Loading

seshapad May 28, 2023

Akash-Nayak May 30, 2023

seshapad May 30, 2023

Akash-Nayak May 30, 2023

seshapad May 28, 2023

Akash-Nayak May 30, 2023

seshapad May 28, 2023

Akash-Nayak May 30, 2023

seshapad May 28, 2023

Akash-Nayak May 30, 2023

seshapad May 28, 2023

Akash-Nayak May 30, 2023

seshapad May 30, 2023

Akash-Nayak May 30, 2023 •

edited

Loading

seshapad May 31, 2023

seshapad May 30, 2023

ashokponkumar May 30, 2023

Akash-Nayak May 30, 2023

seshapad May 30, 2023

feat: Allow to collect only selected CF apps metadata through yaml; Support query depth parameter and spaceguid as filters #1026

feat: Allow to collect only selected CF apps metadata through yaml; Support query depth parameter and spaceguid as filters #1026

Conversation

Akash-Nayak commented Apr 12, 2023

github-actions bot commented Apr 12, 2023

codecov bot commented Apr 12, 2023 • edited Loading

Codecov Report

ashokponkumar left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashokponkumar commented May 25, 2023

Akash-Nayak commented May 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Akash-Nayak May 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 12, 2023 •

edited

Loading

ashokponkumar left a comment •

edited

Loading

Akash-Nayak commented May 25, 2023 •

edited

Loading

Akash-Nayak May 30, 2023 •

edited

Loading