Make EKS fetcher to lazy initialize AWS EKS client #35077
Conversation
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
AntonAM
force-pushed
the
anton/eks-fetcher-error-skipping
branch
from
3607e72
to
f22a9ef
Compare
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
AntonAM
force-pushed
the
anton/eks-fetcher-error-skipping
branch
from
f22a9ef
to
4da6fbc
Compare
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
AntonAM
force-pushed
the
anton/eks-fetcher-error-skipping
branch
from
4da6fbc
to
f286327
Compare
| s.Log.WithError(err).Warnf("Could not initialize EKS fetcher(Region=%q, Labels=%q, AssumeRole=%q), skipping.", region, matcher.Tags, matcherAssumeRole.RoleARN) | ||
| continue |
There was a problem hiding this comment.
In this way, the issue where the Teleport service is unable to start will be mitigated, though the UX won't be perfect.
The EKS discovery with missing IAM permissions will be skipped, so if the missing IAM permission is updated, the discovery service requires a restart to apply this change.
Can do "lazy evaluation" when the EKS feature is created so we will allow to crate a EKS fetcher without full IAM permissions but the evolution will be done in the factual API call ?
There was a problem hiding this comment.
I've added lazy initialization for the EKS client, so now it will be retrying automatically on every Get.
AntonAM
force-pushed
the
anton/eks-fetcher-error-skipping
branch
from
969dbba
to
579d000
Compare
AntonAM
force-pushed
the
anton/eks-fetcher-error-skipping
branch
from
579d000
to
98c73f1
Compare
|
@fspmarshall @rudream friendly ping |
lib/cloud/mocks/aws.go
Outdated
| URL *url.URL | ||
| ARN string | ||
| URL *url.URL | ||
| AssumeRoleErrors map[string]error |
There was a problem hiding this comment.
Is AssumeRoleErrors an accidental leftover from an earlier iteration of this work? I can't see anywhere that errors are ever actually added to this mapping outside of tests.
There was a problem hiding this comment.
Yep, we don't need it anymore after I changed EKS fetcher to lazy initialization. Removed.
AntonAM
changed the title
If one of EKS fetchers initialization encountered an error (for example non authorized to assume IAM role), it stopped the whole discovery process from starting, instead of just skipping this fetcher and logging the problem. This PR changes it so we log warning and continue initialization.
Changelog: Prevent EKS fetcher not having correct IAM permissions from stopping whole Discovery service start up
Fixes #34949