odo 🚀 - odo service create should add operator CR to the devfile

It seems the Uri holds a link to the yaml or we can provide the yaml as a string in Inlined. both seem mutually exclusive.

I'm not sure what purpose does having both Inlined and Uri serve. When I read this, I think of Inlined as a string representation of the CRD used to spin up a CR. It would be the same as what we have right now when doing odo service create <operator-name>/<cr-name> --dry-run. Then what is Uri/reference in this case?

What is odo supposed to do with name? At the moment, we have odo service create etcdoperator.v0.9.4/EtcdCluster newetcd which creates a new service from the CR EtcdCluster provided by the Operator etcdoperator.v0.9.4 and names it as newetcd. Should odo use the value in name for the newly created service?

devfile/api#36

This is a proposal, IIUC. Do we have to implement things based on this proposal? Or do we use Kubernetes/OpenShift lingo instead? If we should be using Kubernetes/OpenShift based on current devfile schema, what happens when this change goes in? We make changes to work with the new terminology?

Few other questions (more along the lines of "thinking out loud"):

What's the timeline for this? It seems more like an epic than a user story to me.
What about Service Catalog backed services? Are we planning to support those in the new approach?
What about the combination of "s2i components" + "Service Catalog services"? Are we planning to support that using existing tooling using if..else that we had for experimental mode?
How does this play with devfile parser that's being moved out to a separate project? What parts are supposed to be handled by devfile parser and what is to be handled by odo?

dharmit on 28 Oct 2020

Then what is Uri/reference in this case?

I think inline and Uri/reference are mutually exclusive. i.e. if you provide inline then you then the uri would be ignored.

What's the timeline for this? It seems more like an epic than a user story to me.

well I was hoping for this to be done in this sprint? only the devfile + operator backed service and that too just adding the CR into the devfile and still do that original work so we dont break operator backed services. Essentially
do what odo service create does and also store the CR in the devfile for now.

What about Service Catalog backed services? Are we planning to support those in the new approach?

Yeah that is something I was thinking alot about and right now I would say no.

Some more thinking on what kind of combination we are looking at

[ ] devfile component + operator backed service - scoped in this issue

[ ] devfile component + service catalog services - we could store the service template in the devfile directly
[ ] s2i component + operator backed service - we dont have a devfile for s2i component so we might need to add the CR in the config.yaml. So imagine an interface ServiceStorageProvider that can be implemented by a config.yaml and the devfile?
[ ] s2i component + service catalog service - again store the service template in the config.yaml ( many other things are possible so think about this point )

I bet there is a newer service catalog CR someone would have implemented to move the service catalog into an operator but I haven't searched about it.

girishramnani on 2 Nov 2020

Sorry about writing an essay. I have a lot of questions around this and I couldn't find a better way to put it.

I think inline and Uri/reference are mutually exclusive. i.e. if you provide inline then you then the uri would be ignored.

OK. And what would be a valid value for URI in this case? What would we like to store as URI?

About storing the CRD in devfile. Currently, we don't support passing parameters for odo service create when the user wants to spin up an Operator backed service. I think, storing CRD in devfile would be a lot more valuable when we have this support.

Then there's also this notion of keeping the information in devfile compatible with what Che might need.

well I was hoping for this to be done in this sprint? only the devfile + operator backed service and that too just adding the CR into the devfile and still do that original work so we dont break operator backed services. Essentially do what odo service create does and also store the CR in the devfile for now.

If we put the CRD (the definition used to bring up the CR, a.k.a. service, provided by an Operator) in the devfile when doing odo service create, and if odo push fails to bring up the service for some reason, what do we do about the info that's gone into devfile? Should we delete it. What if this happens when user2 is cloning the code provided by user1 and odo push fails to spin up the service because the required Operator doesn't exist on their environment? Deleting the CRD from devfile might be a bad idea in that case. Also, does thinking of failure scenarios from odo watch's perspective add any other challenges?

Also, if I understand correctly, you mean to only add the CRD into devfile as Step 1 and not changing the flow to create the service when doing odo push. Sounds doable.

Yeah that is something I was thinking alot about and right now I would say no.

OK. So what would be our approach to handle it? Separate code (as is the case right now)? Do we want to provide different UX to the users when they spin up a service using Operators vs. Service Catalog? By different approach, I mean that if we store CRD info in devfile, we eventually want to provide the ability to create a component+service in one odo push when, say, user2 clones the repo on their system. But this wouldn't hold true for Service Catalog based services.

