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

Do not create instance role when skipDefaultNodeGroup is enabled #1411

Merged
Merged
Show file tree
Hide file tree
Changes from 6 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
4 changes: 4 additions & 0 deletions docs/eks-v3-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,7 @@ The `NodeGroup` and `NodeGroupV2` components now accept inputs for the following
- `nodeAssociatePublicIpAddress`

If you're using Go you'll need to adjust your program to handle those types being inputs.

### Cluster does not create extraneous instance IAM role if `skipDefaultNodeGroup` is set to `true`

Previously the Cluster component created a default instance IAM role even if `skipDefaultNodeGroup` was set to `true`. This role gets correctly omitted now if you're specifying `skipDefaultNodeGroup`.
Copy link
Member

Choose a reason for hiding this comment

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

"if you are"

2 changes: 2 additions & 0 deletions examples/custom-managed-nodegroup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const cluster = new eks.Cluster("example-managed-nodegroup", {
}
});

export const defaultInstanceRoles = cluster.instanceRoles;

// Export the cluster's kubeconfig.
export const kubeconfig = cluster.kubeconfig;

Expand Down
3 changes: 3 additions & 0 deletions examples/examples_nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,9 @@ func TestAccManagedNodeGroupCustom(t *testing.T) {
info.Outputs["kubeconfig"],
)

defaultInstanceRoles := info.Outputs["defaultInstanceRoles"]
assert.Empty(t, defaultInstanceRoles, "Expected no instanceRoles to be created")

expectedLaunchTemplateName := info.Outputs["launchTemplateName"]

var launchTemplateFound bool = false
Expand Down
19 changes: 9 additions & 10 deletions nodejs/eks/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ export function createCore(
}
instanceRoles = pulumi.output([args.instanceRole]);
defaultInstanceRole = pulumi.output(args.instanceRole);
} else {
} else if (!args.skipDefaultNodeGroup) {
const instanceRole = new ServiceRole(
`${name}-instanceRole`,
{
Expand Down Expand Up @@ -888,15 +888,14 @@ export function createCore(
);
}

// Create an instance profile if using a default node group
if (!skipDefaultNodeGroup) {
nodeGroupOptions.instanceProfile = createOrGetInstanceProfile(
name,
parent,
instanceRole,
args.instanceProfileName,
);
}
nodeGroupOptions.instanceProfile = createOrGetInstanceProfile(
name,
parent,
instanceRole,
args.instanceProfileName,
);
} else {
instanceRoles = pulumi.output([]);
}

let eksNodeAccess: k8s.core.v1.ConfigMap | undefined = undefined;
Expand Down
1 change: 1 addition & 0 deletions provider/provider_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
)

func TestEksAuthModeUpgrade(t *testing.T) {
t.Skip("Temporarily skipping upgrade tests. Needs to be addressed in pulumi/pulumi-eks#1387")
t.Parallel()
if testing.Short() {
t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without credentials")
Expand Down
14 changes: 7 additions & 7 deletions sdk/python/pulumi_eks/_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ class ClusterNodeGroupOptionsArgsDict(TypedDict):

Note: Given the inheritance of auto-generated CF tags and `cloudFormationTags`, you should either supply the tag in `autoScalingGroupTags` or `cloudFormationTags`, but not both.
"""
bootstrap_extra_args: NotRequired[str]
bootstrap_extra_args: NotRequired[pulumi.Input[str]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merging #1414 & #1415 ontop of each other dropped those changes.
This adds is back now, work tree is clean again

Copy link
Member

Choose a reason for hiding this comment

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

Are we missing CI check guarantees that the worktree should be clean possibly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're merging those into a feature branch that's only built when cutting an alpha release or when triggered manually. It's checking it as part of those workflows

"""
Additional args to pass directly to `/etc/eks/bootstrap.sh`. For details on available options, see: https://github.com/awslabs/amazon-eks-ami/blob/master/files/bootstrap.sh. Note that the `--apiserver-endpoint`, `--b64-cluster-ca` and `--kubelet-extra-args` flags are included automatically based on other configuration parameters.
"""
Expand Down Expand Up @@ -370,11 +370,11 @@ class ClusterNodeGroupOptionsArgsDict(TypedDict):
"""
Name of the key pair to use for SSH access to worker nodes.
"""
kubelet_extra_args: NotRequired[str]
kubelet_extra_args: NotRequired[pulumi.Input[str]]
"""
Extra args to pass to the Kubelet. Corresponds to the options passed in the `--kubeletExtraArgs` flag to `/etc/eks/bootstrap.sh`. For example, '--port=10251 --address=0.0.0.0'. Note that the `labels` and `taints` properties will be applied to this list (using `--node-labels` and `--register-with-taints` respectively) after to the explicit `kubeletExtraArgs`.
"""
labels: NotRequired[Mapping[str, str]]
labels: NotRequired[pulumi.Input[Mapping[str, pulumi.Input[str]]]]
"""
Custom k8s node labels to be attached to each worker node. Adds the given key/value pairs to the `--node-labels` kubelet argument.
"""
Expand All @@ -394,7 +394,7 @@ class ClusterNodeGroupOptionsArgsDict(TypedDict):
"""
The minimum number of worker nodes running in the cluster. Defaults to 1.
"""
node_associate_public_ip_address: NotRequired[bool]
node_associate_public_ip_address: NotRequired[pulumi.Input[bool]]
"""
Whether or not to auto-assign public IP addresses on the EKS worker nodes. If this toggle is set to true, the EKS workers will be auto-assigned public IPs. If false, they will not be auto-assigned public IPs.
"""
Expand Down Expand Up @@ -480,7 +480,7 @@ class ClusterNodeGroupOptionsArgsDict(TypedDict):
"""
Bidding price for spot instance. If set, only spot instances will be added as worker node.
"""
taints: NotRequired[Mapping[str, 'TaintArgsDict']]
taints: NotRequired[pulumi.Input[Mapping[str, pulumi.Input['TaintArgsDict']]]]
"""
Custom k8s node taints to be attached to each worker node. Adds the given taints to the `--register-with-taints` kubelet argument
"""
Expand Down Expand Up @@ -2507,11 +2507,11 @@ class TaintArgsDict(TypedDict):
"""
Represents a Kubernetes `taint` to apply to all Nodes in a NodeGroup. See https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/.
"""
effect: str
effect: pulumi.Input[str]
"""
The effect of the taint.
"""
value: str
value: pulumi.Input[str]
"""
The value of the taint.
"""
Expand Down
Loading