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

bug(fe): route created with service can not use the service upstream #1278

Closed
liuxiran opened this issue Jan 13, 2021 · 10 comments · Fixed by #1302
Closed

bug(fe): route created with service can not use the service upstream #1278

liuxiran opened this issue Jan 13, 2021 · 10 comments · Fixed by #1302
Assignees
Labels
bug Something isn't working checking frontend
Milestone

Comments

@liuxiran
Copy link
Contributor

Bug report

Describe the bug

Refer to the description, when create a route with service id, this route will have the same upstream and plugin configuration.

When I try to create a route with service in dashboard, although I have selected a service in the first step, I still have to define a newupstream because of the required validation in the second step, and at the same time, the new upstream definition will cover the service's upstream.

How to Reproduce

  1. create a service
  2. create a route, select the service in the first step, click Next button
  3. in the second step, we have to define the upstream

Expected behavior

I would be better to use service data to auto fill in the upstream form in the second step.

System information

  • OS: Fedora 32
  • Browser (if applies) Chrome
  • Version: master branch
@liuxiran liuxiran added the bug Something isn't working label Jan 13, 2021
@juzhiyuan
Copy link
Member

Creating Route needs to be updated according to [1], please take a look.

image

[1] #1213

@juzhiyuan juzhiyuan added this to the 2.4 milestone Jan 13, 2021
@juzhiyuan
Copy link
Member

ping

@liuxiran
Copy link
Contributor Author

Creating Route needs to be updated according to [1], please take a look.

image

[1] #1213

Got it, so IMHO, when create a route with a service, we need to :

  1. auto fill in the upstream with the service's upstream info
  2. user can also specify difference upstream according to their requirements

what do you think? @juzhiyuan @LiteSun @idbeta

@idbeta
Copy link
Contributor

idbeta commented Jan 14, 2021

Got it, so IMHO, when create a route with a service, we need to :

  1. auto fill in the upstream with the service's upstream info
  2. user can also specify difference upstream according to their requirements

what do you think? @juzhiyuan @LiteSun @idbeta

Agree, but the option of specify difference upstream should not be necessary.
@membphis

@juzhiyuan
Copy link
Member

Hi @liuxiran, specify different upstream is a historical legacy 🤔 not sure if we should remove this part in the future, only allow users to bind an upstream or a service.

@liuxiran
Copy link
Contributor Author

Thanks for your reply @juzhiyuan @idbeta .

so when create a route with a service, we only need to auto fill in the upstream with the service upstream info, and at the same time, user could not specify it, am I right?

@juzhiyuan
Copy link
Member

juzhiyuan commented Jan 14, 2021

Once we auto fill upstream info from service, a new upstream object would be bound to that route unexpectedly IMO ⚠️

@liuxiran liuxiran self-assigned this Jan 14, 2021
@liuxiran
Copy link
Contributor Author

liuxiran commented Jan 14, 2021

Once we auto fill upstream info from service, a new upstream object would be bound to that route unexpectedly IMO

you mean we should omit the upstream in the request body, right? Ok, got it, it is the same as mentioned in the https://github.com/apache/apisix/blob/master/doc/architecture-design.md#service.

I just assigned myself, and the pr will be opened later this week^_^

@LiteSun LiteSun self-assigned this Jan 15, 2021
@liuxiran liuxiran removed their assignment Jan 15, 2021
@liuxiran
Copy link
Contributor Author

When I tried to create a route with service_id, None select option would add to the select_upstream, but I could still custom the upstream

image

image

this result seems not meet my expectation, so I wonder if it is intentional or it still needs to improve?

cc @LiteSun @juzhiyuan

@juzhiyuan
Copy link
Member

Service or Upstream is an entity for Route, because the older dashboard supports custom upstream, and the upstream data is stored but not linked or referenced in etcd, once we omit upstream after selecting service, some unexpected issues would happen.

But we could support after service is selected, upstream turns to be optional 🤔 Does it make sense to your concern? Let me know if not 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working checking frontend
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants