Add scalar types for most commonly used resource outputs #1445

flostadler · 2024-10-16T09:36:00Z

To ease the impact of the breaking API changes caused by generating the node SDK, we decided to add additional scalar inputs that simplify UX across all SDKs (for more details see internal doc).

This change adds the scalar properties mentioned in the doc and adds acceptance tests for them.
While adding the acceptance tests I noticed that running pods on Fargate doesn't work deterministically. In some cases the cluster fails to get healthy (coredns stuck in pending).
This was caused by a race-condition between coredns starting and the fargate profile being created. If the fargate profile deployed after coredns, the pods got stuck in pending because they got assigned to the default-scheduler instead of the fargate-scheduler.
The fix is relatively easy; making coredns depend on the fargate profile.

I'll separately update the migration guide.

New properties

Existing Resource		New Top Level Property	Description
`clusterSecurityGroup: Output<aws.ec2.SecurityGroup \| undefined>`		`clusterSecurityGroupId: Output<string>`	Only really useful property of a security group. Used to add additional ingress/egress rules. Default to `the EKS created security group id`
`nodeSecurityGroup: Output<aws.ec2.SecurityGroup \| undefined>`		`nodeSecurityGroupId: Output<string>`
`eksClusterIngressRule: Output<aws.ec2.SecurityGroupRule \| undefined>`		`clusterIngressRuleId: Output<string>`	Only really useful property of a rule. Default to `””`
`defaultNodeGroup: Output<eks.NodeGroupData \| undefined>`		`defaultNodeGroupAsgName: Output<string>`	The only useful property of the default node group is the auto scaling group. Exposing its name allows users to reference it in IAM roles, tags, etc. Default to `””`
`core`	`fargateProfile: Output<aws.eks.FargateProfile \| undefined>`	`fargateProfileId: Output<string>`	The id of the fargate profile. Can be used to reference it. Default to `””`
		`fargateProfileStatus: Output<string>`	The status of the fargate profile. Default to `””`
	`oidcProvider: Output<aws.iam.OpenIdConnectProvider \| undefined>`	`oidcProviderArn: Output<string>` & `oidcProviderUrl: Output<string>` & `oidcIssuer: Output<string`	Arn and Url are properties needed to set up IAM identities for pods (required for the assume role policy of the IAM role). Users currently need to trim the `https://` part of the url to actually use it. We should expose `oidcProvider` with that already done to ease usage.

Fixes #1041

flostadler · 2024-10-16T09:37:58Z

nodejs/eks/cluster.ts

