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

Normalized hostname by concatenating scenarioName with index number #1209

Closed

Conversation

zrlw
Copy link
Contributor

@zrlw zrlw commented Mar 5, 2025

By now, the default hostname of every dubbo-samples testing container is set to the service name, but

  1. Many test projects define same service name in their case-configuration.yml, these test projects could not be ran simutaneously because two or more containers could not using same hostname at the same time,
  2. it is difficult to ensure that manually setting service name do not overlap with other projects,
  3. scenarioName might be unique because it's generated from project name which should be unique,
  4. there are maximum length limit of hostname in container,
    so normalizing hostname by concatenating scenarioName with index number might alleviate this issue.

Setting hostname manually if some service system property or environment value contains hostname as it is difficult to determine whether part of a property or environment value is a hostname. In this situation, hostname should be set to global unique value explicitly at case-configuration.yml, and it should be different from serviceName to let this PR don't normalize it, just like this one:

services:
  provider:
    #set global unique hostname if some service system property or environment value contains but is not equal to the servicename: provider
    hostname: some-global-unique-name
    ...
  test:
    environment: 
      # the XXX value contains but is not equal to the hostname of provider.
      - XXX=jdbc:mysql://some-global-unique-name:3306/xxx
    systemProps:
      #the property value contains but is not equal to the hostname of provider.
      - some-property-name=some-global-unique-name:3306

@AlbumenJ PTAL

@zrlw zrlw force-pushed the feature-addScenarioNamePrefixToHostname branch from e9c2e81 to 773c210 Compare March 5, 2025 12:38
@zrlw zrlw changed the title Added scenarioName prefix to host name Normalized hostname by concatenating scenarioName with index number Mar 5, 2025
@zrlw zrlw force-pushed the feature-addScenarioNamePrefixToHostname branch from 773c210 to 1fa8c10 Compare March 5, 2025 23:10
@zrlw zrlw force-pushed the feature-addScenarioNamePrefixToHostname branch from 1fa8c10 to 488f9c4 Compare March 6, 2025 00:23
@zrlw
Copy link
Contributor Author

zrlw commented Mar 7, 2025

for example, dubbo-samples-async-generated-future-springboot named test as it's consumer hostname,
dubbo-samples-async-generated-future-springboot
but there are more than one test projects besides dubbo-samples-async-generated-future-springboot named their test container hostname as test, when github actions run them concurrently, the problem that test container could not be started might occur, just like this one
https://github.com/apache/dubbo/actions/runs/13666970507/job/38210252086

Failed tests: 1
[4/34] [dubbo-sample-async-generated-future-springboot:1/1] TEST FAILURE: Run tests timeout, version: -Ddubbo.version=3.2.17 -Dspring.version=5.3.24 -Djava.version=8, please check logs: /home/runner/work/dubbo/dubbo/2-advanced/dubbo-samples-async/dubbo-sample-async-generated-future-springboot/target/logs

TEST FAILURE

@zrlw zrlw force-pushed the feature-addScenarioNamePrefixToHostname branch from 522e53b to 6196e0d Compare March 7, 2025 07:18
@zrlw
Copy link
Contributor Author

zrlw commented Mar 7, 2025

i think there is an alternative solution:
Throw invalid configuration exception if service name does not contains scenarioName at ConfigurationImpl, force developers revise servicename at case-configuration.yml :)

@AlbumenJ
Copy link
Member

i think there is an alternative solution: Throw invalid configuration exception if service name does not contains scenarioName at ConfigurationImpl, force developers revise servicename at case-configuration.yml :)

I agree :)

@zrlw
Copy link
Contributor Author

zrlw commented Mar 11, 2025

ok, too many changes might have a huge impact, let's endure it for a while, just modify the problematic case-configuration.yml separately when issues arise.

@zrlw zrlw closed this Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants