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 mesheryctl to deploy service meshes #25

Closed

Conversation

s1ntaxe770r
Copy link

Signed-off-by: Jubril Oyetunji [email protected]

Description
I added a bash script to check if mesheryctl is installed and deploy a service mesh
This PR fixes #
#20
Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Jubril Oyetunji <[email protected]>
@s1ntaxe770r s1ntaxe770r changed the title Use mesheryctl to deploy service meshes Use mesheryctl to deploy service meshes Nov 18, 2021
Copy link
Member

@Aisuko Aisuko left a comment

Choose a reason for hiding this comment

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

Thanks for contributing, please take a look the comment below.

if mesheryctl

then
echo found mesheryctl, deploying $MESH
Copy link
Member

Choose a reason for hiding this comment

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

Is there we need add "?


then
echo found mesheryctl, deploying $MESH
mesheryctl mesh deploy $mesh
Copy link
Member

Choose a reason for hiding this comment

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

$MESH?


install_mesheryctl(){
curl -L https://github.com/meshery/meshery/releases/download/v0.5.67/mesheryctl_0.5.67_Linux_x86_64.zip -o mesheryctl.zip
unzip -n mesheryctl.zip
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason we shouldn't overwrite the existing files?

@@ -0,0 +1,27 @@
MESH=$1

echo "checking if mesheryctl is installed"
Copy link
Member

Choose a reason for hiding this comment

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

Cheking would be good.

install_mesheryctl(){
curl -L https://github.com/meshery/meshery/releases/download/v0.5.67/mesheryctl_0.5.67_Linux_x86_64.zip -o mesheryctl.zip
unzip -n mesheryctl.zip
mv mesheryctl /usr/local/bin/mesheryctl
Copy link
Member

Choose a reason for hiding this comment

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

If I remember right, there would be a permission issue? Maybe there is other solution here?https://unix.stackexchange.com/questions/37724/permissions-ownership-of-usr-local-bin

Copy link
Author

Choose a reason for hiding this comment

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

🤔 I'm not sure, as far as I know, workflow runners usually run as a privileged user

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. We don't need to change this.

Copy link
Member

Choose a reason for hiding this comment

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

If you use the approved installation procedure, this is already considered for.

// @hexxdump

Signed-off-by: Jubril Oyetunji <[email protected]>
Signed-off-by: Jubril Oyetunji <[email protected]>
Signed-off-by: Jubril Oyetunji <[email protected]>


install_mesheryctl(){
curl -L https://github.com/meshery/meshery/releases/download/v0.5.67/mesheryctl_0.5.67_Linux_x86_64.zip -o mesheryctl.zip
Copy link
Member

Choose a reason for hiding this comment

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

Don't hardcode the version. Use the same command from the official docs: https://meshery.io. Any reason to deviate?

install_mesheryctl(){
curl -L https://github.com/meshery/meshery/releases/download/v0.5.67/mesheryctl_0.5.67_Linux_x86_64.zip -o mesheryctl.zip
unzip -n mesheryctl.zip
mv mesheryctl /usr/local/bin/mesheryctl
Copy link
Member

Choose a reason for hiding this comment

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

If you use the approved installation procedure, this is already considered for.

// @hexxdump

@pottekkat
Copy link
Contributor

@s1ntaxe770r Could you also add a check in the SMP action to see if Meshery is already installed? Skip install if the environment already has Meshery.

We should also onboard an application on the mesh once it is installed. We could leverage the mesheryctl-service-mesh-patterns-action for this I guess or we could use mesheryctl in the bash script directly as well.

// @sayantan1413 @Utkarsh-pro

Once we have that, we can switch the scripts used in the scheduled benchmarks workflow.

@s1ntaxe770r
Copy link
Author

@navendu-pottekkat is the action functional now? i haven't been following progress on that.
🤔 about checking if meshery is installed, I thought we were leaving that to the user ? I could be wrong on this

Signed-off-by: Jubril Oyetunji <[email protected]>
@leecalcote
Copy link
Member

@s1ntaxe770r tests run daily - https://github.com/layer5io/meshery-smp-action/actions

@s1ntaxe770r
Copy link
Author

I'll address the failing osm deployment in a separate PR

@s1ntaxe770r s1ntaxe770r force-pushed the feature/s1ntaxe770r/20 branch from 7fb82c3 to adbd3db Compare November 20, 2021 06:36
@pottekkat
Copy link
Contributor

@s1ntaxe770r The action is functional now. We should ideally check if Meshery is already deployed in the action as I mentioned above. And we should also include deploying the service mesh and an application on the service mesh in this script.

This script takes in arguments, which is good so we can just plug this script in the workflow to deploy a mesh and an application on top of the mesh.

@Revolyssup Can point you to a pattern file that does this. We can start with one mesh and then make progress.

@Revolyssup
Copy link
Contributor

@s1ntaxe770r The action is functional now. We should ideally check if Meshery is already deployed in the action as I mentioned above. And we should also include deploying the service mesh and an application on the service mesh in this script.

This script takes in arguments, which is good so we can just plug this script in the workflow to deploy a mesh and an application on top of the mesh.

@Revolyssup Can point you to a pattern file that does this. We can start with one mesh and then make progress.

To get a little context. a patternfile that does "what"? Installs a service mesh?

@pottekkat
Copy link
Contributor

@Revolyssup We will use mesheryctl mesh deploy to deploy the service mesh. We need a pattern file that say, onboards BookInfo onto Istio. We want to use mesheryctl for deploying a mesh and onboarding an application, so a pattern file that can onboard applications is what is needed here.

@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the issue/stale Issue has not had any activity for an extended period of time label Jan 9, 2022
@leecalcote
Copy link
Member

Gents, this has gone stale. What are the next steps here?

@stale stale bot removed the issue/stale Issue has not had any activity for an extended period of time label Jan 9, 2022
@s1ntaxe770r
Copy link
Author

@leecalcote i think we need to merge this into one review, i accidentally pushed a fix for #25 and #26 over here
#26

@leecalcote
Copy link
Member

So close this PR?

@leecalcote
Copy link
Member

This has become stale again, @s1ntaxe770r @navendu-pottekkat.

Should we toss in the towel?

@s1ntaxe770r
Copy link
Author

Nope, a fix is available in #26

@pottekkat
Copy link
Contributor

pottekkat commented Feb 5, 2022

We can close this and consolidate it to #26

// @s1ntaxe770r

@stale
Copy link

stale bot commented Mar 22, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the issue/stale Issue has not had any activity for an extended period of time label Mar 22, 2022
@leecalcote
Copy link
Member

@gyohuangxin will you review and decide which direction to go from here?

@stale stale bot removed the issue/stale Issue has not had any activity for an extended period of time label Mar 22, 2022
@gyohuangxin gyohuangxin mentioned this pull request Mar 23, 2022
1 task
@l5io
Copy link

l5io commented May 20, 2022

PR abandoned.

@l5io l5io closed this May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants