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

Allow configurable resource requests and limits received by helm chart. #196

Merged
merged 21 commits into from
Jul 25, 2024

Conversation

musa-asad
Copy link
Contributor

@musa-asad musa-asad commented Jul 17, 2024

Description of changes:
Receive the new auto-instrumentation-config command argument and apply those values to resources for init containers to have resource limits.

Testing:

  1. Built the operator changes into an image stored in ECR.
  2. Made changes to the helm chart, here.
  3. Locally updated the helm-charts/charts/amazon-cloudwatch-observability/templates/operator-deployment.yaml to include the new image stored in ECR.
  4. Created an EKS cluster.
  5. Ran kubectl apply -f helm-charts/charts/amazon-cloudwatch-observability/crds to make sure the CRDs are up to date.
  6. Ran helm upgrade --install amazon-cloudwatch-observability helm-charts/charts/amazon-cloudwatch-observability --values helm-charts/charts/amazon-cloudwatch-observability/values.yaml --set clusterName=cloudwatchagent-operator --set region=us-west-2 --set manager.autoInstrumentationImage.java.resources.limits.cpu=200m --set manager.autoInstrumentationImage.java.resources.limits.memory=512Mi --set manager.autoInstrumentationImage.java.resources.requests.cpu=200m --set manager.autoInstrumentationImage.java.resources.requests.memory=51Mi using the updated helm chart with the local changes.

We can confirm this works since when we run kubectl edit deployment amazon-cloudwatch-observability-controller-manager, we can see the following:
Screenshot 2024-07-25 at 1 58 48 PM

  1. Ran kubectl apply -f amazon-cloudwatch-agent-operator/integration-tests/java/sample-deployment-java.yaml to deploy a sample application, which is already in the operator repo for testing.
  2. Ran kubectl get pods to see the name of the sample app.
  3. Ran kubectl describe pod <sample-app-pod> to see if the limits are what we configured.
Screenshot 2024-07-25 at 2 00 11 PM

The limits are correctly set.

Test cases:
helm upgrade --install amazon-cloudwatch-observability helm-charts/charts/amazon-cloudwatch-observability --values helm-charts/charts/amazon-cloudwatch-observability/values.yaml --set clusterName=cloudwatchagent-operator --set region=us-west-2 --set manager.autoInstrumentationImage.java.resources.limits.cpu=200m --set manager.autoInstrumentationImage.java.resources.limits.memory="" --set manager.autoInstrumentationImage.java.resources.requests.cpu=200m --set manager.autoInstrumentationImage.java.resources.requests.memory=51Mi
Screenshot 2024-07-25 at 2 00 57 PM
Screenshot 2024-07-25 at 2 01 47 PM

helm upgrade --install amazon-cloudwatch-observability helm-charts/charts/amazon-cloudwatch-observability --values helm-charts/charts/amazon-cloudwatch-observability/values.yaml --set clusterName=cloudwatchagent-operator --set region=us-west-2 --set manager.autoInstrumentationImage.java.resources.limits.cpu=test --set manager.autoInstrumentationImage.java.resources.limits.memory=512Mi --set manager.autoInstrumentationImage.java.resources.requests.cpu=200m --set manager.autoInstrumentationImage.java.resources.requests.memory=51Mi
Screenshot 2024-07-25 at 2 03 10 PM
Screenshot 2024-07-25 at 2 03 48 PM

Removed { to break the argument.
Screenshot 2024-07-25 at 2 04 40 PM
Screenshot 2024-07-25 at 2 05 39 PM

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

main.go Outdated Show resolved Hide resolved
pkg/instrumentation/defaultinstrumentation.go Outdated Show resolved Hide resolved
pkg/instrumentation/defaultinstrumentation.go Outdated Show resolved Hide resolved
@musa-asad musa-asad marked this pull request as draft July 18, 2024 14:11
@musa-asad musa-asad marked this pull request as ready for review July 23, 2024 18:09
@musa-asad musa-asad requested review from bjrara and lisguo July 24, 2024 13:47
pkg/instrumentation/defaultinstrumentation_test.go Outdated Show resolved Hide resolved
pkg/instrumentation/defaultinstrumentation.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@musa-asad musa-asad changed the title Allow configurable resource limits received by helm chart. Allow configurable resource requests and limits received by helm chart. Jul 24, 2024
@musa-asad musa-asad marked this pull request as draft July 24, 2024 21:11
@musa-asad musa-asad marked this pull request as ready for review July 25, 2024 18:05
lisguo
lisguo previously approved these changes Jul 25, 2024
main.go Outdated Show resolved Hide resolved
pkg/instrumentation/defaultinstrumentation.go Outdated Show resolved Hide resolved
lisguo
lisguo previously approved these changes Jul 25, 2024
main.go Outdated Show resolved Hide resolved
@musa-asad musa-asad merged commit 1882049 into aws:main Jul 25, 2024
6 checks passed
@musa-asad musa-asad deleted the init-resource branch August 16, 2024 20:52
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.

5 participants