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

[aws-cloudwatch-metrics] add tolerations, nodeSelector, affinity #606

Merged
merged 4 commits into from
Dec 6, 2021
Merged

[aws-cloudwatch-metrics] add tolerations, nodeSelector, affinity #606

merged 4 commits into from
Dec 6, 2021

Conversation

chmoon93
Copy link
Contributor

@chmoon93 chmoon93 commented Sep 27, 2021

Issue

[aws-cloudwatch-metrics] add node affinity to the chart #427
currently, aws-cloudwatch-metrics chart do not support nodeSelector, tolerations, affinity.

Description of changes

added tolerations, affinity, nodeSelector options

Checklist

  • Added/modified documentation as required (such as the README.md for modified charts)
  • Incremented the chart version in Chart.yaml for the modified chart(s)
  • Manually tested. Describe what testing was done in the testing section below
  • Make sure the title of the PR is a good description that can go into the release notes

Testing

it tested by eks cluster

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@chmoon93 chmoon93 changed the title add tolerations, nodeSelector, affinity [aws-cloudwatch-metrics] add tolerations, nodeSelector, affinity Sep 30, 2021
@ib-ak
Copy link
Contributor

ib-ak commented Oct 4, 2021

@bwagner5 @jaypipes @kishorj Can you please review. Currently it is blocking us to use taints on EKS nodes.

@RafPe
Copy link

RafPe commented Oct 7, 2021

Any update on this ? It would help to have this in place for our EKS systems

@claudio-vellage
Copy link

@jaypipes @bwagner5 @kishorj Please review this, I need this too, in my case, all nodes have tolerations, so the daemonset is scaled to 0. This is just a small PR and should be easy to merge without any negative impact.

Copy link
Contributor

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

@chmoon93 sincere apologies for the long delay in reviewing and merging this. looks great to me.

@jaypipes jaypipes merged commit 440e1c9 into aws:master Dec 6, 2021
@claudio-vellage
Copy link

claudio-vellage commented Dec 6, 2021

@jaypipes thanks for your quick response, now I've tried to use it, but the CircleCI pipeline says:

/home/circleci/project/scripts/publish-charts.sh
/home/circleci/project/scripts/publish-charts.sh: line 11: CIRCLE_TAG: unbound variable
Not a valid semver release tag! Skip charts publish

Is this expected? If yes, when can I expect this to be released? I need to roll this out and if it's not in the repository, I will have to download the chart for now. A quick update when I can expect this to be published would be great, thanks in advance!

Edit: I've already figured, no tag has been set to v0.0.69, not sure if by accident or if the release schedule is fixed. I will download the chart for now and keep an eye on it.

@jaypipes
Copy link
Contributor

jaypipes commented Dec 6, 2021

@jaypipes thanks for your quick response, now I've tried to use it, but the CircleCI pipeline says:

/home/circleci/project/scripts/publish-charts.sh
/home/circleci/project/scripts/publish-charts.sh: line 11: CIRCLE_TAG: unbound variable
Not a valid semver release tag! Skip charts publish

Is this expected? If yes, when can I expect this to be released? I need to roll this out and if it's not in the repository, I will have to download the chart for now. A quick update when I can expect this to be published would be great, thanks in advance!

Edit: I've already figured, no tag has been set to v0.0.69, not sure if by accident or if the release schedule is fixed. I will download the chart for now and keep an eye on it.

Looks like there's some bug in our release automation... I will check with @bwagner5 and @jayanthvn and figure it out.

@claudio-vellage
Copy link

@jaypipes thanks again, right now I've got the agents running with the downloaded chart, works perfectly, will update as soon as a tag is pushed. I think above is not a bug, but expected behavior, see edit on my old comment.

Anyway, since this is already working, maybe you want to close the duplicate PRs here: #585 and #540

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