Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eks): kubectl resources fail before fargate profiles are created #8855

Merged
merged 1 commit into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/cluster-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class ClusterResource extends Construct {
}));

this.creationRole.addToPolicy(new iam.PolicyStatement({
actions: [ 'iam:GetRole' ],
actions: [ 'iam:GetRole', 'iam:listAttachedRolePolicies' ],
resources: [ '*' ],
}));

Expand Down
90 changes: 71 additions & 19 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as autoscaling from '@aws-cdk/aws-autoscaling';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as ssm from '@aws-cdk/aws-ssm';
import { CfnOutput, Construct, IResource, Resource, Stack, Tag, Token } from '@aws-cdk/core';
import { CfnOutput, CfnResource, Construct, IResource, Resource, Stack, Tag, Token } from '@aws-cdk/core';
import * as fs from 'fs';
import * as path from 'path';
import * as YAML from 'yaml';
Expand Down Expand Up @@ -392,6 +392,20 @@ export class Cluster extends Resource implements ICluster {

private readonly version: string | undefined;

/**
* A dummy CloudFormation resource that is used as a wait barrier which
* represents that the cluster is ready to receive "kubectl" commands.
*
* Specifically, all fargate profiles are automatically added as a dependency
* of this barrier, which means that it will only be "signaled" when all
* fargate profiles have been successfully created.
*
* When kubectl resources call `_attachKubectlResourceScope()`, this resource
* is added as their dependency which implies that they can only be deployed
* after the cluster is ready.
*/
private readonly _kubectlReadyBarrier?: CfnResource;

/**
* Initiates an EKS Cluster with the supplied arguments
*
Expand Down Expand Up @@ -448,6 +462,18 @@ export class Cluster extends Resource implements ICluster {
if (this.kubectlEnabled) {
resource = new ClusterResource(this, 'Resource', clusterProps);
this._clusterResource = resource;

// we use an SSM parameter as a barrier because it's free and fast.
this._kubectlReadyBarrier = new CfnResource(this, 'KubectlReadyBarrier', {
type: 'AWS::SSM::Parameter',
properties: {
Type: 'String',
Value: 'aws:cdk:eks:kubectl-ready',
},
});

// add the cluster resource itself as a dependency of the barrier
this._kubectlReadyBarrier.node.addDependency(this._clusterResource);
} else {
resource = new CfnCluster(this, 'Resource', clusterProps);
}
Expand Down Expand Up @@ -502,7 +528,7 @@ export class Cluster extends Resource implements ICluster {
}

if (this.kubectlEnabled) {
this.defineCoreDnsComputeType(props.coreDnsComputeType || CoreDnsComputeType.EC2);
this.defineCoreDnsComputeType(props.coreDnsComputeType ?? CoreDnsComputeType.EC2);
}
}

Expand Down Expand Up @@ -794,33 +820,59 @@ export class Cluster extends Resource implements ICluster {
}

/**
* Returns the custom resource provider for kubectl-related resources.
* Internal API used by `FargateProfile` to keep inventory of Fargate profiles associated with
* this cluster, for the sake of ensuring the profiles are created sequentially.
*
* @returns the list of FargateProfiles attached to this cluster, including the one just attached.
* @internal
*/
public get _kubectlProvider(): KubectlProvider {
if (!this._clusterResource) {
throw new Error('Unable to perform this operation since kubectl is not enabled for this cluster');
}

const uid = '@aws-cdk/aws-eks.KubectlProvider';
const provider = this.stack.node.tryFindChild(uid) as KubectlProvider || new KubectlProvider(this.stack, uid);
public _attachFargateProfile(fargateProfile: FargateProfile): FargateProfile[] {
this._fargateProfiles.push(fargateProfile);

// allow the kubectl provider to assume the cluster creation role.
this._clusterResource.addTrustedRole(provider.role);
// add all profiles as a dependency of the "kubectl-ready" barrier because all kubectl-
// resources can only be deployed after all fargate profiles are created.
if (this._kubectlReadyBarrier) {
this._kubectlReadyBarrier.node.addDependency(fargateProfile);
}

return provider;
return this._fargateProfiles;
}

/**
* Internal API used by `FargateProfile` to keep inventory of Fargate profiles associated with
* this cluster, for the sake of ensuring the profiles are created sequentially.
* Adds a resource scope that requires `kubectl` to this cluster and returns
* the `KubectlProvider` which is the custom resource provider that should be
* used as the resource provider.
*
* Called from `HelmResource` and `KubernetesResource`
*
* @property resourceScope the construct scope in which kubectl resources are defined.
*
* @returns the list of FargateProfiles attached to this cluster, including the one just attached.
* @internal
*/
public _attachFargateProfile(fargateProfile: FargateProfile): FargateProfile[] {
this._fargateProfiles.push(fargateProfile);
return this._fargateProfiles;
public _attachKubectlResourceScope(resourceScope: Construct): KubectlProvider {
const uid = '@aws-cdk/aws-eks.KubectlProvider';

if (!this._clusterResource) {
throw new Error('Unable to perform this operation since kubectl is not enabled for this cluster');
}

// singleton
let provider = this.stack.node.tryFindChild(uid) as KubectlProvider;
if (!provider) {
// create the provider.
provider = new KubectlProvider(this.stack, uid);

// allow the kubectl provider to assume the cluster creation role.
this._clusterResource.addTrustedRole(provider.role);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You moved this inside the singleton creation, but won't this mean that if two clusters are defined on the same stack, one of them wont have the trusted role?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, nice catch! Let me add a test and fix

}

if (!this._kubectlReadyBarrier) {
throw new Error('unexpected: kubectl enabled clusters should have a kubectl-ready barrier resource');
}

resourceScope.node.addDependency(this._kubectlReadyBarrier);

return provider;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/helm-chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class HelmChart extends Construct {

const stack = Stack.of(this);

const provider = props.cluster._kubectlProvider;
const provider = props.cluster._attachKubectlResourceScope(this);

const timeout = props.timeout?.toSeconds();
if (timeout && timeout > 900) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/k8s-patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class KubernetesPatch extends Construct {
super(scope, id);

const stack = Stack.of(this);
const provider = props.cluster._kubectlProvider;
const provider = props.cluster._attachKubectlResourceScope(this);

new CustomResource(this, 'Resource', {
serviceToken: provider.serviceToken,
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/k8s-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class KubernetesResource extends Construct {
super(scope, id);

const stack = Stack.of(this);
const provider = props.cluster._kubectlProvider;
const provider = props.cluster._attachKubectlResourceScope(this);

new CustomResource(this, 'Resource', {
serviceToken: provider.serviceToken,
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-eks/lib/kubectl-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class KubectlProvider extends NestedStack {
runtime: lambda.Runtime.PYTHON_3_7,
handler: 'index.handler',
timeout: Duration.minutes(15),
description: 'onEvent handler for EKS kubectl resource provider',
layers: [ KubectlLayer.getOrCreate(this, { version: '2.0.0' }) ],
memorySize: 256,
});
Expand Down
Loading