Granulate gMaestro Addon #727

marina-vrublevsky · 2023-06-15T13:02:31Z

Issue #, if available:

Description of changes:
This PR continues #576

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

shapirov103 · 2023-06-16T14:53:44Z

@marina-vrublevsky a couple of questions:

Please confirm that I can close add granulate gmaestro addon #576 in favor of this one.
Please fix the branch conflicts, e.g. merge upstream main back into your fork.

elamaran11 · 2023-06-16T15:58:04Z

@marina-vrublevsky Also please fix the Pattern aws-samples/cdk-eks-blueprints-patterns#57 for Granulate.

marina-vrublevsky · 2023-06-18T14:27:17Z

Hi @elamaran11

Please confirm that I can close add granulate gmaestro addon #576 in favor of this one.

Yes

Please fix the branch conflicts, e.g. merge upstream main back into your fork.

Done

Also please fix the Pattern aws-samples/cdk-eks-blueprints-patterns#57 for Granulate.

On it

marina-vrublevsky · 2023-06-18T17:43:47Z

@marina-vrublevsky Also please fix the Pattern aws-samples/cdk-eks-blueprints-patterns#57 for Granulate.

@elamaran11 please review aws-samples/cdk-eks-blueprints-patterns#101

elamaran11 · 2023-06-19T14:58:56Z

@marina-vrublevsky Also please fix the Pattern aws-samples/cdk-eks-blueprints-patterns#57 for Granulate.

@elamaran11 please review aws-samples/cdk-eks-blueprints-patterns#101

@marina-vrublevsky Have few comments on the Patterns PR, please address it. Once we have patterns PR merged, we can review this doc PR.

elamaran11 · 2023-07-06T00:26:26Z

@marina-vrublevsky Please provide an update on this PR and Patterns Repo PR.

elamaran11 · 2023-08-04T18:55:36Z

@marina-vrublevsky Please provide an update on this PR and Patterns Repo PR.

freschri · 2023-08-31T07:42:40Z

@marina-vrublevsky I am picking this review up from Ela. I don't see a lib/addons/gmaestro/index.ts file, is there a reason for that? I see this PR is open for a while. Happy to support to get to completion. Please respond thanks.

freschri · 2023-09-03T13:29:13Z

docs/addons/gmaestro.md