From k8s point of view, I don't think we will/should be deprecating support for Service Catalog in a rush. OTOH, we could also deprecate it since it's no longer available on OCP and focus solely on Operators. This reduces the scope of bugs in odo code. IMO, we should think this hard because supporting both requires considering a lot of permutations and combinations.

This piece is confusing and I'd prefer we think of reaching a conclusion sooner rather than later.

[ ] devfile component + operator backed service - scoped in this issue

This means storing the CRD in the devfile. It doesn't mean actually spinning up a service when doing odo push after odo service create stores the info in devfile. Should we add a few unambiguous acceptance criteria to this issue?

[ ] devfile component + service catalog services - we could store the service template in the devfile directly

At the moment, this scenario is not supported. I think I need to come up with a matrix about all the possibilities and their support status. Otherwise, this is quickly going to become insane.

[ ] s2i component + operator backed service - we dont have a devfile for s2i component so we might need to add the CR in the config.yaml. So imagine an interface ServiceStorageProvider that can be implemented by a config.yaml and the devfile?

[ ] s2i component + service catalog service - again store the service template in the config.yaml ( many other things are possible so think about this point )

We discussed under the hood conversion of s2i components to devfile on the Cabal. So I think this needs a rethink?

I bet there is a newer service catalog CR someone would have implemented to move the service catalog into an operator but I haven't searched about it.

I'm not so sure about the CR/Operator for Service Catalog. But I think we can give a try to the steps mentioned here on a minikube setup. But again, I'd prefer we discuss how long we intend to support it. And I am seeking this info only because OCP doesn't support it anymore.

dharmit on 4 Nov 2020

Sorry about writing an essay. I have a lot of questions around this and I couldn't find a better way to put it.

I think inline and Uri/reference are mutually exclusive. i.e. if you provide inline then you then the uri would be ignored.

OK. And what would be a valid value for URI in this case? What would we like to store as URI?

_It would be a public link to a kubernetes manifest and when there is something inline then the uri would be blank._
reference: https://.../mongo.yaml

About storing the CRD in devfile. Currently, we don't support passing parameters for odo service create when the user wants to spin up an Operator backed service. I think, storing CRD in devfile would be a lot more valuable when we have this support.

_Agreed but for now lets not add the functionality of passing parameters - first step is simple and that is store the CRD in the devfile then maybe we can think about this_

Then there's also this notion of keeping the information in devfile compatible with what Che might need.

well I was hoping for this to be done in this sprint? only the devfile + operator backed service and that too just adding the CR into the devfile and still do that original work so we dont break operator backed services. Essentially do what odo service create does and also store the CR in the devfile for now.

If we put the CRD (the definition used to bring up the CR, a.k.a. service, provided by an Operator) in the devfile when doing odo service create, and if odo push fails to bring up the service for some reason, what do we do about the info that's gone into devfile? Should we delete it. What if this happens when user2 is cloning the code provided by user1 and odo push fails to spin up the service because the required Operator doesn't exist on their environment? Deleting the CRD from devfile might be a bad idea in that case. Also, does thinking of failure scenarios from odo watch's perspective add any other challenges?

_I asked the same question to tomas and he mentioned that the person who added the service operator in the devfile knows that their application wont function correctly without it so the person cloning the application should have the operator present and if not - its a good thing that push fails. We need to provide good error messages here though_

Also, if I understand correctly, you mean to only add the CRD into devfile as Step 1 and not changing the flow to create the service when doing odo push. Sounds doable.

_Yes as this flow of storing the CRD in the devfile is still a smaller part of the whole puzzle we need to keep the original work intact until the new flow is completely implemented_

Yeah that is something I was thinking alot about and right now I would say no.

OK. So what would be our approach to handle it? Separate code (as is the case right now)? Do we want to provide different UX to the users when they spin up a service using Operators vs. Service Catalog? By different approach, I mean that if we store CRD info in devfile, we eventually want to provide the ability to create a component+service in one odo push when, say, user2 clones the repo on their system. But this wouldn't hold true for Service Catalog based services.

From k8s point of view, I don't think we will/should be deprecating support for Service Catalog in a rush. OTOH, we could also deprecate it since it's no longer available on OCP and focus solely on Operators. This reduces the scope of bugs in odo code. IMO, we should think this hard because supporting both requires considering a lot of permutations and combinations.

