Odo: Improve performance for `odo catalog list components` .

Created on 27 May 2020  路  21Comments  路  Source: openshift/odo

/kind feature

currently we are parsing all devfiles from all registries during odo catalog list components, We should list devfiles from index.json

aredevfile kinuser-story triagneeds-information

Most helpful comment

@kadel @elsony what display name should we use for odo? if we start listing from index.json
https://github.com/elsony/devfile-registry/blob/master/devfiles/index.json#L3
long names would not fit for odo create.

We will have to add the devfile name into the index.json for now.

Currently, there is no definition of how the proper devfile v2 registry interface should look like.
The current index.json is just a temporary solution.

All 21 comments

for V1 we are parsing devfile from each registry to this struct https://github.com/openshift/odo/blob/master/pkg/catalog/types.go#L45
and validating for supported component here
https://github.com/openshift/odo/blob/master/pkg/catalog/types.go#L45

IIUC We are parsing and validating each devfile from each registry before listing, We are parsing the devfile for two things

  • Name
  • Supported

is parsing each devfile really required?

We can get name from index.json of a devfile registry.

We can decide supported on the basis of devfile registry
odo specific registry : Ideally if a devfile has been added to an odo specific registry it is supported, no need to validate with each call of odo catalog list components. The devfile should be validated and tested before adding it to the registry. So for odo specific registries(registry by odo team) we could default them to true.

other registries : other registries could be added by user, or some other open registries.
devfiles in those are not tested by odo, so we should not list them as supported and hence they can be defaulted to false

cc @kadel @elsony

is parsing each devfile really required?

We should avoid parsing each devfile from the registry. It is not a good long term solution and it will definitely create problems for even moderately large devfile registries.

The odo specific registry is a point-in-time thing. Our eventual goal is share the same devfile registry as Che (and other dev tools). In that case, we don't have a registry that we know for sure we support all devfiles in there. That leave us a couple of options:

  1. Continue to scan for all devfiles in a repo: as @kadel mentioned, it may causes performance problem on large registry.
  2. Not scanning any at all and just assume all devfiles are supported. We just error out during the execution if someone tries to run do odo push on some devfiles that we cannot support, e.g. without any run command: I don't think this will provide a good user experience since it discover quite late on the cycle.
  3. Inject some info into the index.json, e.g. an executable or dev tag, that indicates those devfiles contain the minimum that we can support. We basically move the scanning from real time on all devfiles to one time on building the index.json. Once we have that, we can easily filter out just based on info within the index.json.

I think #3 is a good trade off to inject a build step on the repo for which we already have scripts to generate the index.json in our repo.

聽Not scanning any at all and just assume all devfiles are supported. We just error out during the execution if someone tries to run do odo push on some devfiles that we cannot support, e.g. without any run command: I don't think this will provide a good user experience since it discover quite late on the cycle.

I'm not a big fan of filtering supported devfiles in registries. It will fragment the devfile ecosystem.
Eventually, odo should support the devfiles that don't have run or build command.
Such devfiles are still valid and they can be useful. They won't have much use for innerloop, but I could have a devfile that deploys some service (it uses just kubernetes or openshift component types).

@kadel @elsony what display name should we use for odo? if we start listing from index.json
https://github.com/elsony/devfile-registry/blob/master/devfiles/index.json#L3
long names would not fit for odo create.

@kadel @elsony what display name should we use for odo? if we start listing from index.json
https://github.com/elsony/devfile-registry/blob/master/devfiles/index.json#L3
long names would not fit for odo create.

We will have to add the devfile name into the index.json for now.

Currently, there is no definition of how the proper devfile v2 registry interface should look like.
The current index.json is just a temporary solution.

@adisky I've forked @elsony repository to https://github.com/odo-devfiles/registry and updated index.json to include name field for each devfile.

@elsony I've removed the version hierarchy that you had there. Once we have a good idea of how we will handle versions and we have support for it on odo side we can update the hierarchy again to include versions.

There is https://github.com/openshift/odo/issues/3298 which is related to this

This issue is already covered by #2938 and implemented by PR #https://github.com/openshift/odo/pull/3291

so this is a dup issue, we need to identify who is working on this to avoid duplicated efforts

cc @kadel @elsony

@kadel @elsony @adisky so did we reach to any conclusion on this topic? Scanning the other discussion, it looks like we want to stop parsing individual devfiles for names and to check if they're supported..

index.json can have the name, but for the matter of checking if they're supported, have we reached an agreement? PR #3291 currently already does parsing for v2 like v1 at this point

I think one way is to move the logic for checking if devfile is supported, from the odo repo to https://github.com/odo-devfiles/registry repo and store that information in index.yaml.

For example, here is a branch i worked on which showcases the above discussion https://github.com/odo-devfiles/registry/compare/master...maysunfaisal:update-registry-index-support-1?expand=1

Then we can just read index.json for component name and to check if the component is supported. Of course, we should also consider what to do if index.json does not have the value of its supported. Should we default to false in odo catalog list components?

@maysunfaisal I think we are agreed on not parsing the devfile and using name from index.json,
for supported i am also not sure, If you have implemented odo catalog list components for devfile V2 upto V1 level, should i rename this issue to Do not parse each devfile from registry? if you wish this improvement can be handled separately.

And also as said above index.json is temporary solution, I think it is fine to have supported: true in index.json for now.

Can I get some reviewers on the v1 and v2 registry PRs for the discussion above?
v1: https://github.com/elsony/devfile-registry/pull/16
v2: https://github.com/odo-devfiles/registry/pull/3

this is implemented in odo WIP PR https://github.com/openshift/odo/pull/3291

cc @adisky @kadel

EDIT - looks like we need more discussion on the above topic since Che registry's index.json does not have the name field or supported field. Supported defaults to false in Che's case which is fine but the absence of the name field is a problem https://che-devfile-registry.openshift.io/devfiles/index.json

Added a topic to odo's contributors call to discuss this.

@maysunfaisal is it possible to extract out name from displayName e.g
displayName; NodeJS React Web Application name would be nodejs-react

I know it might be weird for some cases and not a viable solution, just shared the thought.

We had a discussion on this topic in the odo contributors call:

  • We are going to show all the stacks from a registry ie; no more supported column (in the future if we determine, Che devfiles need to filtered; we can reintroduce a supported field)
  • Remove Che and odo v1 registry from default selection on odo catalog list components

Also opened an issue to request Che v2 registry to have a name field in index.json https://github.com/devfile/kubernetes-api/issues/78

  1. Do we want to remove the SUPPORTED column for the s2i components?

This is how it looks between s2i and devfiles(notice that I removed the devfile SUPPORTED column).

  1. Do we want to remove the -a flag since it was only used to filter the Che v1 devfiles (and had no difference between filtering supported and unsupported)? And since we're removing the default Che v1 devfile registry; the -a flag does nothing.
$ odo catalog list components (odo catalog list components -a shows the same output) 
Odo OpenShift Components:
NAME              PROJECT       TAGS                        SUPPORTED
java              openshift     11,8,latest                 YES
nodejs            openshift     10-SCL,8,8-RHOAR,latest     YES
dotnet            openshift     2.1,2.2,3.0,latest          NO
golang            openshift     1.11.5,latest               NO
httpd             
Odo Devfile Components:
NAME            DESCRIPTION                            REGISTRY
maven           Upstream Maven and OpenJDK 11          DefaultDevfileRegistry
nodejs          Stack with NodeJS 10                   DefaultDevfileRegistry
openLiberty     Open Liberty microservice in Java      DefaultDevfileRegistry
quarkus         Upstream Quarkus with Java+GraalVM     DefaultDevfileRegistry
springBoot      Spring Boot庐 using Java                DefaultDevfileRegistry

Copying what I mentioned on slack:

  1. Do we want to remove the SUPPORTED column for the s2i components?

Keep it for s2i components as it's still needed / useful

  1. Do we want to remove the -a flag since it was only used to filter the Che v1 devfiles (and had no difference between filtering supported and unsupported)? And since we're removing the default Che v1 devfile registry; the -a flag does nothing.

Since -a is only for the Che devfiles really, we can take it out.

@adisky there was a related discussion in https://github.com/openshift/odo/issues/3249#issuecomment-641350713

/close

@adisky: Closing this issue.

In response to this:

/close

Instructions 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/test-infra repository.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

valaparthvi picture valaparthvi  路  3Comments

kadel picture kadel  路  4Comments

maysunfaisal picture maysunfaisal  路  8Comments

prietyc123 picture prietyc123  路  7Comments

mik-dass picture mik-dass  路  7Comments