Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Make e2e ports configurable #3969

Closed
ksubrmnn opened this issue Aug 13, 2021 · 3 comments
Closed

Make e2e ports configurable #3969

ksubrmnn opened this issue Aug 13, 2021 · 3 comments
Labels

Comments

@ksubrmnn
Copy link
Contributor

Please describe the Improvement and/or Feature Request

Create a flag that would make service ports configurable for the e2e test applications.

Scope (please mark with X where applicable)

  • New Functionality [ ]
  • Install [ ]
  • SMI Traffic Access Policy [ ]
  • SMI Traffic Specs Policy [ ]
  • SMI Traffic Split Policy [ ]
  • Permissive Traffic Policy [ ]
  • Ingress [ ]
  • Egress [ ]
  • Envoy Control Plane [ ]
  • CLI Tool [ ]
  • Metrics [ ]
  • Certificate Management [ ]
  • Sidecar Injection [ ]
  • Logging [ ]
  • Debugging [ ]
  • Tests [X]
  • CI System [ ]
  • Demo [ ]
  • Project Release [ ]

Possible use cases

This would help developers run the tests in a variety of cluster configurations. For example, OpenShift does not allow port 80, but devs may still want to test port 80 in a different environment.

@ksubrmnn ksubrmnn added area/e2e-tests End-to-end tests good first issue Good for newcomers labels Aug 13, 2021
@shashankram
Copy link
Member

@ksubrmnn I don't see a strong use case for developers wanting to run the e2e tests on specific ports. Exposing more configurations and permutations in e2e only makes it unnecessarily complex and harder to debug. So far, I haven't seen a user request for this functionality, and am wondering if this is really necessary. It's not that it's hard to implement but might not add much value given that e2e testbeds are run in a controlled environment.

If there is room to avoid unnecessary complexity, I'd strongly prefer that.

@ksubrmnn
Copy link
Contributor Author

ksubrmnn commented Aug 13, 2021

@shashankram I see your point of view. I created this issue so we could talk about it more since I know developers have gone back and forth on this issue. It would be great to close this out and have the final discussion here so it's documented.

I created this issue to follow up on this #2755 (comment) by @draychev. wdyt?

I think I heard interest from you in the past to make this configurable so that we weren't coding around just one environment (OpenShift), and we went with hardcoding to get the pipeline up quickly.

@shashankram
Copy link
Member

@shashankram I see your point of view. I created this issue so we could talk about it more since I know developers have gone back and forth on this issue. It would be great to close this out and have the final discussion here so it's documented.

I created this issue to follow up on this #2755 (comment) by @draychev. wdyt?

I think I heard interest from you in the past to make this configurable so that we weren't coding around just one environment (OpenShift), and we went with hardcoding to get the pipeline up quickly.

I think we discussed this a few times recently and the general agreement was to avoid introducing complexity unless necessary. While we had to change the ports from 80 -> 14001 for OpenShift, we concluded that 14001 seemed like a port unlikely to result in conflicts with any distributions. In the past, there was an opinion on always using port 80, but that concern was resolved.

#2755 (comment) talks about randomizing ports within a specific range, and not allowing users to configure it. Allowing users to configure the port would conflict with the randomization ask in #2755 (comment). I think we could create a new issue to randomize the ports in e2e tests, but that should be a new issue with context on the coverage that it would provide.

I am closing this issue since this has been discussed a few times already in the past and I don't see a use case for it. I suggest opening a new issue for the ask in #2755 (comment) to randomize the ports used in e2e testing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants