Add mechanism to run benchmark tests against a given cluster endpoint #4541
Conversation
Divyaasm
requested review from
dblock,
peterzhuamazon,
bbarani,
gaiksaya,
rishabh6788,
zelinh,
jordarlu,
prudhvigodithi and
tianleh
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4541 +/- ##
==========================================
+ Coverage 92.17% 92.39% +0.22%
==========================================
Files 192 193 +1
Lines 6297 6314 +17
==========================================
+ Hits 5804 5834 +30
+ Misses 493 480 -13 ☔ View full report in Codecov by Sentry. |
Divyaasm
changed the title
Signed-off-by: Divya Madala <[email protected]>
| load_balancer_url = cdk_output[self.stack_name].get('loadbalancerurl', None) | ||
| if load_balancer_url is None: | ||
| raise RuntimeError("Unable to fetch the cluster endpoint from cdk output") | ||
| self.args.cluster_endpoint = load_balancer_url |
There was a problem hiding this comment.
Why are we overwriting the args.cluster_endpoint?
Need better way to handle this.
There was a problem hiding this comment.
Assuming cluster_endpoint is provided in self.args.cluster_endpoint, In cases where manifest and distribution _url are provided this is empty. I will remove the above line.
self.cluster_endpoint_with_port should do the work for us
There was a problem hiding this comment.
override the self.cluster_endpoint class var here as mentioned in other comments.
| ) | ||
| self.params = "".join(params_list) + role_params | ||
| self.is_endpoint_public = False | ||
| self.cluster_endpoint = None |
There was a problem hiding this comment.
declare self.cluster_endpoint = args.cluster_endpoint and then override this class var in BenchmarkCreateCluster class?
|
|
||
| @property | ||
| def endpoint(self) -> str: | ||
| return self.cluster_endpoint | ||
| return self.args.cluster_endpoint |
There was a problem hiding this comment.
return self.cluster_endpoint as per above comment.
| raise RuntimeError("Unable to fetch the cluster endpoint from cdk output") | ||
| self.cluster_endpoint = loadbalancer_url | ||
| self.cluster_endpoint_with_port = "".join([loadbalancer_url, ":", str(self.port)]) | ||
| self.cluster_endpoint_with_port = "".join([self.args.cluster_endpoint, ":", str(self.port)]) |
There was a problem hiding this comment.
This may cause an issue. We will have to explicitly state that the endpoint should be passed without http or https.
Verify if benchmark command accepts --target-url in the form of https://<endpoint>:443. If yes then ignore else we need to clarify it.
|
|
||
| def start(self) -> None: | ||
| command = f"npm install && cdk deploy \"*\" {self.params} --outputs-file {self.output_file}" | ||
| command = f"curl -X GET http://{self.args.cluster_endpoint}" if self.args.insecure else f"curl -X GET https://{self.args.cluster_endpoint} -u 'admin:{get_password('2.12.0')}' --insecure" |
There was a problem hiding this comment.
use the form https://<endpoint> -ku user:pass, remove --insecure flag.
Signed-off-by: Divya Madala <[email protected]>
|
LGTM! Thank you for contributing this change. |
Description
Issues Resolved
Adresses #4478
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.