This piece is confusing and I'd prefer we think of reaching a conclusion sooner rather than later.

[ ] devfile component + operator backed service - scoped in this issue

This means storing the CRD in the devfile. It doesn't mean actually spinning up a service when doing odo push after odo service create stores the info in devfile. Should we add a few unambiguous acceptance criteria to this issue?

[ ] devfile component + service catalog services - we could store the service template in the devfile directly

At the moment, this scenario is not supported. I think I need to come up with a matrix about all the possibilities and their support status. Otherwise, this is quickly going to become insane.

[ ] s2i component + operator backed service - we dont have a devfile for s2i component so we might need to add the CR in the config.yaml. So imagine an interface ServiceStorageProvider that can be implemented by a config.yaml and the devfile?

[ ] s2i component + service catalog service - again store the service template in the config.yaml ( many other things are possible so think about this point )

We discussed under the hood conversion of s2i components to devfile on the Cabal. So I think this needs a rethink?

I bet there is a newer service catalog CR someone would have implemented to move the service catalog into an operator but I haven't searched about it.

I'm not so sure about the CR/Operator for Service Catalog. But I think we can give a try to the steps mentioned here on a minikube setup. But again, I'd prefer we discuss how long we intend to support it. And I am seeking this info only because OCP doesn't support it anymore.

girishramnani on 5 Nov 2020

OK. And what would be a valid value for URI in this case? What would we like to store as URI?

_It would be a public link to a kubernetes manifest and when there is something inline then the uri would be blank._
reference: https://.../mongo.yaml

:+1: makes sense.

About storing the CRD in devfile. Currently, we don't support passing parameters for odo service create when the user wants to spin up an Operator backed service. I think, storing CRD in devfile would be a lot more valuable when we have this support.

_Agreed but for now lets not add the functionality of passing parameters - first step is simple and that is store the CRD in the devfile then maybe we can think about this_

Ah, I didn't mean to stall this till we have parameters support. Totally agree about adding CRD info into devfile as a first step.

If we put the CRD (the definition used to bring up the CR, a.k.a. service, provided by an Operator) in the devfile when doing odo service create, and if odo push fails to bring up the service for some reason, what do we do about the info that's gone into devfile? Should we delete it. What if this happens when user2 is cloning the code provided by user1 and odo push fails to spin up the service because the required Operator doesn't exist on their environment? Deleting the CRD from devfile might be a bad idea in that case. Also, does thinking of failure scenarios from odo watch's perspective add any other challenges?

_I asked the same question to tomas and he mentioned that the person who added the service operator in the devfile knows that their application wont function correctly without it so the person cloning the application should have the operator present and if not - its a good thing that push fails. We need to provide good error messages here though_

OK. So if the push fails for user2, what do we want the state of the component to be? Only the service creation on the cluster fails for them or does the entire push become invalid? I get it that this is not a part of the first step we just agreed on above, but this is going to be the next step, IIUC.

Other questions remain unanswered. Can we discuss this as well because there are a lot of possible scenarios that we need to think of? cc @kadel

In the meantime, I'll start looking into adding just the CRD to the devfile.

dharmit on 6 Nov 2020

The first thing that needs to be addressed is https://github.com/openshift/odo/issues/4159
Otherwise, services stored in Devfile can't be created.

Once we have that odo service create should be quite simple.
Instead of creating Kubernetes resources in cluster it will save it into devfile as inlined kubernetes component

For example command odo service create mongodb-enterprise.v1.8.0/MongoDB mymongo will add a new component into Devfile that looks like this:

components:
  - name: mymongo
    kubernetes:
      inlined: >
        apiVersion: mongodb.com/v1
        kind: MongoDB
        metadata:
          name: mymongo
        spec:
          credentials: my-credentials
          members: 3
          opsManager:
            configMapRef:
              name: my-project
          persistent: true
          type: ReplicaSet
          version: 4.4.0-ent
        status:
          phase: Running
          type: ReplicaSet

odo service create etcdoperator.v0.9.4-clusterwide/EtcdCluster myetcd --dry-run > myetcd.yaml
odo service create --from-file myetcd.yaml

Will save the content of myetcd.yaml as a new kubernetes component in Devfile

