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

Update hack/generate-manifest.sh --mode antrea-e2e #321

Closed
heanlan opened this issue Jun 6, 2023 · 5 comments
Closed

Update hack/generate-manifest.sh --mode antrea-e2e #321

heanlan opened this issue Jun 6, 2023 · 5 comments
Labels
good first issue Good for newcomers test

Comments

@heanlan
Copy link
Contributor

heanlan commented Jun 6, 2023

hack/generate-manifest.sh --mode antrea-e2e is used to generates YAML manifest for e2e tests in Antrea repository, which should only includes a ClickHouse server with default credentials. We need the script to create the YAML manifest because it is easier to keep the ClickHouse data schema in both Theia and Antrea in-sync.

The current script is a bit out-of-date, since we added new features to the Theia, by running the command will give us a manifest with a lot of more resources other than the required ClickHouse server. We would like to remove all the unnecessary resources from the generated script, including theia-manager, zookeeper, theia-cli. We would also like to replace projects.registry.vmware.com/antrea/theia-clickhouse-server by projects.registry.vmware.com/antrea/clickhouse-server in the generated manifest, as the Antrea e2e test does not need to be run based on theia built images.

@heanlan heanlan added good first issue Good for newcomers internal internal code improvements labels Jun 6, 2023
@heanlan
Copy link
Contributor Author

heanlan commented Jun 6, 2023

The issue was firstly raised by @yanjunz97 in antrea-io/antrea#5020 (comment). Please feel free to correct me if I made anything wrong in the issue description.

@heanlan heanlan changed the title Update hack/generate-manifest.sh --antrea-e2e mode Update hack/generate-manifest.sh --mode antrea-e2e Jun 6, 2023
@yanjunz97
Copy link
Contributor

Thanks @heanlan for creating the issue! I would like to discuss a little bit more regarding ZooKeeper. As current data schema has dependency on ZooKeeper, e.g. we use replicated engine for tables, removing ZooKeeper in the generated YAML file might be a little bit difficult. But we still have some options:

  • Add some new parameters in the helm chart values file and replace the ZooKeeper related contents by what we have before introducing ZooKeeper when e2e is enabled. But this option might not be meaningful to expose to users.
e2e:
  enable: false
  • Instead of modifying in Helm chart, we can replace the ZooKeeper related text in hack/generate-manifest.sh to make sure the yaml file for antrea-e2e is still as expected.

I personally prefer the second one if we can make sure that implementation in shell script is readable.

@heanlan heanlan added test and removed internal internal code improvements labels Jun 8, 2023
@github-actions
Copy link

github-actions bot commented Sep 7, 2023

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@yuntanghsu
Copy link
Contributor

@yanjunz97 @heanlan
I had removed the flow-visibility-e2e.yaml and we created a helm charts under antrea/e2e/test in this PR
antrea-io/antrea#5171
Do we still want to update --mode antrea-e2e for Antrea? Or we can close this issue?

@yanjunz97
Copy link
Contributor

Thanks YunTang for pointing out this. I think its good to close this issue. Let me close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers test
Projects
None yet
Development

No branches or pull requests

3 participants