Revised Proposal to param based operator service creation

[girishramnani]

What type of PR is this?
/kind design
/kind documentation
[skip ci]

What does this PR do / why we need it:
see $SUBJECT

Which issue(s) this PR fixes:

Fixes #4240

PR acceptance criteria:

• Unit test

• Integration test

• Documentation

• Update changelog

• I have read the test guidelines

How to test changes / Special notes to the reviewer:

[girishramnani]

@kadel @dharmit The swagger.json that is present on the cluster is currently 15MBs when I tried. What would be the best approach to cache it?
And we need to be sure that we update the swagger when a new operator is installed on the cluster.

So I was thinking of fetching it parallely when a user does odo catalog list services? This would give us a headstart?

Once we have the swagger.json, we can either store it whole, or specifically store all the CRDs that are listed in odo catalog list services?

I was thinking of storing the operator listing ( just the operator part of odo catalog list services) in the cache as well so when the user runs the command again we can diff between the old and new operator listing to confirm if a new one was installed? and if it was then we fetch the swagger.json again from the cluster?

[dharmit]

What's the "plain vanilla" in this case? Is it normal user on vanilla k8s or non-admin user on any of k8s/ocp clusters?

[girishramnani]

Both actually, a normal user or non admin user. From the metadata perspective both are same

[dharmit]

In what scenario(s) is the CRD accessible to non-admin user? My understanding is that kubectl get crds fails when done as a non-admin user.

[girishramnani]

If the user has permissions to that endpoint but is not admin user then they can use it

[dharmit]

OK. Technically feasible. But doesn't seem like a scenario we would have too often. Again, I can't back up my comment with Telemetry. It's more of a hunch.

[girishramnani]

But we should still account for such event as web console uses that same approach so they would have a reason to do this

[dharmit]

We're already listing all the CRDs provided by all the installed Operators. I think this won't change at all.

[girishramnani]

ack, changing that

[girishramnani]

changed

[dharmit]

@girishramnani @kadel I'm probably missing something simple here. Where in the swagger AP (http://<cluster-url>/openapi/v2) that we're discussing here is the information about parameters available?

I'm referring to the JSON available at this endpoint and I can't find the parameters that we see in the alm-example. I'm currently trying to find parameters for etcdcluster.

Besides that, in the JSON, is there one place where I can find all the Operators? I ask this because information for etcd Operator and service binding Operator are available at two different endpoints:

• /apis/etcd.database.coreos.com/v1beta2/etcdclusters
• /apis/binding.operators.coreos.com/v1alpha1/servicebindings

Am I looking at the right place?

[girishramnani]

@girishramnani @kadel I'm probably missing something simple here. Where in the swagger AP (http://<cluster-url>/openapi/v2) that we're discussing here is the information about parameters available?

I'm referring to the JSON available at this endpoint and I can't find the parameters that we see in the alm-example. I'm currently trying to find parameters for etcdcluster.

Besides that, in the JSON, is there one place where I can find all the Operators? I ask this because information for etcd Operator and service binding Operator are available at two different endpoints:

• /apis/etcd.database.coreos.com/v1beta2/etcdclusters
• /apis/binding.operators.coreos.com/v1alpha1/servicebindings

Am I looking at the right place?

The json that I would have pasted wouldn’t have etcd as I didn’t have that installed at the time.
Also that gist is half, will paste a full file soon

[dharmit]

The json that I would have pasted wouldn’t have etcd as I didn’t have that installed at the time.
Also that gist is half, will paste a full file soon

I was referring to the Swagger data I got from my cluster which had etcd Opreator installed.

[girishramnani]

The json that I would have pasted wouldn’t have etcd as I didn’t have that installed at the time.
Also that gist is half, will paste a full file soon

I was referring to the Swagger data I got from my cluster which had etcd Opreator installed.

Could you please share that file?

[kadel]

how are arrays of objects going to be represented in this output?

lets say that there is that first.second.envVars is and array of env variable objects ({name: "foo", value:"bar"})
How the FieldPath will look like, so users know that envVars is an array of objects and the format they need to use is first.second.envVars[0].name=foo first.second.envVars[0].value=bar)
and they don't try to do something like first.second.envVars.name=foo first.second.envVars.value=bar or first.second[0].envVars.name=foo first.second[0].envVars.value=bar)

[girishramnani]

my thought was to Maybe we can show it as first.second.envVars[*].name and explain in describe that * needs to be an index 0,1….?

[kadel]

my thought was to Maybe we can show it as first.second.envVars[*].name and explain in describe that * needs to be an index 0,1….?

👍 If there is enough information to do that that would be great!

[girishramnani]

yeah we have information on what is an array from swagger

[girishramnani]

added conversion detail in the proposal

[girishramnani]

So I worked on going through the swagger and realised some CRs dont have any information.

      "com.redhat.quay.v1.QuayRegistry": {
        "type": "object",
        "x-kubernetes-group-version-kind": [
          {
            "group": "quay.redhat.com",
            "kind": "QuayRegistry",
            "version": "v1"
          }
        ]
      }

in such cases we will as usual fallback to x-descriptor

[kadel]

There is still a lot of details that need to be figured out. But those are going to be easier to figure out once we start implementing this.
From a high level, this looks good.
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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:

• docs/OWNERS [kadel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dharmit]

So I worked on going through the swagger and realised some CRs dont have any information.

      "com.redhat.quay.v1.QuayRegistry": {
        "type": "object",
        "x-kubernetes-group-version-kind": [
          {
            "group": "quay.redhat.com",
            "kind": "QuayRegistry",
            "version": "v1"
          }
        ]
      }

in such cases we will as usual fallback to x-descriptor

This particular one is my main concern/worry. But we got to start implementing things to realize what's working and what's not.

/LGTM

[feloy]

I'm not very comfortable with managing an automatic cache (which, without a serious protocol between the two parts, is doomed to be inefficient).

What about a specific and explicit odo command, that the user could run to fetch the param information for one or more services?

User story:

1. odo catalog list services indicates the params or number of params if known, or if not known, that the user can fetch them.
2. fetch the params for a specific service

$ odo catalog list services
Services available through Operators
NAME                                             CRDs                                                          Params
etcdoperator.v0.9.4-clusterwide     EtcdCluster, EtcdBackup, EtcdRestore       (to fetch)
service-binding-operator.v0.7.1     ServiceBinding                                             3 parameters
To fetch params for a service, you can run `odo service fetch-params <service-name>


$ odo service fetch-params etcdoperator.v0.9.4-clusterwide/EtcdCluster
...

Also, only the params information could be stored locally (and not the complete swagger/CRD info), independently of the source of the params (ClusterServiceVersion, swagger, CRD), so the use of this data by odo service create is generic.

The user could detect which method for fetching the data is available, or give the user the choice to select one.

[kadel]

fetch the params for a specific service

I'm not sure about this. As a user I would be pretty annoyed that I have to run extra command to get information that is essentially required for creating services.
Why odo catalog list services couldn't do that automatically?

Also, only the params information could be stored locally (and not the complete swagger/CRD info), independently of the source of the params (ClusterServiceVersion, swagger, CRD), so the use of this data by odo service create is generic.

I like the idea of caching just params for all sources. This is good.

The user could detect which method for fetching the data is available, or give the user the choice to select one.

I think that this is exposing implementation to users. The user doesn't care where the data comes from. Especially when it can be a fairly complicated decision.