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

Adds Remote Write changes for Prometheus #325

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

ashishranjan738
Copy link
Contributor

This commit adds changes to remote write Prometheus metrics to a AMP workspace.

Signed-off-by: Ashish Ranjan [email protected]

results:
- name: datapoint
description: Stores the CL2 result that can be consumed by other tasks (e.g. cloudwatch)
workspaces:
- name: source
mountPath: /src/k8s.io/
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just preferred this path as it coincides with GOPATH and also some hardcoding are there in CL2 code regarding this path https://github.com/kubernetes/perf-tests/blob/release-1.23/clusterloader2/pkg/prometheus/prometheus.go#L66. However if we are passing the manifests explicitly which indeed we are, we can avoid this. But this shouldn't be a breaking change.

tests/tasks/generators/clusterloader/load.yaml Outdated Show resolved Hide resolved
tests/tasks/setup/eks/awscli-mng.yaml Outdated Show resolved Hide resolved
@hakuna-matatah
Copy link
Contributor

could you add permissions/policies required to perform AMP operations here - https://github.com/awslabs/kubernetes-iteration-toolkit/blob/main/infrastructure/lib/kit-infrastructure.ts#L129-L148 so it works out of box without manual intervention during setup.

Also, could you add a readme to setup AMP as it's a pre-req for some of our tekton pipelines in our doc here

@hakuna-matatah
Copy link
Contributor

Very nice to see all this coming through. Thank you Ashish.

@ashishranjan738
Copy link
Contributor Author

ashishranjan738 commented Dec 1, 2022

could you add permissions/policies required to perform AMP operations here - https://github.com/awslabs/kubernetes-iteration-toolkit/blob/main/infrastructure/lib/kit-infrastructure.ts#L129-L148 so it works out of box without manual intervention during setup.

@hakuna-matatah how is adding to place gonna help us ? We need this permission to the role we pass in the mng task which i guess this is currently not generated via cdk

- name: host-cluster-node-role-arn

@hakuna-matatah
Copy link
Contributor

could you add permissions/policies required to perform AMP operations here - https://github.com/awslabs/kubernetes-iteration-toolkit/blob/main/infrastructure/lib/kit-infrastructure.ts#L129-L148 so it works out of box without manual intervention during setup.

@hakuna-matatah how is adding to place gonna help us ? We need this permission to the role we pass in the mng task which i guess this is currently not generated via cdk

- name: host-cluster-node-role-arn

No, tekton pods leverage IRSA hence the code link i shared in my comment will work for us if you add it there. The one you are pointing to is a node role which MNG APIs need as param for creating MNGs.

@hakuna-matatah
Copy link
Contributor

hakuna-matatah commented Dec 1, 2022

could you add permissions/policies required to perform AMP operations here - https://github.com/awslabs/kubernetes-iteration-toolkit/blob/main/infrastructure/lib/kit-infrastructure.ts#L129-L148 so it works out of box without manual intervention during setup.

@hakuna-matatah how is adding to place gonna help us ? We need this permission to the role we pass in the mng task which i guess this is currently not generated via cdk

- name: host-cluster-node-role-arn

> No, tekton pods leverage IRSA hence the code link i shared in my comment will work for us if you add it there. The one you are pointing to is a node role which MNG APIs need as param for creating MNGs.

Oops accidentally closed instead of adding comment.
Synced with Ashish on this. Here in this case, prom pod on guest cluster needs access, we are going to go with node role creation tekton step/task for this and wrap all policies required inside it for guest clusters.

@hakuna-matatah hakuna-matatah reopened this Dec 1, 2022
ashishranjan738 added a commit to ashishranjan738/kubernetes-iteration-toolkit that referenced this pull request Dec 1, 2022
This commit addresses the comments of PR awslabs#325.

Signed-off-by: Ashish Ranjan <[email protected]>
This commit adds changes to remote write Prometheus metrics to a AMP
workspace.

Signed-off-by: Ashish Ranjan <[email protected]>
This commit addresses the comments of PR awslabs#325.

Signed-off-by: Ashish Ranjan <[email protected]>
hakuna-matatah
hakuna-matatah previously approved these changes Dec 2, 2022
Copy link
Contributor

@hakuna-matatah hakuna-matatah left a comment

Choose a reason for hiding this comment

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

LGTM!
/Do-not-merge

@mengqiy
Copy link
Contributor

mengqiy commented Dec 5, 2022

Please work with @hakuna-matatah to understand why he added /Do-not-merge in the comment above.

@hakuna-matatah hakuna-matatah merged commit a21a7a3 into awslabs:main Dec 5, 2022
ashishranjan738 added a commit to ashishranjan738/kubernetes-iteration-toolkit that referenced this pull request Dec 7, 2022
This commit refactors mng create task based  on comments provided on

Signed-off-by: Ashish Ranjan <[email protected]>
hakuna-matatah pushed a commit that referenced this pull request Dec 13, 2022
…ups for #325 (#327)

* Adds ASG with Prometheus taints for CL2 tests

This commit adds an extra dedicated ASG for scheduling Prometheus
monitoring pod.

Signed-off-by: Ashish Ranjan <[email protected]>

* Adds cluster name label for prometheus monitoring

This commit adds cluster name label patch for prometheus monitoring.

Signed-off-by: Ashish Ranjan <[email protected]>

* Adds node selector for Prometheus manifest

This commit adds node selector with monitoring asg to Prometheus
manifest so that it is always tied to correct node.

Signed-off-by: Ashish Ranjan <[email protected]>

* Moves cw-metrics to addons folder.

This commit moves cw-metrics file to addons to folder and corrects exit
code.

Signed-off-by: Ashish Ranjan <[email protected]>

* Adds prometheus label for s3 path

This commit adds prometheus label for s3 result path.

Signed-off-by: Ashish Ranjan <[email protected]>

* Updates load test to utilize tekton result for passing s3_path

This commit updates load test to utilize tekton result for passing
s3_path.

Signed-off-by: Ashish Ranjan <[email protected]>

* Refactor mng create task based on the comments on #325

This commit refactors mng create task based  on comments provided on

Signed-off-by: Ashish Ranjan <[email protected]>

* Adds tasks for cluster role creation

This commit adds tasks for cluster role creation.

Signed-off-by: Ashish Ranjan <[email protected]>

* Updates pipelines with role creation tasks

This commit updates pipelines with role creation tasks.

Signed-off-by: Ashish Ranjan <[email protected]>

* Refactors Role Creation Tasks

This commit refactors role creation tasks.

Signed-off-by: Ashish Ranjan <[email protected]>

* Updates role task name

This comit updates role task name.

Signed-off-by: Ashish Ranjan <[email protected]>

* Updates Roles arns to to Role Names

This commit updates logic to use Role Names instead of Role ARNs.

Signed-off-by: Ashish Ranjan <[email protected]>

* Removes default value for role names.

This commit removes default value for role names.

Signed-off-by: Ashish Ranjan <[email protected]>

* Addresses comments of PR #327

Signed-off-by: Ashish Ranjan <[email protected]>

Signed-off-by: Ashish Ranjan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants