type fixes and upgrade eks to 3.0.1 #180
Conversation
…olate, comments out labels on nodegroups as needs string, not Output<string>
…ers into jdavredbeard/issues-103
…_ERROR_STRING_OUTPUT
…instanceProfile to eks stack to work around pulumi/pulumi#17515
jdavredbeard
changed the title
|
|
||
| const instanceProfile = new aws.iam.InstanceProfile("ng-standard", {role: config.eksInstanceRoleName}) |
There was a problem hiding this comment.
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.
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 (like aws.iam.InstanceProfile.get) - and further, the argument instanceProfile on the NodeGroup (and also NodeGroupV2) is a scalar InstanceProfile type rather than a pulumi.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.
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.
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.
.get is 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.
I just checked and it turns out that the instanceProfile input was marked as plain for no good reason IMO: https://github.com/pulumi/pulumi-eks/blob/0bbae2f5d240b8881ab014d964cc1f884451bf32/provider/cmd/pulumi-gen-eks/main.go#L2282
At 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
| // Create a standard node group. | ||
| const ngStandard = new eks.NodeGroup(`${baseName}-ng-standard`, { | ||
| const ngStandard = new eks.NodeGroupV2(`${baseName}-ng-standard`, { |
There was a problem hiding this comment.
Do we need to use NodeGroupV2?
This will cause the nodegroups being recreated and so if we do stay with NodeGroupV2, we need to capture the steps to explain and hopefully minimize the disruption.
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 NodeGroup component uses the deprecated AWS Launch Configuration (see AWS docs). Launch Configurations do not support new instance types released after December 31, 2022 and starting on October 1, 2024, new AWS accounts will not be able to create launch configurations.
I'd highly recommend to migrate to NodeGroupV2, otherwise you'll not be able to deploy this to new AWS accounts.
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.
| @@ -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.NodeGroup(`${baseName}-ng-standard-pulumi`, { | |||
| const ngStandardPulumi = new eks.NodeGroupV2(`${baseName}-ng-standard-pulumi`, { | |||
There was a problem hiding this comment.
NodeGroupV2 needed?
Remove unused
ssoRoleArnconfig.Fix type errors found while going through process of provisioning all the stacks.
Update deprecated NodeGroup resource to NodeGroupV2.
Complete #181 - Update pulumi/eks to 3.0.1, and in order to work around pulumi/pulumi#17515 , update iam stack to export the role resources as well as the names for the eks stack to consume, and move the InstanceProfile to the eks stack (bc of the above bug and bc the argument takes a scalar, not an Input).
Update the README for clarity and add some details to the stack README configs.
Lint README with markdownlint-cli.