-
Notifications
You must be signed in to change notification settings - Fork 76
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
Use the new EKS bootstrap script #5
Conversation
Just what it says on the tin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
nodejs/eks/cluster.ts
Outdated
systemctl daemon-reload | ||
systemctl restart kubelet kube-proxy | ||
const clusterCaData = eksCluster.certificateAuthority.apply(ca => Buffer.from(ca.data).toString("base64")); | ||
const userdata = pulumi.all([awsRegion, eksCluster.name, eksCluster.endpoint, clusterCaData]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor - but looks like we no longer need region
? (and similarly the line above that get
s it?)
BTW - Any reason we aren't running integration tests on PRs (or merges to master)? Seems it would be useful to run that extra validation. |
- Add the ability to attach a public key to EC2 worker nodes. - Attach a root volume to worker nodes with a configurable size that defaults to 20GiB. - Incorporate parent names in all child resources. - Add an ingress rule that accommodates extension API servers. - Use CloudFormation to set up the ASG in order to get rolling updates on launch configuration changes. - Fix the type token for `ServiceRole`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
nodejs/eks/cluster.ts
Outdated
instanceLaunchConfiguration.id, | ||
pulumi.output(args.desiredCapacity || 2), | ||
pulumi.output(args.minSize || 1), | ||
pulumi.output(args.maxSize || 2), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor - but pretty sure the pulumi.output
is not needed here.
nodejs/eks/transform.ts
Outdated
} | ||
} | ||
|
||
export default function transform<T, U>(name: string, input: T, func: (v: T) => U, opts?: pulumi.CustomResourceOptions): pulumi.Output<U> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to include this (and it seems overkill for the task) - let's add a comment.
Just what it says on the tin.