Skip to content
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

odo service create should add operator CR to the devfile #4160

Closed
4 tasks done
girishramnani opened this issue Oct 27, 2020 · 25 comments · Fixed by #4761
Closed
4 tasks done

odo service create should add operator CR to the devfile #4160

girishramnani opened this issue Oct 27, 2020 · 25 comments · Fixed by #4761
Assignees
Labels
estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person kind/user-story An issue of user-story kind v2 Issue or PR that applies to the v2 of odo

Comments

@girishramnani
Copy link
Contributor

girishramnani commented Oct 27, 2020

/kind user-story

User Story

As a developer working with odo, when I add a new service to the application (like DB), I want to be able to easily share service definitions with my team so they can start the application with the service in the same way as me.

Acceptance Criteria

  • odo service create should not create any services on cluster (Creates service on cluster upon "odo push" #4650)
  • odo service create should store service definition in devfile as kubernetes component (ServiceInstance for SericeCatalog or CR for operator backed service) (Add and remove service info from devfile #4465)
  • odo service delete should delete the service from devfile (remove appropriate kubernetes component). Deletion from the cluster will happen once odo push is executed (similar to how we handle URLs).
  • odo service list should list services and their status. Important is to differentiate between statues like DeletedLocally, DeletedRemotely etc... (Similarly to what odo does for URLs)
    odo link should add Service Binding information to devfile.yaml and odo unlink should remove it from the file. Subsequent odo push should create/delete the actual link (a Service Binding). (being tracked by odo link should store link information in devfile #4208)

Links

/kind user-story

@openshift-ci-robot openshift-ci-robot added the kind/user-story An issue of user-story kind label Oct 27, 2020
@girishramnani girishramnani added the triage/needs-information Indicates an issue needs more information in order to work on it. label Oct 27, 2020
@dharmit
Copy link
Member

dharmit commented Oct 28, 2020

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?

@girishramnani
Copy link
Contributor Author

girishramnani commented Nov 2, 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.

@dharmit
Copy link
Member

dharmit commented Nov 4, 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.

@girishramnani
Copy link
Contributor Author

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.

@dharmit
Copy link
Member

dharmit commented Nov 6, 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

👍 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.

@kadel
Copy link
Member

kadel commented Nov 6, 2020

The first thing that needs to be addressed is #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
#4159. This needs to be the first step.

@dharmit
Copy link
Member

dharmit commented Nov 10, 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?

@kadel
Copy link
Member

kadel commented Nov 10, 2020

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

We need to create a separate issue for this. (just created #4208)

The link will come as next after we have #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.

@dharmit
Copy link
Member

dharmit commented Dec 4, 2020

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

@dharmit
Copy link
Member

dharmit commented Feb 9, 2021

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)

@kadel
Copy link
Member

kadel commented Feb 10, 2021

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

@dharmit dharmit added the estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person label Feb 11, 2021
@dharmit
Copy link
Member

dharmit commented Mar 1, 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
Copy link
Member

dharmit commented Mar 4, 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 Add Delete Comp, Command, Proj, StarterProj devfile/library#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 dharmit added estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person and removed estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person labels Mar 4, 2021
@dharmit
Copy link
Member

dharmit commented Mar 22, 2021

@kadel PR #4465 implements the items discussed in #4160 (comment) and #4160 (comment). 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.

@kadel
Copy link
Member

kadel commented Mar 23, 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.

@dharmit
Copy link
Member

dharmit commented Mar 24, 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?

@kadel
Copy link
Member

kadel commented Mar 24, 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.

@dharmit
Copy link
Member

dharmit commented Mar 25, 2021

  • 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?

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)

@deboer-tim
Copy link

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.

@dharmit
Copy link
Member

dharmit commented Mar 29, 2021

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
Copy link
Member

dharmit commented Mar 29, 2021

Scope for next pass on this issue:

@dharmit dharmit added estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person and removed triage/needs-information Indicates an issue needs more information in order to work on it. estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person labels Apr 12, 2021
@dharmit
Copy link
Member

dharmit commented Apr 20, 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
Copy link
Member

dharmit commented May 6, 2021

Scope of this issue for Sprint 201:

  • odo service list should show output like below from the component directory of component-1:
    $ 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.

@valaparthvi
Copy link
Contributor

  ```shell
  $ 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
  ```

@feloy I noticed that the state doesn't show up when running this command outside of a component directory. I don't think I have a question here, just an fyi, or is this expected for now and we'll be iterating over it?

➜  odo git:(pr/4819) odo service list
NAME               MANAGED BY ODO     STATE     AGE
Database/bkddb     Yes (bkd)                    15m22s

@rm3l rm3l added the v2 Issue or PR that applies to the v2 of odo label Jun 16, 2023
@rm3l rm3l added this to odo Project Jun 16, 2023
@rm3l rm3l moved this to Done ✅ in odo Project Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
estimated-size/XL (40-60) Rough sizing for Epics. About 3 sprints of work for a person kind/user-story An issue of user-story kind v2 Issue or PR that applies to the v2 of odo
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants