fix(eks): neuron plugin not added with addNodegroupCapacity()

[gdamjan]

Previously, the neuron plugin is already added in the addAutoScalingGroupCapacity() method, when the instanceType was some of the INFERENTIA types.

Let's also add the plugin if at least one (some/any) of the instanceTypes in addNodegroupCapacity() is of the INFERENTIA types.

---

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

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[gdamjan]

Clarification Request

❌ Features must contain a change to a README file.

The neuron plugin was not mentioned previously in the aws-eks README file, so I didn't know where to document this semi-new behaviour. Should I add a note that the plugin is added when using both addNodegroupCapacity() and addAutoScalingGroupCapacity() ?

[lpizzinidev]

Thanks for the contribution 👍

I think that this is more a fix than a feature, can please you update the title following the guidelines?

Also, an integration test is needed.
You can use this as a template and check the documentation for its execution.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[lpizzinidev]

Thanks, looks good 👍
Can you please update the title to fix(eks): neuron plugin not added with addNodegroupCapacity?
It should describe the bug (not the solution).

[gdamjan]

Thanks, looks good 👍 Can you please update the title to fix(eks): neuron plugin not added with addNodegroupCapacity? It should describe the bug (not the solution).

should I change the commit itself too, or you'll squash merge with using the PR topic?

[lpizzinidev]

@gdamjan
No, it's fine like this, thanks for the update 👍
The reason is that fix and feature titles will appear on the release CHANGELOG, so they need to follow a certain format.

[scanlonp]

One nit.
Do you think it would be helpful to have information about neuron plugins in the readme? If so add it, if not, then I think the exemption is fine.
Other than that, I am good to approve.

[scanlonp]

Suggested change

aws-eks/test/integ.eks-inference-nodegroup.js

Do not think we need to update this file.

[gdamjan]

afaik, the integration tests were tying to run the .ts file in that case and failing. I can try again.

[gdamjan]

Do you think it would be helpful to have information about neuron plugins in the readme?

I would add something like this:

> **NOTE:** If you add instances with the inferentia (`inf1` or `inf2`) class the [neuron plugin](https://awsdocs-neuron.readthedocs-hosted.com/en/latest/containers/dlc-then-eks-devflow.html) will be automatically 
> installed in the kubernetes cluster.

just above the #### Node Groups with IPv6 Support line in the ### Managed node groups section.

what do you think?

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 0497951
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).