+                {
+                    parent,
+                    provider,
+                    dependsOn: fargateProfile.apply((profile) => {


That's the fix for the Fargate issues. I needed to include it here otherwise I couldn't test the changes in this PR.

github-actions · 2024-10-16T09:38:27Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

flostadler · 2024-10-16T12:19:33Z

provider/cmd/pulumi-gen-eks/main.go

@@ -1103,6 +1145,7 @@ func generateSchema(version semver.Version) schema.PackageSpec {
 				},
 			},
 			"eks:index:VpcCniAddon": {
+				IsComponent: true,


Unrelated to this PR, but I noticed this was incorrectly set and we had .get functions for this component.

t0yv0 · 2024-10-16T13:35:34Z

provider/cmd/pulumi-resource-eks/schema.json

@@ -1077,10 +1077,18 @@
                    "$ref": "/aws/v6.18.2/schema.json#/provider",
                    "description": "The AWS resource provider."
                },
+                "clusterIngressRuleId": {


The additions here can be copied into PR description to make searching easier. Referring to an internal doc is not that helpful for posterity. Thanks!

Added it to the PR description

t0yv0 · 2024-10-16T13:36:26Z

sdk/dotnet/VpcCniAddon.cs

@@ -14,7 +14,7 @@ namespace Pulumi.Eks
    /// For more information see: https://docs.aws.amazon.com/eks/latest/userguide/eks-add-ons.html
    /// </summary>
    [EksResourceType("eks:index:VpcCniAddon")]
-    public partial class VpcCniAddon : global::Pulumi.CustomResource
+    public partial class VpcCniAddon : global::Pulumi.ComponentResource


Will this break things? Should we call it out? Or it is a new resource.

New resource, we added it as part of v3 but forgot to mark it as a component

t0yv0

LGTM

This change builds on top of #1445 and makes `NodeGroup` & `NodeGroupV2` accept the scalar security group properties introduced in that PR. This way users can connect their node groups to the cluster without having to use any applies.

#1445 and #1446 introduced new scalar properties as a workaround to the breaking Node.js SDK changes. This documents those in the migration guide.

To ease the impact of the breaking API changes caused by generating the node SDK, we decided to add additional scalar inputs that simplify UX across all SDKs (for more details [see internal doc](https://docs.google.com/document/d/1f97nmDUG_nrZSllYxu_XSeI7ON8vhZzfVrdBTQQmZzw/edit#heading=h.fbweiu8gc5bw)). This change adds the scalar properties mentioned in the doc and adds acceptance tests for them. While adding the acceptance tests I noticed that running pods on Fargate doesn't work deterministically. In some cases the cluster fails to get healthy (coredns stuck in pending). This was caused by a race-condition between coredns starting and the fargate profile being created. If the fargate profile deployed after coredns, the pods got stuck in pending because they got assigned to the `default-scheduler` instead of the `fargate-scheduler`. The fix is relatively easy; making coredns depend on the fargate profile. I'll separately update the migration guide. ### New properties | Existing Resource | | New Top Level Property | Description | | :---- | :---- | :---- | :---- | | `clusterSecurityGroup: Output<aws.ec2.SecurityGroup \| undefined>` | | `clusterSecurityGroupId: Output<string>` | Only really useful property of a security group. Used to add additional ingress/egress rules. Default to `the EKS created security group id` | | `nodeSecurityGroup: Output<aws.ec2.SecurityGroup \| undefined>` | | `nodeSecurityGroupId: Output<string>` | | | `eksClusterIngressRule: Output<aws.ec2.SecurityGroupRule \| undefined>` | | `clusterIngressRuleId: Output<string>` | Only really useful property of a rule. Default to `””` | | `defaultNodeGroup: Output<eks.NodeGroupData \| undefined>` | | `defaultNodeGroupAsgName: Output<string>` | The only useful property of the default node group is the auto scaling group. Exposing its name allows users to reference it in IAM roles, tags, etc. Default to `””` | | `core` | `fargateProfile: Output<aws.eks.FargateProfile \| undefined>` | `fargateProfileId: Output<string>` | The id of the fargate profile. Can be used to reference it. Default to `””` | | | | `fargateProfileStatus: Output<string>` | The status of the fargate profile. Default to `””` | | | `oidcProvider: Output<aws.iam.OpenIdConnectProvider \| undefined>` | `oidcProviderArn: Output<string>` & `oidcProviderUrl: Output<string>` & `oidcIssuer: Output<string` | Arn and Url are properties needed to set up IAM identities for pods (required for the assume role policy of the IAM role). Users currently need to trim the `https://` part of the url to actually use it. We should expose `oidcProvider` with that already done to ease usage. | Fixes #1041

This change builds on top of #1445 and makes `NodeGroup` & `NodeGroupV2` accept the scalar security group properties introduced in that PR. This way users can connect their node groups to the cluster without having to use any applies.

#1445 and #1446 introduced new scalar properties as a workaround to the breaking Node.js SDK changes. This documents those in the migration guide.

pulumi-bot · 2024-10-17T16:15:13Z

This PR has been shipped in release v3.0.0-beta.2.

To ease the impact of the breaking API changes caused by generating the node SDK, we decided to add additional scalar inputs that simplify UX across all SDKs (for more details [see internal doc](https://docs.google.com/document/d/1f97nmDUG_nrZSllYxu_XSeI7ON8vhZzfVrdBTQQmZzw/edit#heading=h.fbweiu8gc5bw)). This change adds the scalar properties mentioned in the doc and adds acceptance tests for them. While adding the acceptance tests I noticed that running pods on Fargate doesn't work deterministically. In some cases the cluster fails to get healthy (coredns stuck in pending). This was caused by a race-condition between coredns starting and the fargate profile being created. If the fargate profile deployed after coredns, the pods got stuck in pending because they got assigned to the `default-scheduler` instead of the `fargate-scheduler`. The fix is relatively easy; making coredns depend on the fargate profile. I'll separately update the migration guide. ### New properties | Existing Resource | | New Top Level Property | Description | | :---- | :---- | :---- | :---- | | `clusterSecurityGroup: Output<aws.ec2.SecurityGroup \| undefined>` | | `clusterSecurityGroupId: Output<string>` | Only really useful property of a security group. Used to add additional ingress/egress rules. Default to `the EKS created security group id` | | `nodeSecurityGroup: Output<aws.ec2.SecurityGroup \| undefined>` | | `nodeSecurityGroupId: Output<string>` | | | `eksClusterIngressRule: Output<aws.ec2.SecurityGroupRule \| undefined>` | | `clusterIngressRuleId: Output<string>` | Only really useful property of a rule. Default to `””` | | `defaultNodeGroup: Output<eks.NodeGroupData \| undefined>` | | `defaultNodeGroupAsgName: Output<string>` | The only useful property of the default node group is the auto scaling group. Exposing its name allows users to reference it in IAM roles, tags, etc. Default to `””` | | `core` | `fargateProfile: Output<aws.eks.FargateProfile \| undefined>` | `fargateProfileId: Output<string>` | The id of the fargate profile. Can be used to reference it. Default to `””` | | | | `fargateProfileStatus: Output<string>` | The status of the fargate profile. Default to `””` | | | `oidcProvider: Output<aws.iam.OpenIdConnectProvider \| undefined>` | `oidcProviderArn: Output<string>` & `oidcProviderUrl: Output<string>` & `oidcIssuer: Output<string` | Arn and Url are properties needed to set up IAM identities for pods (required for the assume role policy of the IAM role). Users currently need to trim the `https://` part of the url to actually use it. We should expose `oidcProvider` with that already done to ease usage. | Fixes #1041

This change builds on top of #1445 and makes `NodeGroup` & `NodeGroupV2` accept the scalar security group properties introduced in that PR. This way users can connect their node groups to the cluster without having to use any applies.

#1445 and #1446 introduced new scalar properties as a workaround to the breaking Node.js SDK changes. This documents those in the migration guide.

flostadler added 8 commits October 16, 2024 11:27

Mark VpcCniAddon as component

50c6dcf

Add scalar properties for nested optional resources

c8e881b

Wire up new scalar properties

b4a62b5

Regenerate schema

61c2c0d

Regenerate SDK

7011c5c

Fix fargate profile race condition

041a68d

Formatting

756ceb8

Add test for scalar types

8affc67

flostadler requested review from t0yv0 and a team October 16, 2024 09:36

flostadler self-assigned this Oct 16, 2024

flostadler added the needs-release/major Marking a PR to compute the next major version label Oct 16, 2024

Simplify examples

c4cac37

flostadler force-pushed the flostadler/scalar-types branch from dd44cba to c4cac37 Compare October 16, 2024 09:36

flostadler commented Oct 16, 2024

View reviewed changes

flostadler mentioned this pull request Oct 16, 2024

Allow passing scalar security group properties to node groups #1446

Merged

Fix tests

d81c1ee

flostadler commented Oct 16, 2024

View reviewed changes

flostadler changed the title ~~Add scalar types for most commonly used outputs~~ Add scalar types for most commonly used resource outputs Oct 16, 2024

t0yv0 reviewed Oct 16, 2024

View reviewed changes

t0yv0 approved these changes Oct 16, 2024

View reviewed changes

flostadler merged commit 1e2e4aa into release-3.x.x Oct 16, 2024
34 checks passed

flostadler deleted the flostadler/scalar-types branch October 16, 2024 14:19

flostadler mentioned this pull request Oct 16, 2024

Add recommendation for scalar output to v3 migration guide #1448

Merged

pulumi-bot mentioned this pull request Oct 17, 2024

Fargate tests are flakey #1041

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add scalar types for most commonly used resource outputs #1445

Add scalar types for most commonly used resource outputs #1445

flostadler commented Oct 16, 2024 •

edited

Loading

flostadler Oct 16, 2024

github-actions bot commented Oct 16, 2024

flostadler Oct 16, 2024

t0yv0 Oct 16, 2024

flostadler Oct 16, 2024

t0yv0 Oct 16, 2024

flostadler Oct 16, 2024

t0yv0 left a comment

pulumi-bot commented Oct 17, 2024

Add scalar types for most commonly used resource outputs #1445

Add scalar types for most commonly used resource outputs #1445

Conversation

flostadler commented Oct 16, 2024 • edited Loading

New properties

flostadler Oct 16, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 16, 2024

Does the PR have any schema changes?

flostadler Oct 16, 2024

Choose a reason for hiding this comment

t0yv0 Oct 16, 2024

Choose a reason for hiding this comment

flostadler Oct 16, 2024

Choose a reason for hiding this comment

t0yv0 Oct 16, 2024

Choose a reason for hiding this comment

flostadler Oct 16, 2024

Choose a reason for hiding this comment

t0yv0 left a comment

Choose a reason for hiding this comment

pulumi-bot commented Oct 17, 2024

flostadler commented Oct 16, 2024 •

edited

Loading