feat: Update EFA device plugin to use the EKS charts repo #26
Conversation
| repository = try(var.aws_efa_k8s_device_plugin_helm_config["repository"], null) | ||
| chart = try(var.aws_efa_k8s_device_plugin_helm_config["chart"], "${path.module}/helm-charts/aws-efa-k8s-device-plugin") | ||
| version = try(var.aws_efa_k8s_device_plugin_helm_config["version"], "0.1.0") | ||
| repository = try(var.aws_efa_k8s_device_plugin_helm_config["repository"], "https://aws.github.io/eks-charts") |
There was a problem hiding this comment.
I like to use the upstream repo instead of our local one, have you validated that all the configs that we are using by default in the values.yaml file are used into the default chart values? I'm asking that because people could be relying in the default values. @vara-bonthu need you on that.
There was a problem hiding this comment.
Thats a good call, I haven't checked what those defaults are in the local copy. I'll double check those and make sure we aren't breaking anything.
We may have added tolerations that the upstream doesn't.
There was a problem hiding this comment.
Looking at a diff of the values:
- Upstream Drops
beta.kubernetes.io/instance-typefrom the label selector (Starting in v1.17, this label is deprecated in favor of node.kubernetes.io/instance-type.) - Upstream moves version from
v0.3.3tov0.4.4 - upstream has added a requests block to the values:
resources:
requests:
cpu: 10m
memory: 20Mi
- our Local copy has a toleration to run everywhere, upstream values has an empty toleration config.
tolerations:
- operator: Exists # DaemonSet is tolerant of any taints, regardless of the key or value of the taint.
- key: CriticalAddonsOnly
operator: Exists
I'll add that toleration to our addon config but I think the other changes are ok.
There was a problem hiding this comment.
I tried to follow the way the Nvidia plugin configures those values
alanty
changed the title
What does this PR do?
Updates aws-efa-k8s-device-plugin.tf to use the eks-charts repo (https://github.com/aws/eks-charts/tree/master/stable/aws-efa-k8s-device-plugin) instead of a local copy of the device plugin chart
Motivation
this should make it easier to update later and includes the latest updates and instance types in the nodeAffinity.
More
pre-commit run -awith this PRFor Moderators
Additional Notes