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

[lifecycle] Manage Un/Deployment of Meshery Adapters #8393

Merged
merged 62 commits into from
Aug 11, 2023

Conversation

theBeginner86
Copy link
Member

@theBeginner86 theBeginner86 commented Aug 6, 2023

Notes for Reviewers

  • Kubernetes
  • Docker
    • Able to deploy/undeploy
    • Detect running Meshery container and install adapters' containers in same network

This PR fixes #

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
@theBeginner86 theBeginner86 added the pr/draft WIP/Draft pull request label Aug 6, 2023
@github-actions github-actions bot added component/server framework/helm Issues related to helm-charts labels Aug 6, 2023
@github-actions
Copy link

github-actions bot commented Aug 6, 2023

@codecov
Copy link

codecov bot commented Aug 6, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04% ⚠️

Comparison is base (d02bf15) 5.20% compared to head (afe737c) 5.17%.
Report is 2 commits behind head on master.

❗ Current head afe737c differs from pull request most recent head 4a450a3. Consider uploading reports for the commit 4a450a3 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #8393      +/-   ##
=========================================
- Coverage    5.20%   5.17%   -0.04%     
=========================================
  Files         124     124              
  Lines       17664   17774     +110     
=========================================
  Hits          919     919              
- Misses      16573   16683     +110     
  Partials      172     172              
Flag Coverage Δ
gointegrationtests 5.17% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
server/handlers/mesh_ops_handlers.go 0.74% <ø> (ø)
server/helpers/adapters_tracker.go 0.00% <0.00%> (ø)
server/helpers/error.go 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

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

Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
@suhail34
Copy link
Contributor

suhail34 commented Aug 7, 2023

Doubt :
As you have removed adapters from docker-compose how will this thing work with mesheryctl as it uses the same docker-compose file and downloads it

…re/18

Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
…re/18

Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
@github-actions github-actions bot added the component/ui User Interface label Aug 10, 2023
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
@theBeginner86 theBeginner86 removed the pr/draft WIP/Draft pull request label Aug 10, 2023
Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
@theBeginner86
Copy link
Member Author

Doubt :
As you have removed adapters from docker-compose how will this thing work with mesheryctl as it uses the same docker-compose file and downloads it

You can make use of Adapter Tracker Interface. It does have the info for adapters as it's seeded on Meshery boot as env vars.
FYI: https://github.com/meshery/meshery/blob/master/server/cmd/main.go#L125

@theBeginner86
Copy link
Member Author

theBeginner86 commented Aug 10, 2023

This PR is ready.

System Flows:

  1. For Docker env, performing un/deploying would create new containers (or stop and delete running containers) of each adapter. Would ensure that the new adapters are on the same Docker network as the one for Meshery container
  2. For K8s env, performing un/deploying would internally use Helm with overrides for each adapter (enabled: true/false). Then on each un/deployment Helm upgrade is performed.
  3. For local env, its not supported and wouldn't perform any action.

@theBeginner86
Copy link
Member Author

The initial code written for this functionality is not clean would require some refactoring considering most of this functionality can be extracted into MeshKit. Like provisioning of containers or supporting multi-k8s deployment through our deployment modals.

Signed-off-by: Pranav Singh <pranavsingh02@hotmail.com>
@@ -1,7 +1,7 @@
version: "3"
services:
meshery:
image: layer5/meshery:latest
image: layer5/meshery:stable-latest
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like someone had changed this tag for meshery image. 🤔

As there is no image tag of latest for layer5/meshery image. https://hub.docker.com/r/layer5/meshery/tags?page=1&name=latest

@theBeginner86
Copy link
Member Author

theBeginner86 commented Aug 10, 2023

I have one more question.

Should we start publishing all Meshery Adapter's charts separately too?

So that users can directly do following for managing deployments of their individual adapters irrespective of Meshery deployment:
helm install meshery-istio meshery/meshery-istio

Instead of deploying them as follows:

helm pull meshery/meshery --untar
cd meshery/charts
helm install meshery-istio ./meshery-istio

The above would have no change in the current sub chart structure that we have its just that instead of referring a local charts dir we would be referring the remotely published charts.

The above would also be helpful when we would be deploying Meshery in K8s and trying to manage lifecycle of Adapters. As currently, since both Meshery and Adapters are part of the same release so during deployment/un-deployment of any adapter, the Meshery release is upgraded that leads to restart of Meshery pods. So, if Adapters charts are published separately then we can easily put them under separate release and that would not hinder Meshery pods.

@leecalcote
Copy link
Member

I'm not opposed to it, @theBeginner86.

@AugustasV @nebula-aac @Revolyssup thoughts?

Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
@leecalcote
Copy link
Member

Good error messages.

@leecalcote
Copy link
Member

Error handling of local builds that try to perform deployments works.

@leecalcote leecalcote merged commit b8c2113 into meshery:master Aug 11, 2023
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server component/ui User Interface framework/helm Issues related to helm-charts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants