Check more indicators for EKS discovery #2615
Conversation
pkg/controller/utils/discovery.go
Outdated
| return false, err | ||
| } | ||
| if cm != nil { |
There was a problem hiding this comment.
I see you're just copying this logic from the existing code so that's fine but it looks odd to me because presence of this configmap is indicated by err being IsNotFound. I don't think cm being nil is behavior that's meant to be relied on for anything.
Fine to merge as is or take a crack at refactoring, either is ok to me
| if err != nil { | ||
| if kerrors.IsNotFound(err) { | ||
| return false, nil | ||
| _, err := c.CoreV1().ConfigMaps("kube-system").Get(ctx, "aws-auth", metav1.GetOptions{}) |
There was a problem hiding this comment.
Would be nice for future readers to add some comments to this code explaining what we're checking. I'm not sure what "aws-auth" is, so it's hard to review whether or not this is an appropriate way to determine if we're running on EKS (e.g., as opposed to other AWS based Kubernetes providers).
|
I've put some comments in. Unfortunately eks-certificates-controller and the kube-dns label are not something I found any specific documentation about them. |
Description
We have found in testing that the provider is not being detected as EKS in setups that were detected as EKS before. Upon looking at our detection/discovery code and looking at some EKS clusters, there was and still isn't a great means of detecting an EKS cluster. This PR expands the detection a bit to include a couple more indicators.
For PR author
For PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.