IAM authenticator only on workload support #3594
Conversation
eks-distro-bot
added
the
size/M
Codecov Report
@@ Coverage Diff @@
## main #3594 +/- ##
==========================================
+ Coverage 65.61% 66.05% +0.43%
==========================================
Files 358 359 +1
Lines 29118 29278 +160
==========================================
+ Hits 19107 19340 +233
+ Misses 8698 8627 -71
+ Partials 1313 1311 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
eks-distro-bot
added
size/L
| if commandContext.ClusterSpec.AWSIamConfig != nil { | ||
| logger.Info("Creating aws-iam-authenticator certificate and key pair secret on bootstrap cluster") | ||
| if err := commandContext.ClusterManager.CreateAwsIamAuthCaSecret(ctx, commandContext.BootstrapCluster, commandContext.ClusterSpec.Cluster.Name); err != nil { | ||
| commandContext.SetError(err) |
There was a problem hiding this comment.
are you able to cover these via a test?
|
/approve |
| if commandContext.ClusterSpec.AWSIamConfig != nil { | ||
| logger.Info("Creating aws-iam-authenticator certificate and key pair secret on bootstrap cluster") | ||
| if err := commandContext.ClusterManager.CreateAwsIamAuthCaSecret(ctx, commandContext.BootstrapCluster, commandContext.ClusterSpec.Cluster.Name); err != nil { | ||
| commandContext.SetError(err) | ||
| return &CollectMgmtClusterDiagnosticsTask{} | ||
| } |
There was a problem hiding this comment.
This is being called here as well as on line 141 below. Can you move this before if commandContext.BootstrapCluster != nil { check on line 101 and remove it on line 141?
There was a problem hiding this comment.
Actually I think that might not work? The call on line 141 actually uses the newly created KinD bootstrap cluster, and that reference is only available on 120. That is why I had to repeat the calls.
So basically the secret was only used when writing the cert files, so technically even though we referenced the same secret before, each cluster had separate files on their CP nodes host. |
|
/lgtm |
|
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavmpandey08, ptrivedi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/override eks-anywhere-e2e-presubmit |
|
@pokearu: Overrode contexts on behalf of pokearu: eks-anywhere-e2e-presubmit, eks-anywhere-mocks-presubmits In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Issue #2814
Description of changes:
Changes enable setting up
aws-iam-authenticatoron a workload cluster when its not configured on the management cluster.We now prepend cluster name to the
aws-iam-authenticator-cathat is needed on the bootstrap/management cluster before adding it on a workload cluster.Testing:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.