feat: expose additional eks bootstrap options #2965

richardcase · 2021-11-16T17:32:41Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

Added additional options to EKSConfig so that additional configuration
options have be set when bootstrapping a node when it joins an eks
cluster.

Note: the IPv6 services cidr is deliberately not exposed as this will be
done when we come to do the IPv6 work. There are some comments left
in on purpose for this.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2853

Special notes for your reviewer:

Checklist:

squashed commits
adds unit tests

Release note:

Expose additional EKS node bootstrap configuration options via EKSConfig.

k8s-ci-robot · 2021-11-16T17:32:49Z

@richardcase: This issue is currently awaiting triage.

If CAPA/CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sedefsavas · 2021-11-16T17:40:44Z

@richardcase do you want this in before v1.1.0 release or is it okay to merge this after we make the release?

Feel free to add a milestone when you decide.

richardcase · 2021-11-30T08:39:43Z

/test ?

k8s-ci-robot · 2021-11-30T08:39:57Z

@richardcase: The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-aws-build
/test pull-cluster-api-provider-aws-test
/test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-blocking
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
/test pull-cluster-api-provider-aws-e2e-eks

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-aws-build
pull-cluster-api-provider-aws-test
pull-cluster-api-provider-aws-verify

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

richardcase · 2021-11-30T08:42:36Z

/test pull-cluster-api-provider-aws-e2e-eks

sedefsavas

Non-blocking comments. LGTM.

sedefsavas · 2021-12-03T00:19:59Z

bootstrap/eks/internal/userdata/node.go

+	PauseContainerVersion *string
+	UseMaxPods            *bool
+	IPFamily              *string
+	ServiceIPV6Cidr       *string


It'd be good to have a note here for both ServiceIPV6Cidr and IPFamily as this is not implemented yet and no-op.

Added a comment

sedefsavas · 2021-12-03T00:24:10Z

bootstrap/eks/controllers/eksconfig_controller.go

+	// TODO(richardcase): uncomment when we support ipv6 / dual stack
+	/*if config.Spec.ServiceIPV6Cidr != nil && *config.Spec.ServiceIPV6Cidr != "" {
+		nodeInput.ServiceIPV6Cidr = config.Spec.ServiceIPV6Cidr
+		nodeInput.IPFamily = pointer.String("ipv6")


For future implementation:
If setting IPFamily without setting ServiceIPV6Cidr possible during EKS bootstrap, then I think having IPFamily as a separate field is needed.

It is possible to set IPFamily without setting ServiceIPV6Cidr but i think we should hide this detail based on whether the user has supplied ipv4 or 6

sedefsavas · 2021-12-10T09:39:48Z

/test?

k8s-ci-robot · 2021-12-10T09:39:49Z

@sedefsavas: The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-aws-build
/test pull-cluster-api-provider-aws-test
/test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-blocking
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
/test pull-cluster-api-provider-aws-e2e-eks

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-aws-build
pull-cluster-api-provider-aws-test
pull-cluster-api-provider-aws-verify

In response to this:

/test?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sedefsavas · 2021-12-10T09:40:45Z

/test pull-cluster-api-provider-aws-e2e-eks

sedefsavas · 2021-12-10T13:15:12Z

/test pull-cluster-api-provider-aws-e2e-eks

sedefsavas · 2021-12-10T13:31:14Z

bootstrap/eks/api/v1alpha4/conversion.go

 )

 // ConvertTo converts the v1alpha4 EKSConfig receiver to a v1beta1 EKSConfig.
 func (r *EKSConfig) ConvertTo(dstRaw conversion.Hub) error {
 	dst := dstRaw.(*v1beta1.EKSConfig)
-
-	return Convert_v1alpha4_EKSConfig_To_v1beta1_EKSConfig(r, dst, nil)
+	if err := Convert_v1alpha4_EKSConfig_To_v1beta1_EKSConfig(r, dst, nil); err != nil {


Do we need to do the same for v1alpha3?

We do, added.

Added additional options to `EKSConfig` so that additional configuration options have be set when bootstrapping a node when it joins an eks cluster. Note: the IPv6 services cidr is deliberately not exposed as this will be done when we come to do the IPv6 work. There are some comments left in on purpose for this. Signed-off-by: Richard Case <richard@weave.works>

sedefsavas · 2021-12-10T15:17:12Z

/lgtm
/approve

k8s-ci-robot · 2021-12-10T15:17:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sedefsavas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sedefsavas]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…fig_options feat: expose additional eks bootstrap options

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 16, 2021

k8s-ci-robot requested review from Ankitasw and randomvariable November 16, 2021 17:32

richardcase mentioned this pull request Nov 16, 2021

WIP feat: expose additional EKS bootstrap options #2854

Closed

4 tasks

k8s-ci-robot added this to the v1.2.0 milestone Nov 16, 2021

richardcase removed this from the v1.2.0 milestone Nov 16, 2021

sedefsavas reviewed Dec 3, 2021

View reviewed changes

sedefsavas reviewed Dec 10, 2021

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 10, 2021

k8s-ci-robot assigned sedefsavas Dec 10, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 10, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 10, 2021

k8s-ci-robot merged commit c356b7e into kubernetes-sigs:main Dec 10, 2021

k8s-ci-robot added this to the v1.x milestone Dec 10, 2021

cnmcavoy mentioned this pull request Mar 9, 2022

prebootstrapcommand in machinepool / managed machinepool #2771

Closed

richardchen-db pushed a commit to databricks/cluster-api-provider-aws-1 that referenced this pull request Jan 14, 2023

Merge pull request kubernetes-sigs#2965 from richardcase/2853_eks_con…

0a2d518

…fig_options feat: expose additional eks bootstrap options

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: expose additional eks bootstrap options #2965

feat: expose additional eks bootstrap options #2965

richardcase commented Nov 16, 2021

k8s-ci-robot commented Nov 16, 2021

sedefsavas commented Nov 16, 2021 •

edited

Loading

richardcase commented Nov 30, 2021

k8s-ci-robot commented Nov 30, 2021

richardcase commented Nov 30, 2021

sedefsavas left a comment

sedefsavas Dec 3, 2021

richardcase Dec 10, 2021

sedefsavas Dec 3, 2021

richardcase Dec 10, 2021

sedefsavas commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

sedefsavas commented Dec 10, 2021

sedefsavas commented Dec 10, 2021

sedefsavas Dec 10, 2021

richardcase Dec 10, 2021

sedefsavas commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

feat: expose additional eks bootstrap options #2965

feat: expose additional eks bootstrap options #2965

Conversation

richardcase commented Nov 16, 2021

k8s-ci-robot commented Nov 16, 2021

sedefsavas commented Nov 16, 2021 • edited Loading

richardcase commented Nov 30, 2021

k8s-ci-robot commented Nov 30, 2021

richardcase commented Nov 30, 2021

sedefsavas left a comment

Choose a reason for hiding this comment

sedefsavas Dec 3, 2021

Choose a reason for hiding this comment

richardcase Dec 10, 2021

Choose a reason for hiding this comment

sedefsavas Dec 3, 2021

Choose a reason for hiding this comment

richardcase Dec 10, 2021

Choose a reason for hiding this comment

sedefsavas commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

sedefsavas commented Dec 10, 2021

sedefsavas commented Dec 10, 2021

sedefsavas Dec 10, 2021

Choose a reason for hiding this comment

richardcase Dec 10, 2021

Choose a reason for hiding this comment

sedefsavas commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

sedefsavas commented Nov 16, 2021 •

edited

Loading