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

Use vineyardctl API to inject the vineyard sidecar #2612

Merged
merged 4 commits into from May 31, 2023

Conversation

dashanji
Copy link
Collaborator

@dashanji dashanji commented Apr 17, 2023

🤖 Generated by Copilot at 1581da2

This pull request refactors the vineyard deployment in GraphScope to use a sidecar container instead of a separate container in the engine pod. This improves the performance and stability of the system and simplifies the management of vineyard. It also fixes some typos and errors in the documentation. The main changes are in coordinator/gscoordinator/cluster_builder.py and coordinator/gscoordinator/kubernetes_launcher.py.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 17, 2023

🎊 PR Preview 8d1721f has been successfully built and deployed to https://alibaba-graphscope-build-pr-2612.surge.sh

🤖 By surge-preview

@dashanji dashanji changed the title [WIP] Use vineyardctl API to inject the vineyard sidecar Use vineyardctl API to inject the vineyard sidecar Apr 19, 2023
@dashanji dashanji force-pushed the install-vineyard-as-sidecar branch 3 times, most recently from 1785330 to 1581da2 Compare April 19, 2023 11:39
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2023

Codecov Report

Merging #2612 (8d1721f) into main (5b46a68) will decrease coverage by 6.17%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2612      +/-   ##
==========================================
- Coverage   73.61%   67.45%   -6.17%     
==========================================
  Files          99       99              
  Lines       10392    10381      -11     
==========================================
- Hits         7650     7002     -648     
- Misses       2742     3379     +637     

see 20 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b46a68...8d1721f. Read the comment docs.

# 2. get the ownerReference of the coordinator
# 3. apply the etcd cluster for vineyard sidecar
# 4. return the json string of new workload which is injected with vineyard sidecar
def _inject_vineyard_as_sidecar(self, workload):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you give a sample output of the injected json in the comment? One might want to know which portion of the body will be modified and how many components would be added to that, in later development process.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think better to give a link to the documentation of vineyardctl.inject.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I will add it later.

@dashanji dashanji force-pushed the install-vineyard-as-sidecar branch 7 times, most recently from 379ac2b to 6d9a09d Compare April 25, 2023 02:31
@dashanji dashanji force-pushed the install-vineyard-as-sidecar branch 5 times, most recently from c101008 to a35746d Compare May 11, 2023 07:09
@dashanji dashanji added this to In progress in GraphScope Sprint April-May May 23, 2023
@dashanji dashanji force-pushed the install-vineyard-as-sidecar branch 10 times, most recently from 0195452 to de68992 Compare May 30, 2023 09:27
* Fix some typos in the FAQ doc.
* Add the apply_resources args for vineyardctl to deploy an external etcd cluster and a vineyard rpc service.
* Set up the OwnerReferences for all injected manifests of vineyardctl.
* Add examples and usage about vineyardctl inject API.

Signed-off-by: Ye Cao <caoye.cao@alibaba-inc.com>
…vineyardctl.

Signed-off-by: Ye Cao <caoye.cao@alibaba-inc.com>
Signed-off-by: Ye Cao <caoye.cao@alibaba-inc.com>
Signed-off-by: Ye Cao <caoye.cao@alibaba-inc.com>
@dashanji
Copy link
Collaborator Author

@siyuan0322 @sighingnow could you please take a look at this? Thanks a lot.

@sighingnow sighingnow merged commit fdc626e into alibaba:main May 31, 2023
39 of 40 checks passed
GraphScope Sprint April-May automation moved this from In progress to Done May 31, 2023
@dashanji dashanji deleted the install-vineyard-as-sidecar branch May 31, 2023 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants