layer5io / meshery-performance-action

GitHub Action for pipelining microservices and Kubernetes performance testing with Meshery
https://layer5.io/projects/nighthawk
Apache License 2.0
29 stars 22 forks source link

Replace `kubectl apply` with `mesheryctl pattern apply` #56

Closed hershd23 closed 2 years ago

hershd23 commented 2 years ago

Description

This PR fixes #20

Notes for Reviewers

Most changes taken from @alphaX86 's work with minor changes

Signed commits

hershd23 commented 2 years ago

@gyohuangxin @leecalcote

hershd23 commented 2 years ago

Tests :- Istio + wkr2 - https://github.com/hershd23/meshery-smp-action/runs/6903840057?check_suite_focus=true Istio + fortio - https://github.com/hershd23/meshery-smp-action/runs/6886559987?check_suite_focus=true Linkerd + wrk2 - https://github.com/hershd23/meshery-smp-action/runs/6868235120?check_suite_focus=true Linkerd + fortio - https://github.com/hershd23/meshery-smp-action/runs/6903852355?check_suite_focus=true

hershd23 commented 2 years ago

All these tests seem to be working for the time being. Changing app onboard to pattern apply for Linkerd and testing for OSM as well

hershd23 commented 2 years ago

Linkerd erroring out while using pattern apply, but working perfectly with app onboard

runner@fv-az38-392:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl pattern apply -f "./emojivoto.yml"
Error: Response Status Code 400, possible Server Error
runner@fv-az38-392:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl app onboard -f "./emojivoto.yml"
app successfully onboarded

runner@fv-az38-392:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
NAME                                      READY   STATUS            RESTARTS   AGE
linkerd-destination-7f78f877c9-5vkm7      0/4     PodInitializing   0          30s
linkerd-identity-555666dfcb-49s5c         2/2     Running           0          30s
linkerd-proxy-injector-66456889fd-qmn86   0/2     PodInitializing   0          30s

I remember having a conversation sometime back where @piyushsingariya mentioned that pattern apply only accepts URLs and not path to files on local. Is that still the case?

hershd23 commented 2 years ago

There is a problem with the file we are using for OSM. The pattern file expects a name at the very beginning while this is written like separate k8s objects. @Revolyssup how can we construct a pattern file out of the sample application file linked below for OSM?

(Not a ) Pattern for OSM https://raw.githubusercontent.com/openservicemesh/osm-docs/main/manifests/apps/bookstore.yaml

Compare it with Istio pattern https://raw.githubusercontent.com/service-mesh-patterns/service-mesh-patterns/master/samples/bookInfoPattern.yaml

Edit :- Even emojivoto is the same format as osm bookstore. Looks like app onboard is more suitable for this case

hershd23 commented 2 years ago
runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
No resources found in default namespace.
runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl system login --provider None
Initiating login...
successfully authenticated
runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl mesh deploy adapter meshery-linkerd:10001
Verifying prerequisites...
✔ LINKERD
\runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl app onboard -f "./emojivoto.yml"
app successfully onboarded

runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
NAME                                      READY   STATUS            RESTARTS   AGE
linkerd-destination-d58474474-2qf6r       0/4     PodInitializing   0          10s
linkerd-identity-555666dfcb-qtpsw         0/2     PodInitializing   0          10s
linkerd-proxy-injector-6745db7474-44xl7   0/2     PodInitializing   0          10s

Now app onboard works... but not always the first time around.

https://github.com/hershd23/meshery-smp-action/runs/6904759436?check_suite_focus=true#step:4:70

In this flow, it gives a message that app onboarded successfully yet no pods can be seen when trying to debug. Pods show up when the same steps as before are done again. Now this behaviour doesn't happen always but it doesn't look like app onboard is reliable. @leecalcote any ideas?

hershd23 commented 2 years ago

Faced this same behaviour this time with OSM.

runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
No resources found in default namespace.
runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl mesh deploy adapter meshery-osm:10009
Verifying prerequisites...
✔ OPEN_SERVICE_MESH
runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl app onboard -f "./bookstore.yaml"
app successfully onboarded

runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
NAME                             READY   STATUS     RESTARTS   AGE
osm-bootstrap-79bb65f586-7bmxs   0/1     Running    0          14s
osm-controller-dd77575f4-75g64   0/1     Init:0/1   0          14s
osm-injector-5bb8cc9594-sk7nv    0/1     Init:0/1   0          14s
hershd23 commented 2 years ago

The good thing is that apart from being a little unreliable application creation is going through.

The funny thing is that though my tests it seems:- pattern apply only accepts url as input with the -f flag app onboard only accepts file path on local with the -f flag. Hence in this case the application file has to be downloaded first.

@leecalcote @jared Can we look into this behaviour?

gyohuangxin commented 2 years ago

@hershd23 Sorry for looking at this late. Can I confirm that, from your test results, pattern apply only works fine with Istio, but has issues with Linkerd or OSM? And app onboard isn't always reliable with Linkerd and OSM?

hershd23 commented 2 years ago

Yes, because emojivoto for linkerd and OSM demo aren't really pattern files.

https://raw.githubusercontent.com/service-mesh-patterns/service-mesh-patterns/master/samples/bookInfoPattern.yaml This is a pattern file, bookinfo

https://github.com/openservicemesh/osm-docs/blob/main/manifests/apps/bookstore.yaml This isn't, just a collection of k8s objects. This requires app onboard.

The pattern files requires a name field at the top, that is the error trace

hershd23 commented 2 years ago

For all other purposes this works for now, having pattern apply for istio and app onboard for the examples of the rest

gyohuangxin commented 2 years ago

@hershd23 Thanks for reporting this results.

Did you tried to apply the bookinfo pattern on Linkerd and OSM? I think it's worth a try because:

  1. In our plan, we need the unified pattern for all meshes, so that we can report credible performance tests results.
  2. We're supporting other meshes, they may not have own sample applications.

@leecalcote Do you have any suggestions or answers on whether we can apply the bookinfo pattern on every meshes.

leecalcote commented 2 years ago

Yes, Meshery's Adapters come with a few different sample applications - https://docs.meshery.io/guides/sample-apps. Ideally, we get a few of these worked into to the tests.

gyohuangxin commented 2 years ago

As this PR works fine, I think we can merge it. However, Istio is using pattern apply, Linkerd and OSM are still using app onboard. We will also meet these issues about which method to deploy our workloads when we add other meshes #52 . So, @hershd23 can you create another issue to discuss how to unify the method of deploying workloads?

hershd23 commented 2 years ago

Hi @gyohuangxin, recently @Revolyssup and @gr455 explained a way to convert kubernetes manifests into a pattern file.

I'll check if I am able to do that. If so I can also store it with the istio bookinfo pattern, not sure which repository that is but I'll find it and convert everything to pattern apply

gyohuangxin commented 2 years ago

@hershd23 Good, thanks. If it will take some time, we can merge this first. Then you raise another PR if you find it works.

hershd23 commented 2 years ago

Sure we can do that. I don't have the merge option with me so feel free to merge.

I'll work on the other bit in a separate PR