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 the getting-started doc to include Theia Manager #147

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

yanjunz97
Copy link
Contributor

The commit update doc to include Theia Manager, as the CLI will be broken without Theia Manager enabled.
It also sets Theia Manager as enabled by default.

Signed-off-by: Yanjun Zhou zhouya@vmware.com

installing Theia:

```bash
helm install theia antrea/theia --set sparkOperator.enable=true,theiaManager.enable=true -n flow-visibility --create-namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Yanjun, I take a look together with the above paragraphs and found we are missing helm install flow-aggregator here.
Since I feel writing helm install flow-aggregator antrea/flow-aggregator --set clickHouse.enable=true,recordContents.podLabels=true -n flow-aggregator --create-namespace three times is a bit repetitive. Could we split the installation section of flow-aggregator and theia in these paragraphs? It may look better IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also feel the commands repetitive though I remember the original purpose to put them together is to keep the installation in "one step". Anyway it still sounds good to me to split the step under the current condition. Updated, thanks!

Signed-off-by: Yanjun Zhou <zhouya@vmware.com>
Copy link
Contributor

@dreamtalen dreamtalen left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@yuntanghsu yuntanghsu left a comment

Choose a reason for hiding this comment

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

LGTM

@yanjunz97 yanjunz97 merged commit 5b19efa into antrea-io:main Feb 2, 2023
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.

4 participants