feat: add custom environment variables to the 'aws-node' DaemonSet #3374

charlie-haley · 2022-04-01T14:44:59Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Ability to add custom environment variables to the aws-node DaemonSet. This is mostly so you can increase pod limits on nodes through the ENABLE_PREFIX_DELEGATION environment variable, but this PR supports all environment variables.

Which issue(s) this PR fixes:
Fixes #3244

Checklist:

squashed commits
includes documentation
adds unit tests
adds or updates e2e tests

linux-foundation-easycla · 2022-04-01T14:45:03Z

The committers listed above are authorized under a signed CLA.

✅ login: charlie-haley / name: Charlie Haley (26b13ab58d7f17587513cd787b10d6cf0468b9f8)

k8s-ci-robot · 2022-04-01T14:45:07Z

@charlie-haley: 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.

k8s-ci-robot · 2022-04-01T14:45:07Z

Welcome @charlie-haley!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-aws 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-aws has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-04-01T14:45:08Z

Hi @charlie-haley. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 · 2022-04-01T15:13:46Z

/ok-to-test

richardcase · 2022-04-01T15:57:29Z

@charlie-haley - For the verify issue i think you need to run go mod tidy. The linter error is a weird one. Lets chat Monday morning on slack.

richardcase · 2022-04-01T15:57:54Z

And thanks for working on this 👍

pydctw · 2022-04-01T18:30:30Z

@charlie-haley - For the verify issue i think you need to run go mod tidy. The linter error is a weird one. Lets chat Monday morning on slack.

The linter error message is cryptic but basically the import order needs to be changed. See comment

pkg/cloud/services/awsnode/service.go

docs/book/src/topics/eks/pod-networking.md

pydctw · 2022-04-05T16:44:39Z

pull-cluster-api-provider-aws-verify - looks like generated conversion files are out of date. Could you run make generate?

generated files are out of date, run make generate
make: *** [Makefile:319: verify-gen] Error 1

pkg/cloud/services/awsnode/service.go

pydctw · 2022-04-06T15:41:42Z

Great to see tests passing - 🎉. Now, time to run e2e test

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

pydctw · 2022-04-07T15:04:41Z

e2e test result looks good to me

I0406 16:46:04.492514       1 cni.go:44]  "msg"="Reconciling aws-node DaemonSet in cluster"  "cluster-name"="cluster-yknmkm" "cluster-namespace"="eks-nodes-lrf3qi"
I0406 16:46:04.966908       1 cni.go:68]  "msg"="updating aws-node daemonset environment variables"  "cluster-name"="cluster-yknmkm" "cluster-namespace"="eks-nodes-lrf3qi"

AWSManagedControlPlane

spec:
  addons:
  - conflictResolution: overwrite
    name: vpc-cni
    version: v1.8.0-eksbuild.1
  - conflictResolution: overwrite
    name: coredns
    version: v1.8.3-eksbuild.1
  associateOIDCProvider: false
  awsNodeEnvVars:
  - name: FOO
    value: BAR
  - name: ENABLE_PREFIX_DELEGATION
    value: "true"

pydctw · 2022-04-07T15:04:46Z

/lgtm

charlie-haley · 2022-06-21T08:23:42Z

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

charlie-haley · 2022-06-22T10:06:32Z

Not sure what's happening here, I don't get any changes when running make generate locally, I also rebased onto master (which fixed the apidiff issue) any ideas @richardcase @pydctw?

richardcase · 2022-06-22T10:11:42Z

Not sure what's happening here, I don't get any changes when running make generate locally, I also rebased onto master (which fixed the apidiff issue) any ideas @richardcase @pydctw?

Interesting, i've seen this on another PR. Could you do the following to see if you get a diferent result:

make clean
make generate

richardcase · 2022-06-22T11:42:53Z

@charlie-haley - there was an issue with code generation. If you rebase your PR on main this issue with the verify job should go away.

charlie-haley · 2022-06-22T12:21:15Z

@charlie-haley - there was an issue with code generation. If you rebase your PR on main this issue with the verify job should go away.

That did the trick, thanks @richardcase

mloiseleur · 2022-06-27T06:46:29Z

@richardcase @pydctw @dlipovetsky Ping for visibility. This PR is ready to be merged.

richardcase · 2022-06-27T07:23:29Z

/test ?

k8s-ci-robot · 2022-06-27T07:23:30Z

@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-apidiff-main
/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-apidiff-main
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 · 2022-06-27T07:23:46Z

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

richardcase · 2022-06-28T14:18:34Z

The e2e tests are passing.

Thanks for your work on this @charlie-haley 🙇‍♂️ From my side:

/approve

@dlipovetsky @Ankitasw - any chance you could take a look and when you are happy lgtm?

k8s-ci-robot · 2022-06-28T14:20:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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~~ [richardcase]

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

Ankitasw · 2022-06-28T15:15:46Z

@richardcase I will take a look.

Ankitasw

nit, otherwise LGTM

controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go

Signed-off-by: Charlie Haley <charlie.haley@hotmail.com>

Ankitasw · 2022-06-29T08:31:36Z

/lgtm

Skarlso · 2022-06-29T14:32:26Z

controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go

@@ -167,6 +168,10 @@ type AWSManagedControlPlaneSpec struct { //nolint: maligned
 	// +kubebuilder:default=false
 	DisableVPCCNI bool `json:"disableVPCCNI,omitempty"`

+	// VpcCni is used to set configuration options for the VPC CNI plugin
+	// +optional
+	VpcCni VpcCni `json:"VpcCni,omitempty"`


@richardcase Ah I haven't noticed that but this should be lowercase.

I'll fix it in my PR. :)

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 1, 2022

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 1, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 1, 2022

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 1, 2022

k8s-ci-robot requested review from dlipovetsky and pydctw April 1, 2022 14:45

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Apr 1, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 1, 2022

pydctw reviewed Apr 1, 2022

View reviewed changes

pkg/cloud/services/awsnode/service.go Outdated Show resolved Hide resolved

pydctw reviewed Apr 1, 2022

View reviewed changes

docs/book/src/topics/eks/pod-networking.md Show resolved Hide resolved

charlie-haley force-pushed the feat/aws-node-env-vars branch from ce0ed6b to 46d1cf3 Compare April 5, 2022 10:15

pydctw reviewed Apr 5, 2022

View reviewed changes

pkg/cloud/services/awsnode/service.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned pydctw Apr 7, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 7, 2022

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2022

charlie-haley force-pushed the feat/aws-node-env-vars branch 2 times, most recently from 63e0b72 to a01325c Compare June 21, 2022 08:22

charlie-haley force-pushed the feat/aws-node-env-vars branch 3 times, most recently from b2d5aca to 3627fda Compare June 22, 2022 09:10

charlie-haley force-pushed the feat/aws-node-env-vars branch from 3627fda to 4dad04f Compare June 22, 2022 12:03

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 28, 2022

Ankitasw reviewed Jun 29, 2022

View reviewed changes

controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go Outdated Show resolved Hide resolved

feat: add custom environment variables to the 'aws-node' DaemonSet

4de7d4b

Signed-off-by: Charlie Haley <charlie.haley@hotmail.com>

charlie-haley force-pushed the feat/aws-node-env-vars branch from 4dad04f to 4de7d4b Compare June 29, 2022 08:22

k8s-ci-robot assigned Ankitasw Jun 29, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 29, 2022

k8s-ci-robot merged commit 305244d into kubernetes-sigs:main Jun 29, 2022

k8s-ci-robot added this to the v1.x milestone Jun 29, 2022

Skarlso reviewed Jun 29, 2022

View reviewed changes

richardcase mentioned this pull request Jul 19, 2022

feat: external load balancer garbage collection #3518

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add custom environment variables to the 'aws-node' DaemonSet #3374

feat: add custom environment variables to the 'aws-node' DaemonSet #3374

charlie-haley commented Apr 1, 2022

linux-foundation-easycla bot commented Apr 1, 2022 •

edited

Loading

k8s-ci-robot commented Apr 1, 2022

k8s-ci-robot commented Apr 1, 2022

k8s-ci-robot commented Apr 1, 2022

richardcase commented Apr 1, 2022

richardcase commented Apr 1, 2022 •

edited

Loading

richardcase commented Apr 1, 2022

pydctw commented Apr 1, 2022 •

edited

Loading

pydctw commented Apr 5, 2022

pydctw commented Apr 6, 2022

pydctw commented Apr 7, 2022

pydctw commented Apr 7, 2022

charlie-haley commented Jun 21, 2022

charlie-haley commented Jun 22, 2022

richardcase commented Jun 22, 2022

richardcase commented Jun 22, 2022

charlie-haley commented Jun 22, 2022

mloiseleur commented Jun 27, 2022

richardcase commented Jun 27, 2022

k8s-ci-robot commented Jun 27, 2022

richardcase commented Jun 27, 2022

richardcase commented Jun 28, 2022

k8s-ci-robot commented Jun 28, 2022

Ankitasw commented Jun 28, 2022

Ankitasw left a comment •

edited

Loading

Ankitasw commented Jun 29, 2022

Skarlso Jun 29, 2022

feat: add custom environment variables to the 'aws-node' DaemonSet #3374

feat: add custom environment variables to the 'aws-node' DaemonSet #3374

Conversation

charlie-haley commented Apr 1, 2022

linux-foundation-easycla bot commented Apr 1, 2022 • edited Loading

k8s-ci-robot commented Apr 1, 2022

k8s-ci-robot commented Apr 1, 2022

k8s-ci-robot commented Apr 1, 2022

richardcase commented Apr 1, 2022

richardcase commented Apr 1, 2022 • edited Loading

richardcase commented Apr 1, 2022

pydctw commented Apr 1, 2022 • edited Loading

pydctw commented Apr 5, 2022

pydctw commented Apr 6, 2022

pydctw commented Apr 7, 2022

pydctw commented Apr 7, 2022

charlie-haley commented Jun 21, 2022

charlie-haley commented Jun 22, 2022

richardcase commented Jun 22, 2022

richardcase commented Jun 22, 2022

charlie-haley commented Jun 22, 2022

mloiseleur commented Jun 27, 2022

richardcase commented Jun 27, 2022

k8s-ci-robot commented Jun 27, 2022

richardcase commented Jun 27, 2022

richardcase commented Jun 28, 2022

k8s-ci-robot commented Jun 28, 2022

Ankitasw commented Jun 28, 2022

Ankitasw left a comment • edited Loading

Choose a reason for hiding this comment

Ankitasw commented Jun 29, 2022

Skarlso Jun 29, 2022

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Apr 1, 2022 •

edited

Loading

richardcase commented Apr 1, 2022 •

edited

Loading

pydctw commented Apr 1, 2022 •

edited

Loading

Ankitasw left a comment •

edited

Loading