type fixes and upgrade eks to 3.0.1 #180
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| node_modules/ | ||
| bun.lockb |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,24 +8,20 @@ const tags = { "Project": "pulumi-k8s-aws-cluster", "Owner": "pulumi"}; | |
|
|
||
| ///////////////////// | ||
| // --- EKS Cluster --- | ||
|
|
||
| // Create an EKS cluster. | ||
| const cluster = new eks.Cluster(`${baseName}`, { | ||
| name: config.clusterName, | ||
| authenticationMode: "API", | ||
| // We keep these serviceRole and instanceRole properties to prevent the EKS provider from creating its own roles. | ||
| serviceRole: config.eksServiceRole, | ||
| instanceRole: config.eksInstanceRole, | ||
| vpcId: config.vpcId, | ||
| publicSubnetIds: config.publicSubnetIds, | ||
| privateSubnetIds: config.privateSubnetIds, | ||
| providerCredentialOpts: { profileName: process.env.AWS_PROFILE}, | ||
| nodeAssociatePublicIpAddress: false, | ||
| skipDefaultNodeGroup: true, | ||
| version: config.clusterVersion, | ||
| createOidcProvider: false, | ||
| tags: tags, | ||
|
|
@@ -48,7 +44,9 @@ const cluster = new eks.Cluster(`${baseName}`, { | |
| export const kubeconfig = pulumi.secret(cluster.kubeconfig.apply(JSON.stringify)); | ||
| export const clusterName = cluster.core.cluster.name; | ||
| export const region = aws.config.region; | ||
| export const nodeSecurityGroupId = cluster.nodeSecurityGroupId; | ||
|
|
||
| // For RDS | ||
| export const nodeGroupInstanceType = config.pulumiNodeGroupInstanceType; | ||
|
|
||
| ///////////////////// | ||
|
|
@@ -57,23 +55,23 @@ const ssmParam = pulumi.output(aws.ssm.getParameter({ | |
| // https://docs.aws.amazon.com/eks/latest/userguide/retrieve-ami-id.html | ||
| name: `/aws/service/eks/optimized-ami/${config.clusterVersion}/amazon-linux-2/recommended`, | ||
| })) | ||
| const amiId = ssmParam.value.apply(s => <string>JSON.parse(s).image_id) | ||
|
|
||
| const instanceProfile = new aws.iam.InstanceProfile("ng-standard", {role: config.eksInstanceRoleName}) | ||
| // Create a standard node group. | ||
| const ngStandard = new eks.NodeGroupV2(`${baseName}-ng-standard`, { | ||
|
There was a problem hiding this comment. Do we need to use NodeGroupV2? There was a problem hiding this comment. It's definitely a breaking change... but NodeGroup is also listed as deprecated, so it seems like a good change. I think the most recent merge breaking the installer into microstacks is a much larger (but also positive) breaking change, so I would argue that this breaking change on top of it is not significant enough to need explanation - I doubt many people have set up self hosted in the last week or so since the microstacks were released, those would be be the only people who would need that explanation. If we do want to capture release notes, looks like the last release listed on the repo was in 2022... do you want to make a release of the repo where it is now, and then we could add release notes after we merge this? Or, we could merge this and then make a release, bundling the breaking changes together. There was a problem hiding this comment. The I'd highly recommend to migrate to There was a problem hiding this comment. If you already make a switch here, I'd recommend moving to managed node groups. They're much easier to operate. |
||
| cluster: cluster, | ||
| instanceProfile: instanceProfile, | ||
| nodeAssociatePublicIpAddress: false, | ||
| nodeSecurityGroupId: cluster.nodeSecurityGroupId, | ||
| clusterIngressRuleId: cluster.clusterIngressRuleId, | ||
| amiId: amiId, | ||
| instanceType: <aws.ec2.InstanceType>config.standardNodeGroupInstanceType, | ||
| desiredCapacity: config.standardNodeGroupDesiredCapacity, | ||
| minSize: config.standardNodeGroupMinSize, | ||
| maxSize: config.standardNodeGroupMaxSize, | ||
|
|
||
| labels: {"amiId": amiId}, | ||
| cloudFormationTags: clusterName.apply(clusterName => ({ | ||
| "k8s.io/cluster-autoscaler/enabled": "true", | ||
| [`k8s.io/cluster-autoscaler/${clusterName}`]: "true", | ||
|
|
@@ -84,20 +82,20 @@ const ngStandard = new eks.NodeGroup(`${baseName}-ng-standard`, { | |
| }); | ||
|
|
||
| // Create a standard node group tainted for use only by self-hosted pulumi. | ||
| const ngStandardPulumi = new eks.NodeGroupV2(`${baseName}-ng-standard-pulumi`, { | ||
|
There was a problem hiding this comment. NodeGroupV2 needed? |
||
| cluster: cluster, | ||
| instanceProfile: instanceProfile, | ||
| nodeAssociatePublicIpAddress: false, | ||
| nodeSecurityGroupId: cluster.nodeSecurityGroupId, | ||
| clusterIngressRuleId: cluster.clusterIngressRuleId, | ||
| amiId: amiId, | ||
|
|
||
| instanceType: <aws.ec2.InstanceType>config.pulumiNodeGroupInstanceType, | ||
| desiredCapacity: config.pulumiNodeGroupDesiredCapacity, | ||
| minSize: config.pulumiNodeGroupMinSize, | ||
| maxSize: config.pulumiNodeGroupMaxSize, | ||
|
|
||
| labels: {"amiId": amiId}, | ||
| taints: { "self-hosted-pulumi": { value: "true", effect: "NoSchedule"}}, | ||
| cloudFormationTags: clusterName.apply(clusterName => ({ | ||
| "k8s.io/cluster-autoscaler/enabled": "true", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We can't have any IAM resources outside of the 01-iam project. A bit driver for this refactor of the eks installer is to allow customers to bring their own IAM (and networking).
And 01-iam already sets up the instanceProfile and exports the name
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.
This is a consequence of upgrading to eks 3.0.1 - the latest version of eks is now a Multi Language Component, and MLC's have a bug pulumi/pulumi#17515 which prevent them from using resources provided by a
get(likeaws.iam.InstanceProfile.get) - and further, the argumentinstanceProfileon the NodeGroup (and also NodeGroupV2) is a scalarInstanceProfiletype rather than apulumi.Input<InstanceProfile>type so we can't create the InstanceProfile in the iam stack and export the whole resource like I did with the roles either (whose arguments on the Cluster have an Input type). As far as I can tell this is the only way to work around that bug right now - and good point, I forgot to remove the creation/export of the instance profile name from the iam stack - I'll do that now. Unfortunately I think as long as the bug is open, upgrading to eks 3.0.1 means we can't allow users to bring their own InstanceProfile.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.
Then we have to pin to an earlier version of the eks package.
We cannot create IAM resources outside of the 01-iam project.
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.
And we should open an issue against the eks package to allow providing a profile via get or other mechanism.
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.
.getis blocked on pulumi/pulumi#17515. Sadly this is nothing specific to EKS, but MLCs in general.What I can see as an even better alternative is allowing to pass those resources in by their IDs. There's no need to pass the whole resource, the id is all we need. Will create an issue in EKS for this.
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.
pulumi/pulumi-eks#1488
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.
I just checked and it turns out that the
instanceProfileinput was marked as plain for no good reason IMO: https://github.com/pulumi/pulumi-eks/blob/0bbae2f5d240b8881ab014d964cc1f884451bf32/provider/cmd/pulumi-gen-eks/main.go#L2282At least I couldn't find any part in the code base that would require it to be plain.
Created an issue to track it: pulumi/pulumi-eks#1489