Skip to content

Commit

Permalink
Merge pull request #2085 from michaelbeaumont/cluster_key_name
Browse files Browse the repository at this point in the history
馃悰 AWSManagedMachinePool: Fix cloud provider key usage and nodegroup IAM role name
  • Loading branch information
k8s-ci-robot committed Nov 9, 2020
2 parents d3e9e7b + 1824003 commit b7294ce
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 11 deletions.
1 change: 1 addition & 0 deletions exp/controllers/awsmanagedmachinepool_controller.go
Expand Up @@ -143,6 +143,7 @@ func (r *AWSManagedMachinePoolReconciler) Reconcile(req ctrl.Request) (_ ctrl.Re
Logger: logger,
Client: r.Client,
ControllerName: "awsmanagedmachinepool",
Cluster: cluster,
ControlPlane: controlPlane,
MachinePool: machinePool,
ManagedMachinePool: awsPool,
Expand Down
12 changes: 10 additions & 2 deletions pkg/cloud/scope/managednodegroup.go
Expand Up @@ -40,6 +40,7 @@ import (
type ManagedMachinePoolScopeParams struct {
Client client.Client
Logger logr.Logger
Cluster *clusterv1.Cluster
ControlPlane *controlplanev1exp.AWSManagedControlPlane
ManagedMachinePool *infrav1exp.AWSManagedMachinePool
MachinePool *clusterv1exp.MachinePool
Expand Down Expand Up @@ -76,6 +77,7 @@ func NewManagedMachinePoolScope(params ManagedMachinePoolScopeParams) (*ManagedM
return &ManagedMachinePoolScope{
Logger: params.Logger,
Client: params.Client,
Cluster: params.Cluster,
ControlPlane: params.ControlPlane,
ManagedMachinePool: params.ManagedMachinePool,
MachinePool: params.MachinePool,
Expand All @@ -92,6 +94,7 @@ type ManagedMachinePoolScope struct {
Client client.Client
patchHelper *patch.Helper

Cluster *clusterv1.Cluster
ControlPlane *controlplanev1exp.AWSManagedControlPlane
ManagedMachinePool *infrav1exp.AWSManagedMachinePool
MachinePool *clusterv1exp.MachinePool
Expand All @@ -102,11 +105,16 @@ type ManagedMachinePoolScope struct {
enableIAM bool
}

// Name returns the machine pool name.
func (s *ManagedMachinePoolScope) Name() string {
// ManagedPoolName returns the managed machine pool name.
func (s *ManagedMachinePoolScope) ManagedPoolName() string {
return s.ManagedMachinePool.Name
}

// ClusterName returns the cluster name.
func (s *ManagedMachinePoolScope) ClusterName() string {
return s.Cluster.Name
}

// EnableIAM indicates that reconciliation should create IAM roles
func (s *ManagedMachinePoolScope) EnableIAM() bool {
return s.enableIAM
Expand Down
6 changes: 3 additions & 3 deletions pkg/cloud/services/eks/nodegroup.go
Expand Up @@ -152,7 +152,7 @@ func (s *NodegroupService) createNodegroup() (*eks.Nodegroup, error) {
return nil, err
}
managedPool := s.scope.ManagedMachinePool.Spec
tags := ngTags(s.scope.Name(), additionalTags)
tags := ngTags(s.scope.ClusterName(), additionalTags)

remoteAccess, err := s.remoteAccess()
if err != nil {
Expand Down Expand Up @@ -370,10 +370,10 @@ func (s *NodegroupService) reconcileNodegroup() error {
}
s.scope.Info("Created EKS nodegroup in AWS", "cluster-name", eksClusterName, "nodegroup-name", eksNodegroupName)
} else {
tagKey := infrav1.ClusterAWSCloudProviderTagKey(s.scope.Name())
tagKey := infrav1.ClusterAWSCloudProviderTagKey(s.scope.ClusterName())
ownedTag := ng.Tags[tagKey]
if ownedTag == nil {
return errors.Wrapf(err, "owner of %s mismatch: %s", eksNodegroupName, s.scope.Name())
return errors.Wrapf(err, "owner of %s mismatch: %s", eksNodegroupName, s.scope.ClusterName())
}
s.scope.V(2).Info("Found owned EKS nodegroup in AWS", "cluster-name", eksClusterName, "nodegroup-name", eksNodegroupName)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/cloud/services/eks/roles.go
Expand Up @@ -149,7 +149,7 @@ func (s *NodegroupService) reconcileNodegroupIAMRole() error {
roleName = infrav1exp.DefaultEKSNodegroupRole
} else {
s.scope.Info("no EKS nodegroup role specified, using role based on nodegroup name")
roleName = fmt.Sprintf("%s-nodegroup-iam-service-role", s.scope.Name())
roleName = fmt.Sprintf("%s-%s-nodegroup-iam-service-role", s.scope.KubernetesClusterName(), s.scope.NodegroupName())
}
s.scope.ManagedMachinePool.Spec.RoleName = roleName
}
Expand All @@ -165,20 +165,20 @@ func (s *NodegroupService) reconcileNodegroupIAMRole() error {
return ErrNodegroupRoleNotFound
}

role, err = s.CreateRole(s.scope.ManagedMachinePool.Spec.RoleName, s.scope.Name(), eksiam.NodegroupTrustRelationship(), s.scope.AdditionalTags())
role, err = s.CreateRole(s.scope.ManagedMachinePool.Spec.RoleName, s.scope.ClusterName(), eksiam.NodegroupTrustRelationship(), s.scope.AdditionalTags())
if err != nil {
record.Warnf(s.scope.ManagedMachinePool, "FailedIAMRoleCreation", "Failed to create nodegroup IAM role %q: %v", s.scope.RoleName(), err)
return err
}
record.Eventf(s.scope.ManagedMachinePool, "SucessfulIAMRoleCreation", "Created nodegroup IAM role %q", s.scope.RoleName())
}

if s.IsUnmanaged(role, s.scope.Name()) {
if s.IsUnmanaged(role, s.scope.ClusterName()) {
s.scope.V(2).Info("Skipping, EKS nodegroup role policy assignment as role is unamanged")
return nil
}

err = s.EnsureTagsAndPolicy(role, s.scope.Name(), eksiam.NodegroupTrustRelationship(), s.scope.AdditionalTags())
err = s.EnsureTagsAndPolicy(role, s.scope.ClusterName(), eksiam.NodegroupTrustRelationship(), s.scope.AdditionalTags())
if err != nil {
return errors.Wrapf(err, "error ensuring tags and policy document are set on node role")
}
Expand Down Expand Up @@ -226,7 +226,7 @@ func (s *NodegroupService) deleteNodegroupIAMRole() (reterr error) {
return errors.Wrap(err, "getting EKS nodegroup iam role")
}

if s.IsUnmanaged(role, s.scope.Name()) {
if s.IsUnmanaged(role, s.scope.ClusterName()) {
s.V(2).Info("Skipping, EKS Nodegroup iam role deletion as role is unamanged")
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/services/eks/tags.go
Expand Up @@ -68,7 +68,7 @@ func getTagUpdates(currentTags map[string]string, tags map[string]string) (untag
}

func (s *NodegroupService) reconcileTags(ng *eks.Nodegroup) error {
tags := ngTags(s.scope.Name(), s.scope.AdditionalTags())
tags := ngTags(s.scope.ClusterName(), s.scope.AdditionalTags())

untagKeys, newTags := getTagUpdates(aws.StringValueMap(ng.Tags), tags)

Expand Down

0 comments on commit b7294ce

Please sign in to comment.