odo url create should try to use an existing endpoint in the devfile rather than creating a new one

[scottkurz]

/kind bug
you using?

Operating System: Windows/Linux

Output of odo version: odo v2.0.6 (2f7f4a090)

How did you run odo exactly?

Basically following instructions: https://odo.dev/docs/deploying-a-devfile-using-odo/#deploying-an-open-liberty-application-to-an-openshift--kubernetes-cluster

Steps

1. Start from a minikube+ingress cluster ctx
2. git clone https://github.com/OpenLiberty/application-stack-intro.git && cd application-stack-intro
3. odo create myopenliberty
4. odo url create --host $(minikube ip).nip.io
5. odo push

Actual behavior

In devfile.yaml

I see two endpoints:

components:
- container:
    endpoints:
    - exposure: public
      name: ep1
      path: /
      protocol: http
      targetPort: 9080
    - exposure: public
      name: mol234-9080
      path: /
      protocol: http
      targetPort: 9080

$ odo push

Validation
✓ Validating the devfile [75690ns]

Creating Kubernetes resources for component mol234
✓ Waiting for component to start [5s]
✓ Waiting for component to start [12ms]
⚠ Unable to create ingress, missing host information for Endpoint ep1, please check instructions on URL creation (refer odo url create --help)

Expected behavior

I think odo should reuse my devfile's existing endpoint. I think the devfile should not need an update and I think the warning message on 'ep1' should not be issued (potentially confusing the user).

I had wondered if maybe odo should handle this via an 'odo url update' rather than 'odo url create'. However, thinking about it more, the odo URL constuct already spans across and abstracts the details of each of:

1. state in the devfile.yaml
2. state in the odo env.yaml
3. state in the cluster

I think instead odo url create should do the right thing and use the existing devfile endpoint, as long as it detects there's nothing incompatible about the request.

One other point leading me to this conclusion is noting that I don't need to pass the --port to created the ingress-based endpoint...somehow odo uses the '9080' from my devfile endpoint even though it creates a new endpoint.

Background

We pre-defined the 'ep1' endpoint in the devfile so that, on OpenShift, odo push will cause a usable OpenShift route to be created without an odo url create

Bigger goal

My main goal is to include a pre-defined endpoint such that on each of simple: OpenShift, k8s+ingress scenarios, I can at least avoid rewriting the devfile on url create.

Any logs, error output, etc?

[kadel]

This actually makes a lot of sense. We should do something with this. The current behavior is not good.

[mik-dass]

@kadel @scottkurz What will be the behavior if the cloned devfile has multiple endpoints in a minikube setup? Should the first url create command update one of them and keep the others or delete the rest as well?

[dharmit]

$ odo push

Validation
✓  Validating the devfile [75690ns]
Creating Kubernetes resources for component mol234
✓  Waiting for component to start [5s]
✓  Waiting for component to start [12ms]
warning  Unable to create ingress, missing host information for Endpoint ep1, please check instructions on URL creation (refer odo url create --help)

Do we know for sure that we're not hitting the behaviour reported/discussed in #4270? I'm asking because the warning message @scottkurz is seeing here is being reported even when things work just fine. I saw this message quite recently when working with minikube + ingress.

I think odo should reuse my devfile's existing endpoint. I think the devfile should not need an update and I think the warning message on 'ep1' should not be issued (potentially confusing the user).

@mik-dass this sounds along the lines of what I was saying on the call yesterday - if endpoint ep1 already exists for the port & protocol combination 9080 & http, subsequent attempt to create an endpoint mol234-9080 should fail instead of overriding the existing endpoint ep1 in the devfile.

[dharmit]

What will be the behavior if the cloned devfile has multiple endpoints in a minikube setup?

Do you mean multiple endpoints for same port+protocol combination? Unless someone has manually put this information in a devfile, should the library even allow such multiple endpoints in devfile.yaml? Is it valid from devfile schema point of view?

[mik-dass]

@mik-dass this sounds along the lines of what I was saying on the call yesterday - if endpoint ep1 already exists for the port & protocol combination 9080 & http, subsequent attempt to create an endpoint mol234-9080 should fail instead of overriding the existing endpoint ep1 in the devfile.

This issue seems to be about removing the unusable URLs from the devfile or updating the existing ones with the new info.

Do you mean multiple endpoints for same port+protocol combination? Unless someone has manually put this information in a devfile, should the library even allow such multiple endpoints in devfile.yaml? Is it valid from devfile schema point of view?

There is no such limitation mention in the schema so I think it is a valid.

[scottkurz]

UPDATE with 2.0.7

I just tried this again with 2.0.7 and it seems to work differently. A second, new endpoint is created and my original endpoint is deleted (or you might say overwritten).

The problem though is that I have a source change, so if I'm versioning my devfile in Git this shows up as a change, though I'm arguing that it shouldn't.

I'm saying the "odo url create" should logically combine the endpoint already existing in the devfile and add some new odo env.yaml metadata, without updating the original devfile unnecessarily.

If I already expose the endpoint in source there's no need to trigger a source change/update (possibly rewriting my devfile in a totally different order) for this change.

REPLIES TO COMMENTS

@kadel @scottkurz What will be the behavior if the cloned devfile has multiple endpoints in a minikube setup? Should the first url create command update one of them and keep the others or delete the rest as well?

@mik-dass I admit I haven't used components exposing multiple endpoints yet.

But from the help text in odo help url create:

  --port int            Port number for the url of the component, required in case of components which expose more than one service port (default -1)

it seems you would need to pass the port as an argument, so instead of:

odo url create --host $(minikube ip).nip.io

I'd have to do

odo url create --host $(minikube ip).nip.io --port 9080

and perhaps if there are two endpoints both with the same port it would be an error? (That also implies that it is an error to add a second endpoint with the same targetPort, which also supports treating this issue as a concern).

@dharmit wrote:

Do we know for sure that we're not hitting the behaviour reported/discussed in #4270?
I wasn't originally primarily worried about the warning message, but the fact that the devfile was getting updated / rewritten.

[kadel]

UPDATE with 2.0.7

I just tried this again with 2.0.7 and it seems to work differently. A second, new endpoint is created and my original endpoint is deleted (or you might say overwritten).

I've tried this with

▶ odo version
odo v2.1.0 (6040118b0)

And I see original behavior as reported in the issue description.

My take on this is:

Case 1:
If devfile has only one endpoint and the user runs odo url create without specifying port. Odo should assume that the user wants to add or override information for the existing URL.
Example:
if devfile contains

      endpoints:
        - exposure: public
          path: /
          name: ep1
          targetPort: 9080
          protocol: HTTP

running odo url create --host example.com will just attach hostname to the existing ep1 endpoint. This means that it will add the following to env.yaml and it won't touch devfile.yaml

Url:
  - Name: ep1
    Host: 192.168.49.2.nip.io
    Kind: ingress

Case 2:
If devfile has multiple endpoints running odo url create without --port will error out telling the user that --port is required because there are multiple endpoints in devfile.

Case 2a:
Running odo url create with --port that already exists in devfile will result in adding additional information to the endpoint. This is the same situation as in Case 1

Case 2b:
Running odo url create with --port that doesn't exists in devfile, will create a new endpoint in devfile.yaml

[mik-dass]

Currently the default path value seems to be / for odo URLs but devfile doesn't have any default value for path. Should we remove it? Or should we treat the empty path to be /?

[kadel]

Currently the default path value seems to be / for odo URLs but devfile doesn't have any default value for path. Should we remove it? Or should we treat the empty path to be /?

Based on how URL works empty path and / should mean the same thing.

[mik-dass]

Based on how URL works empty path and / should mean the same thing.

Should we write the default value / to the devfile, like we do currently, or change the current behavior and leave it with empty value?

[kadel]

Should we write the default value / to the devfile, like we do currently, or change the current behavior and leave it with empty value?

Empty if path was not specified. If it was specified (even in case someone put /) it should have the correct value.

odo url create - no path in endpoint
odo url create --path /asfd - endpoint should have path: /asfd
odo url create --path / - endpoint should have path: /

[mik-dass]

Empty if path was not specified. If it was specified (even in case someone put /) it should have the correct value.

OK then we would need to change the current behavior of defaulting the value to / and saving it in the devfile.

[mik-dass]

@kadel @scottkurz Please check if https://github.com/openshift/odo/pull/4567 satisfies your requirements. One concern is that this behavior can end up overwriting endpoint values set by other tools or set manually.

[scottkurz]

@kadel @scottkurz Please check if #4567 satisfies your requirements. One concern is that this behavior can end up overwriting endpoint values set by other tools or set manually.

@mik-dass could you mention more what scenario were you concerned about overwriting values with?

If we needed to, I wonder if it'd help to have a new odo url update subcommand to distinguish btw. new vs. existing endpoints. But I think of 'url' as an odo construct rather than a devfile construct so create seems appropriate to me, and I think Tomas' breakdown: https://github.com/openshift/odo/issues/4493#issuecomment-818667275 makes sense.

Commented in #4567 that the fix looks good to me.

[mik-dass]

@mik-dass could you mention more what scenario were you concerned about overwriting values with?

I meant if the user edits the devfile manually and triggers odo url create, we might end up overriding his changes.

# user manually changes the endpoint values in the devfile
odo create <component_type>
odo url create

This might end up overwriting his changes.

Though after a discussion with @kadel , we decided not to worry about such scenarios for now.

[kadel]

I meant if the user edits the devfile manually and triggers odo url create, we might end up overriding his changes.

this is ok
odo url create is a command that can edit devfiles. Users running this should be expected that devfiles will be modified.