components:
  - name: myetcd
    kubernetes:
      inlined: >
        apiVersion: etcd.database.coreos.com/v1beta2
        kind: EtcdCluster
        metadata:
          annotations:
            etcd.database.coreos.com/scope: clusterwide
          name: myetcd
        spec:
          size: 3
          version: 3.2.13

Similarly with service catalog
odo service create mariadb --plan 10-3-22 --parameters param-1=value-1 --parameters param-2=value-2

should result in adding the following kubernetes component into the devfile

components:
  - name: mymaria
    kubernetes:
      inlined: >
        apiVersion: servicecatalog.k8s.io/v1beta1
        kind: ServiceInstance
        metadata:
          name: mymaria
          namespace: tkral-test
        spec:
          clusterServiceClassExternalName: mariadb
          clusterServicePlanExternalName: 10-3-22
          parameters:
            param-1: value-1
            param-2: value-2

But again, before we tackle this we need to make sure that odo understands kubernetes components
https://github.com/openshift/odo/issues/4159. This needs to be the first step.

kadel on 6 Nov 2020

odo service create etcdoperator.v0.9.4-clusterwide/EtcdCluster myetcd --dry-run > myetcd.yaml
odo service create --from-file myetcd.yaml

We can get rid of the --dry-run and --from-file entirely because odo service create will store the CR in the devfile so a user can open the devfile and edit it before doing a odo push if they want to. And --from-file makes no sense either once we start storing the CRD in devfile.

But how/where do we want to store link related information?

dharmit on 10 Nov 2020

But how/where do we want to store link related information?

We need to create a separate issue for this. (just created https://github.com/openshift/odo/issues/4208)

The link will come as next after we have https://github.com/openshift/odo/issues/4159 and this one.

It will work similarly as odo service create. odo link will store ServiceBinding CR in devfile as a kubernetes component. And then it will be created later by running odo push.

kadel on 10 Nov 2020

This will not be possible to do before #4159 which is in turn dependent on https://github.com/devfile/api/issues/216.

dharmit on 4 Dec 2020

Scope of this issue for current iteration

Focussing on second acceptance criteria - odo service create should store service definition in devfile as kubernetes component (ServiceInstance for SericeCatalog or CR for operator backed service)

Store alm example of a CR into devfile. It should store this into devfile for both of the below scenarios:
- odo service create <operator-name>/<cr-name>
- odo service create --from-file
Store ServiceInstance info into devfile (still not sure about how to fetch this.) (Refer: https://github.com/openshift/odo/issues/4160#issuecomment-787601310)
odo service delete should delete the entry from devfile for both Operators & Service Catalog. (Refer: https://github.com/openshift/odo/issues/4160#issuecomment-787601594)
Handled by @mik-dass in https://github.com/openshift/odo/pull/4446. ~Involves updating dependency to devfile v2. (#4346)~
- ~Function signature has changed for GetComponents which is used at various places in odo code. Will have to update related unit tests as well.~
Add unit tests and integration tests.

dharmit on 9 Feb 2021

what about odo service create --from-file? CR defined in the file should also be saved in Devfile

kadel on 10 Feb 2021

Store ServiceInstance info into devfile (still not sure about how to fetch this.)

Managed to figure this out. For a devfile based component, when the user tries to create a Service Catalog backed service, the info gets added to devfile.yaml:

$ odo service create dh-postgresql-apb psql --plan dev -p postgresql_user=luke -p postgresql_password=secret -p postgresql_database=my_data -p postgresql_version=9.6
Deploying service "psql" of type: "dh-postgresql-apb"
 ✓  Service "psql" was created

Progress of the provisioning will not be reported and might take a long time
You can see the current status by executing 'odo service list'
Optionally, link "dh-postgresql-apb" to your component by running: 'odo link <component-name>'

$ cat devfile.yaml
commands:
- exec:
    commandLine: npm install
    component: runtime
    group:
      isDefault: true
      kind: build
    workingDir: /project
  id: install
- exec:
    commandLine: npm start
    component: runtime
    group:
      isDefault: true
      kind: run
    workingDir: /project
  id: run
- exec:
    commandLine: npm run debug
    component: runtime
    group:
      isDefault: true
      kind: debug
    workingDir: /project
  id: debug
- exec:
    commandLine: npm test
    component: runtime
    group:
      isDefault: true
      kind: test
    workingDir: /project
  id: test
components:
- container:
    endpoints:
    - name: http-3000
      targetPort: 3000
    image: registry.access.redhat.com/ubi8/nodejs-14:latest
    memoryLimit: 1024Mi
    mountSources: true
    sourceMapping: /project
  name: runtime
- kubernetes:
    inlined: |
      apiVersion: servicecatalog.k8s.io/v1beta1
      kind: ServiceInstance
      metadata:
        creationTimestamp: null
        labels:
          app: app
          app.kubernetes.io/instance: psql
          app.kubernetes.io/managed-by: odo
          app.kubernetes.io/managed-by-version: v2.0.5
          app.kubernetes.io/name: dh-postgresql-apb
          app.kubernetes.io/part-of: app
        name: psql
        namespace: myproject
      spec:
        clusterServiceClassExternalName: dh-postgresql-apb
        clusterServicePlanExternalName: dev
        externalID: ""
        parameters:
          postgresql_database: my_data
          postgresql_password: secret
          postgresql_user: luke
          postgresql_version: "9.6"
        updateRequests: 0
      status:
        asyncOpInProgress: false
        conditions: null
        deprovisionStatus: ""
        observedGeneration: 0
        orphanMitigationInProgress: false
        provisionStatus: ""
        reconciledGeneration: 0
  name: psql
metadata:
  name: nodejs
  version: 2.0.0
schemaVersion: 2.0.0
starterProjects:
- git:
    remotes:
      origin: https://github.com/odo-devfiles/nodejs-ex.git
  name: nodejs-starter

However, for an s2i based component, this won't work till we have addressed #4281 so that a devfile.yaml is created for odo create --s2i.

dharmit on 1 Mar 2021

odo service delete should delete the entry from devfile for both Operators & Service Catalog.

This is currently blocked on https://github.com/devfile/api/issues/306.

dharmit on 1 Mar 2021

What's done so far (#4465):

odo service create adds CRD (Operators) and ServiceInstance (Service Catalog) yaml into devfile.yaml.
Library has been updated from v1 to v2 by Mrinal

What's pending (scope of this issue for Sprint 198):

[ ] Add CRD yaml to devfile.yaml when --from-file flag is passed. This flag is not available for use with Service Catalog.
[ ] Remove CRD and ServiceInstance yaml from devfile.yaml when user does odo service delete. This can be done only after https://github.com/devfile/library/pull/65 is merged
[ ] Tests.

Question:
Not entirely relevant right now. ~Currently, odo service create and odo service delete work fine from outside a context directory. With the changes we're planning in this issue, it won't be possible to run them without an underlying devfile.yaml. Are we fine with this change? @kadel @deboer-tim~

dharmit on 4 Mar 2021

@kadel PR #4465 implements the items discussed in https://github.com/openshift/odo/issues/4160#issuecomment-790577742 and https://github.com/openshift/odo/issues/4160#issuecomment-775838428. It's in review and I need to make a few changes to it for it to get merged.

But overall, this issue has wider scope. For Sprint 199, what would you think we should cover in the scope?

There's one question that I mentioned on a few calls - how would odo know which service to delete when odo service delete removes the service info from devfile.yaml? You mentioned that we don't want to store service related info in env.yaml. Then how do you suggest we show status information to the user when they do odo service delete (one of the acceptance criteria)?

I've added odo link/unlink to acceptance criteria as well because odo link is creating an Operand of the Service Binding Operator.

dharmit on 22 Mar 2021

A complete picture of how this should work at the end.

odo service create - adds Operator Backed Service CR to devfile.yaml as a kubernetes component.
odo push reads the content of the devfile and creates its representation on the cluster. Every Kubernetes resource created from the devfile needs to be labeled at least with the following labels: app.kubernetes.io/instance, app.kubernetes.io/managed-by, app.kubernetes.io/name
- Those labels are odo implementation-specific. They should not appear in devfile.yaml.
odo push needs to be able to detect differences between the cluster and local state (devfile.yaml). For example, somewhere early in the process, it needs to list all CRs in a given namespace with matching app.kubernetes.io/instance, app.kubernetes.io/managed-by, app.kubernetes.io/name labels and compare that list with kuberentes components in devfile.
- If resource matching the labels is present in the cluster and it is not present in devfile, that is an indication that it was previously deleted from devfile using odo service delete and it should be removed from the cluster.
odo service list should have additional fields to let the user know if the service is managed by odo or not.
▶ odo service list NAME MANAGED BY STATE EtcdCluster/example odo Locally Deleted PostgresSQLCluster/example odo Not Pushed PostgresSQLCluster/example2 odo Pushed Someting/else Someting/else2 other-tool
- The information for MANAGED BY column is taken from app.kubernetes.io/managed-by label. Services managed by odo are listed first.
- STATE column indicates the differences between the cluster and devfile. It makes sense only for services managed by odo.
  - Locally Deleted - present on the cluster but not in the devfile (next odo push will delete it from the cluster)
  - Not Pushed - present in the devfile but not on the cluster (next odo push will create it in the cluster)
  - Pushed - present in both places (next odo push will do nothing or update the existing CR if is is not the same as in devfile)
- odo service delete - deletes the service from devfile. This means that executing it on a service that is not managed by odo will fail.

kadel on 23 Mar 2021

Locally Deleted - present on the cluster but not in the devfile (next odo push will delete it from the cluster)

So odo push should be smart to check for this states and do relevant actions every time, right? Now consider this scenario:

I have two components having their own component directories. Let's call them c1 and c2.
I created a service called s1 from c1's dir. odo makes an entry in devfile. Subsequent odo push from c1's dir creates the service s1 on cluster.
Now I need to do something with c2. I cd into its component directory. Now let's say I only modified something related to code. And I do odo push.
But odo push is smart, and it checks that a service s1 on the cluster is not present in devfile and deletes service s1.

What is the right thing to do? How do you suggest odo do the right thing?

dharmit on 24 Mar 2021

But odo push is smart, and it checks that a service s1 on the cluster is not present in devfile and deletes service s1.

no it won't. If you did odo push from c2 dir won't touch s1 service, because that belongs to c1, odo know that from labels.

kadel on 24 Mar 2021

👍1

odo service list should have additional fields to let the user know if the service is managed by odo or not.

Considering the same example that I gave in https://github.com/openshift/odo/issues/4160#issuecomment-805817724, should odo service list show all services across all components in the namespace or should it only list the services matching the label of a component?

If we go with the former, we could keep odo service list runnable from any directory on the system. Also, should odo link allow linking between services managed by different components in this case? As in, if service s1 was started from component c1's context, should component c2 be able to link to service s1 since it can see the service s1 in the output of odo service list?

If we go with the latter, we need to make sure that odo service list is executed from a valid component directory only.

▶ odo service list
NAME                      MANAGED BY          STATE
EtcdCluster/example          odo            Locally Deleted
PostgresSQLCluster/example   odo            Not Pushed
PostgresSQLCluster/example2  odo            Pushed
Someting/else                
Someting/else2               other-tool

When we show other-tool in above output to the user, should we figure out what tool is managing it? If yes, how do we ensure that other tools will use the same label as odo does? If no, should we just put hardcoded string other-tool to indicate that it's not managed by odo? Can we instead change the column name to MANAGED BY ODO and print a tick or a cross indicating true or false? (thinking out loud)

dharmit on 25 Mar 2021

Well that's a lot of questions :). Let me give answering them a try:

odo service list should have additional fields to let the user know if the service is managed by odo or not.

Considering the same example that I gave in #4160 (comment), should odo service list show all services across all components in the namespace or should it only list the services matching the label of a component?

All, because you might want to link to a service you just created within the component, or you might want to link to an 'external' service or one that already exists on the cluster.

If we go with the former, we could keep odo service list runnable from any directory on the system.

I think that's a very good goal that we should try to keep. The labels should still tell us if some of them are part of a component, and we can show that, we just won't know if that component is local, on someone else's machine, or whatever.

Also, should odo link allow linking between services managed by different components in this case? As in, if service s1 was started from component c1's context, should component c2 be able to link to service s1 since it can see the service s1 in the output of odo service list?

I don't know if that's a frequent case, but yes, I think it should be possible. To c2 it's just another service and doesn't particularly care how it got there.

If we go with the latter, we need to make sure that odo service list is executed from a valid component directory only.

I don't follow that. If you're outside wouldn't it still know that s1 came from c1?

▶ odo service list
NAME                      MANAGED BY          STATE
EtcdCluster/example          odo            Locally Deleted
PostgresSQLCluster/example   odo            Not Pushed
PostgresSQLCluster/example2  odo            Pushed
Someting/else                
Someting/else2               other-tool
When we show other-tool in above output to the user, should we figure out what tool is managing it? If yes, how do we ensure that other tools will use the same label as odo does? If no, should we just put hardcoded string other-tool to indicate that it's not managed by odo? Can we instead change the column name to MANAGED BY ODO and print a tick or a cross indicating true or false? (thinking out loud)

We should use labels that are as standard as we can find, but I agree not every tool will use them, and so we won't know anything about those services. If you were in c2's folder I thought the output might look something like this:

▶ odo service list
NAME                      MANAGED BY          STATE
EtcdCluster/example          c1
PostgresSQLCluster/example   c2            Not Pushed
PostgresSQLCluster/example2  c2            Pushed
Someting/else                app7
Someting/else2               unknown

c1 was pushed by odo (maybe we could indicate this as well), but since we don't know where it came from we don't know the status. We're in c2 so we can output the status of those. app7 came from a different tool, but since it used labels we can at least fill the 'managed by' in, and the last service didn't have any labels.

deboer-tim on 25 Mar 2021

👍1

If we go with the latter, we need to make sure that odo service list is executed from a valid component directory only.

I don't follow that. If you're outside wouldn't it still know that s1 came from c1?

If we add the MANAGED BY column to odo service list, I think it'll be possible to know from anywhere that s1 belongs to component c1.

I'm assuming that MANAGED BY indicates an odo component here and not a tool (odo/ODC/CRW.) That's different from what @kadel suggested in MANAGED BY (more around which tool manages it rather than which odo component.)

If you were in c2's folder I thought the output might look something like this:
▶ odo service list
NAME                      MANAGED BY          STATE
EtcdCluster/example          c1
PostgresSQLCluster/example   c2            Not Pushed
PostgresSQLCluster/example2  c2            Pushed
Someting/else                app7
Someting/else2               unknown
c1 was pushed by odo (maybe we could indicate this as well), but since we don't know where it came from we don't know the status. We're in c2 so we can output the status of those. app7 came from a different tool, but since it used labels we can at least fill the 'managed by' in, and the last service didn't have any labels.

Makes sene. We'll need to make docs clearly say that STATE can be expected only when working from a component dir.

I'll try to jot down the scope based on what we discussed on this issue. Thanks for your inputs @kadel & @deboer-tim

dharmit on 29 Mar 2021

Scope for next pass on this issue:

[x] Get #4465 merged!
[x] odo push should invisibly set labels to the ~component~ service (Kubernetes Inlined component) - "invisibly" means label info won't be stored in devfile because same devfile can be used by other tools (ODC/CRW) and each tool should assign labels of their own. (#4656)
[ ] odo push should create the service on the cluster instead of odo service create doing it right now.
- [x] Operators (#4650)
- [ ] Service Catalog
[ ] odo push should delete the service from the cluster instead of odo service delete doing it right now.
- [ ] Operators
- [ ] Service Catalog

dharmit on 29 Mar 2021

odo push should create the service on the cluster instead of odo service create doing it right now.

I'm considering adding the code to create service right before https://github.com/openshift/odo/blob/16946d4068f30f1e51cecc64877e535338b043b2/pkg/devfile/adapters/kubernetes/component/adapter.go#L175-L178

Thinking along these lines because there's a call to the function WaitForDeploymentRollout. And I'm taking it literally as a function that will wait for something to complete.

@kadel @mik-dass @girishramnani @valaparthvi if you have a differing opinion/suggestion, let me know.

dharmit on 20 Apr 2021

👍1

Scope of this issue for Sprint 201:

[ ] odo service list should show output like below from the component directory of component-1:
sh $ odo service list NAME MANAGED BY ODO STATE EtcdCluster/etcd-1 component-1 Pushed EtcdCluster/etcd-2 component-2 Pushed EtcdCluster/etcd-3 component-1 Not Pushed EtcdCluster/etcd-4 component-1 Deleted Locally
[ ] odo service delete should delete the service only from devfile.yaml; odo push should delete the service from the cluster.
[ ] Tests

I'll defer documentation till we have properly defined structure via #4664.

dharmit on 6 May 2021

odo service create should add operator CR to the devfile

User Story

Acceptance Criteria

Links

All 24 comments

Scope of this issue for current iteration

Related issues