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

Refactor: Test Framework and Update Docker Compose Configuration #701

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

rohansx
Copy link

@rohansx rohansx commented Mar 17, 2024

Key Changes:

  • Created a new docker-compose.yml file to manage the Xline cluster configuration, ensuring all services are correctly networked and equipped with the necessary volumes, ports, and environment settings.
  • Refactored the validation.sh script to utilize Docker Compose for starting and stopping services, aligning it with the new docker-compose.yml configuration. This update simplifies the validation process and ensures compatibility with the Docker Compose setup.
  • Updated the benchmark.sh and quick_start.sh documentation to reflect the new Docker Compose-based approach. This includes instructions on how to use the docker-compose file for setting up a development or testing environment and guidelines for executing validation and benchmark tests.

These changes aim to streamline the project's testing framework, reduce redundancy, and improve the overall developer experience when testing and validating the Xline cluster's functionality.

Resolves: #692

@rohansx
Copy link
Author

rohansx commented Mar 17, 2024

@Phoenix500526 Pleaase review this PR and let me know the changes and then I'll proceed with the documentation

@rohansx rohansx changed the title Refacot: Test Framework and Update Docker Compose Configuration Refactor: Test Framework and Update Docker Compose Configuration Mar 17, 2024
Copy link

codecov bot commented Mar 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.16%. Comparing base (eadf2d9) to head (eaea991).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
- Coverage   75.17%   75.16%   -0.02%     
==========================================
  Files         176      176              
  Lines       26852    26852              
  Branches    26852    26852              
==========================================
- Hits        20187    20183       -4     
- Misses       5452     5454       +2     
- Partials     1213     1215       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ci/docker-compose.yml Outdated Show resolved Hide resolved
ci/docker-compose.yml Outdated Show resolved Hide resolved
ci/docker-compose.yml Show resolved Hide resolved
ci/docker-compose.yml Show resolved Hide resolved
@rohansx
Copy link
Author

rohansx commented Mar 21, 2024

@liangyuanpeng adjusted port configurations for xLine nodes and added client container for enhanced interaction. @Phoenix500526 updated node2 and node3 ports to avoid special port conflicts and included --client-listen-urls and --peer-listen-urls for clearer networking. Also added a client service for testing and interaction purposes.

Copy link
Collaborator

@Phoenix500526 Phoenix500526 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase this pr.

ci/docker-compose.yml Outdated Show resolved Hide resolved
@rohansx
Copy link
Author

rohansx commented Mar 21, 2024

@Phoenix500526 I have done the necessary modification as you mentioned above. I updated client container image for CI validation process to ghcr.io/xline-kv/etcdctl:v3.5.9

@Phoenix500526
Copy link
Collaborator

There are still some problems with the validation CI. Please fix them so that this pr can pass the CI. You can run the validation on your local via executing scripts/validation_test.sh.

@rohansx
Copy link
Author

rohansx commented Mar 22, 2024

There are still some problems with the validation CI. Please fix them so that this pr can pass the CI. You can run the validation on your local via executing scripts/validation_test.sh.

working on this.

@Phoenix500526
Copy link
Collaborator

Phoenix500526 commented Mar 25, 2024

There are several issues in this pr:

  1. please add a signed-off-by information in your commits.
  2. please format the commit message. The commit message "�[200~Refactor Test Framework and Update..." is invalid.
  3. The validation CI is still not pass. It seems the relevant container created by docker-compose named "ci-client-1", not client.

@rohansx
Copy link
Author

rohansx commented Mar 30, 2024

@Phoenix500526 @liangyuanpeng , I am encountering a validation check error, and I have tried to resolve it without success. Could you please explain why this validation check is failing and provide guidance on how I can address the issue?

@@ -0,0 +1,85 @@
version: "3.9"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version is obsolete, please remove this line. FYI: mailcow/mailcow-dockerized#5797


services:
node1:
image: ghcr.io/xline-kv/xline:latest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add container_name to specify the container name.

--peer-listen-urls http://172.22.0.2:2380

node2:
image: ghcr.io/xline-kv/xline:latest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

--peer-listen-urls http://172.22.0.3:2380

node3:
image: ghcr.io/xline-kv/xline:latest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

--peer-listen-urls http://172.22.0.4:2380

client:
image: ghcr.io/xline-kv/etcdctl:v3.5.9
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

driver: bridge
ipam:
config:
- subnet: "172.22.0.0/16"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep this subnet unify with the subnet in validation_test.sh

@Phoenix500526
Copy link
Collaborator

@Phoenix500526 @liangyuanpeng , I am encountering a validation check error, and I have tried to resolve it without success. Could you please explain why this validation check is failing and provide guidance on how I can address the issue?

I think there are several reasons behind this validation failure:

  1. The docker-compose will generate a default name for those containers without specifying their container_name. In this case, the name of the client container is ci-client-1, not client. Hence, the validation test will get an error like "No such container named client"
  2. In this pr, you changed the subnet from 172.20.0.0/16 to 172.22.0.0/16, but you forgot to update the ip address in the validation_test.sh.
  3. The original quick_start.sh mounts ./scripts to /mnt and the validation_test.sh uses the ./scripts/validation_lock_client to do some test stuff.

Finally, please modify our quick start doc to make it unify with the code change in this pr. Thanks 😄

@mergify mergify bot requested a review from a team April 19, 2024 04:36
@Phoenix500526
Copy link
Collaborator

Hi, @rohansx ! This pr has been stalled for 3 weeks. Would you like to update it? 😄

@rohansx
Copy link
Author

rohansx commented Apr 24, 2024

Hi, @rohansx ! This pr has been stalled for 3 weeks. Would you like to update it? 😄

@Phoenix500526 yes I'll update in the next week.. sorry for the delay.. actually kind of looking for a job and got stuck in the interview process and therefore i couldn't look into it..

@Phoenix500526
Copy link
Collaborator

@Mergifyio rebase

Copy link

mergify bot commented May 16, 2024

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/14)
Auto-merging scripts/quick_start.sh
CONFLICT (content): Merge conflict in scripts/quick_start.sh
Auto-merging scripts/validation_test.sh
CONFLICT (content): Merge conflict in scripts/validation_test.sh
error: could not apply ffc737c... ?[200~Refactor Test Framework and Update Docker Compose Configuration
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply ffc737c... �[200~Refactor Test Framework and Update Docker Compose Configuration

Copy link

mergify bot commented May 20, 2024

@rohansx Your PR is in conflict and cannot be merged.

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.

[Feature]: Better testing framework
4 participants