+          --description "Encrypted client ID for Granulate gMaestro" \
+          --secret-string "${MAESTRO_CLIENT_ID}"
+      ```
+


I would replace the above with:
3. Create a secret (as a plaintext) in AWS Secrets Manager:
bash export MAESTRO_CLIENT_ID=<MAESTRO_CLIENT_ID value from the Deployment section in the downloaded config file> export MAESTRO_SECRET_NAME=<your preferred secret name> aws secretsmanager create-secret --name ${MAESTRO_SECRET_NAME} \ --description "Encrypted client ID for Granulate gMaestro" \ --secret-string "${MAESTRO_CLIENT_ID}"

freschri · 2023-09-03T13:32:53Z

docs/addons/gmaestro.md

+const app = new cdk.App();
+
+const addOn = new GmaestroAddOn({
+        clientIdSecretName: "<secret name>", // MAESTRO_SECRET_NAME


is clientIdSecretName a good parameter name? should it be more appropriate secretName? It is a bit confusing seems a merge between MAESTRO_SECRET_NAME and MAESTRO_CLIENT_ID

freschri

approving following discussions and amendments on other channel

elamaran11

@marina-vrublevsky Nice work, couple of minor changes from documentation perspective. Also fix the GH action errors, i see missing Doc links.

elamaran11 · 2023-09-05T13:26:53Z

docs/addons/gmaestro.md

+## Prerequisites
+Before using gMaestro, you need to:
+1. [Sign up](https://app.granulate.io/gMaestroSignup) to the gMaestro platform
+2. Download a sample YAML file - After signing up to gMaestro, navigate to the [Deploy](https://app.granulate.io/deploy) on the left-hand menu, fill in the required fields and click on "Generate Config File" 


Do we have a place to download the sample YAML. If not i would add the sample YAML to this doc page.

The only way to download it is using the instruction, I would rather not add it to the doc since it's being maintained by us and might change frequently

I would atleast hyperlink from where to download the sample, when you say Download a sample YAML file

@elamaran11 I went through the signup process, I think the doc here is clear: there is an image right below line 9 here that shows where to tap to download the file

Oh it wasn't clear initially, it makes sense. So you mean download as shown in below image. I would rather say Download a sample YAML file from step 3 - Download Config File as shown below. But up to you. Im good.

i can't provide a direct hyper link since it's auto generated after you follow the instructions in: https://app.granulate.io/deploy

I think im good on this comment. All i was recommending at last is rephrasing your sentence to say Download a sample YAML file from step 3 - Download Config File as shown below and hyperlink is not needed.

elamaran11 · 2023-09-05T13:29:16Z

docs/addons/gmaestro.md

@@ -0,0 +1,67 @@
+# gMaestro add-on for Amazon EKS Blueprints
+
+The gMaestro Blueprints AddOn deploys the gMaestro Agent on Amazon EKS using the [eks-blueprints](https://github.com/aws-quickstart/cdk-eks-blueprints) [CDK](https://aws.amazon.com/cdk/) construct.


I would rather remove this - using the eks-blueprints CDK construct.
and rephrase the sentence better to say what the addon actually does apart from deploying the agent on Amazon EKS. You need one or two sentence to explain what the addon does?

elamaran11

@marina-vrublevsky Couple of minor feedback almost there. Nice work~

elamaran11 · 2023-09-05T14:23:58Z

mkdocs.yml

@@ -79,6 +79,7 @@ nav:
    - Vpc Cni: 'addons/vpc-cni.md'
    - AWS XRay: 'addons/xray.md'
    - AWS XRay ADOT: 'addons/xray-adot-addon.md'
+    - Gmaestro: 'addons/gmaestro.md'


Move this to after line 48.

docs/addons/gmaestro.md

marina-vrublevsky · 2023-09-05T15:12:33Z

@elamaran11 I believe I addressed all your comments, can you please re-review both PRs?

elamaran11

LGTM

elamaran11 · 2023-09-05T15:39:25Z

@marina-vrublevsky Please see the above GH failures and fix those. Did you pull the latest code, some changes in main should be missed.

marina-vrublevsky · 2023-09-05T16:44:47Z

@elamaran11 I'm merged, looks like the error is unrelated to me ERROR: 1 dead links found! from another pattern's readme

elamaran11 · 2023-09-05T17:00:23Z

@marina-vrublevsky Totally agree its not your issue, rather than we fixing it your pulling it again from main. Can you replace this file https://karpenter.sh/v0.26/concepts/node-templates/#specsubnetselector to https://karpenter.sh/v0.28/concepts/node-templates/#specsubnetselector in the file docs/addons/karpenter.md. Once thats done, this error should go away.

elamaran11

LGTM

elamaran11 · 2023-09-05T18:39:08Z

Waiting on Patterns Repo PR to merge this.

marina-vrublevsky requested review from aws-ia-ci, a team, parkand1, shapirov103, youngjeong46 and elamaran11 as code owners June 15, 2023 13:02

marina-vrublevsky mentioned this pull request Jun 15, 2023

add granulate gmaestro addon #576

Closed

marina-vrublevsky added 3 commits August 31, 2023 19:25

Granulate gMaestro addon

0122593

fix import

ae94871

updated readme

e057297

marina-vrublevsky force-pushed the feature/granulate_gmaestro_addon branch from 82c8336 to e057297 Compare August 31, 2023 16:34

marina-vrublevsky added 3 commits September 3, 2023 10:58

updated readme

b71c96e

updated readme

0df9c85

updated readme

c6660df

freschri reviewed Sep 3, 2023

View reviewed changes

marina-vrublevsky added 5 commits September 3, 2023 17:00

CR fixes

276c51e

remove space

14fe722

CR fixes

d1917b9

CR fixes

85de1d7

CR fixes

2de5f3e

freschri self-assigned this Sep 4, 2023

freschri previously approved these changes Sep 4, 2023

View reviewed changes

elamaran11 requested changes Sep 5, 2023

View reviewed changes

CR fixes

38b9b65

marina-vrublevsky dismissed freschri’s stale review via 38b9b65 September 5, 2023 14:15

CR fixes

b532ac4

marina-vrublevsky requested a review from elamaran11 September 5, 2023 14:20

elamaran11 requested changes Sep 5, 2023

View reviewed changes

CR fixes

ee74189

marina-vrublevsky requested a review from elamaran11 September 5, 2023 14:43

elamaran11 previously approved these changes Sep 5, 2023

View reviewed changes

unrelated fix

f9c1915

marina-vrublevsky dismissed elamaran11’s stale review via f9c1915 September 5, 2023 17:19

marina-vrublevsky requested a review from elamaran11 September 5, 2023 17:21

elamaran11 approved these changes Sep 5, 2023

View reviewed changes

elamaran11 merged commit b488dea into aws-quickstart:main Sep 5, 2023
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Granulate gMaestro Addon #727

Granulate gMaestro Addon #727

marina-vrublevsky commented Jun 15, 2023

shapirov103 commented Jun 16, 2023

elamaran11 commented Jun 16, 2023

marina-vrublevsky commented Jun 18, 2023

marina-vrublevsky commented Jun 18, 2023 •

edited

Loading

elamaran11 commented Jun 19, 2023

elamaran11 commented Jul 6, 2023

elamaran11 commented Aug 4, 2023

freschri commented Aug 31, 2023

freschri Sep 3, 2023

freschri Sep 3, 2023

freschri left a comment

elamaran11 left a comment •

edited

Loading

elamaran11 Sep 5, 2023

marina-vrublevsky Sep 5, 2023

elamaran11 Sep 5, 2023

freschri Sep 5, 2023

elamaran11 Sep 5, 2023

marina-vrublevsky Sep 5, 2023 •

edited

Loading

elamaran11 Sep 5, 2023

elamaran11 Sep 5, 2023

elamaran11 left a comment

elamaran11 Sep 5, 2023

marina-vrublevsky commented Sep 5, 2023

elamaran11 left a comment

elamaran11 commented Sep 5, 2023 •

edited

Loading

marina-vrublevsky commented Sep 5, 2023

elamaran11 commented Sep 5, 2023

elamaran11 left a comment

elamaran11 commented Sep 5, 2023

		@@ -0,0 +1,67 @@
		# gMaestro add-on for Amazon EKS Blueprints

		The gMaestro Blueprints AddOn deploys the gMaestro Agent on Amazon EKS using the [eks-blueprints](https://github.com/aws-quickstart/cdk-eks-blueprints) [CDK](https://aws.amazon.com/cdk/) construct.

Granulate gMaestro Addon #727

Granulate gMaestro Addon #727

Conversation

marina-vrublevsky commented Jun 15, 2023

shapirov103 commented Jun 16, 2023

elamaran11 commented Jun 16, 2023

marina-vrublevsky commented Jun 18, 2023

marina-vrublevsky commented Jun 18, 2023 • edited Loading

elamaran11 commented Jun 19, 2023

elamaran11 commented Jul 6, 2023

elamaran11 commented Aug 4, 2023

freschri commented Aug 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freschri left a comment

Choose a reason for hiding this comment

elamaran11 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marina-vrublevsky Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elamaran11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marina-vrublevsky commented Sep 5, 2023

elamaran11 left a comment

Choose a reason for hiding this comment

elamaran11 commented Sep 5, 2023 • edited Loading

marina-vrublevsky commented Sep 5, 2023

elamaran11 commented Sep 5, 2023

elamaran11 left a comment

Choose a reason for hiding this comment

elamaran11 commented Sep 5, 2023

marina-vrublevsky commented Jun 18, 2023 •

edited

Loading

elamaran11 left a comment •

edited

Loading

marina-vrublevsky Sep 5, 2023 •

edited

Loading

elamaran11 commented Sep 5, 2023 •

edited

Loading