Custom resource requests/limits, extra binds, and extra env for control plane static pod components #1463

Oats87 · 2021-07-27T03:22:38Z

Proposed Changes

Allow for custom resource requests/limits, extra binds, and extra environment variables for control plane static pod components

Types of Changes

CLI, static pod executor

Verification

Linked Issues

Allow customizing of the static pod manifests for CPU, files, and directories #1233
Allow configuring resource requests and limits for control-plane static pods #1046

Further Comments

…d extra binds and env Signed-off-by: Chris Kim <oats87g@gmail.com>

Signed-off-by: Chris Kim <oats87g@gmail.com>

…/rke2/1046

pkg/cli/cmds/root.go

briandowns · 2021-07-27T16:29:51Z

pkg/podexecutor/staticpod.go

@@ -41,17 +41,65 @@ var (
 	defaultAuditPolicyFile = "/etc/rancher/rke2/audit-policy.yaml"
 )

+type ControlPlaneResources struct {


We can probably drop all of the prefixed "kube" since that's implied and will make visually scanning this struct a lot easier.

I am +1 on keeping them as-is for parity with the Image stuff:

rke2/pkg/images/images.go

Lines 47 to 52 in 6c1df6c

type ImageOverrideConfig struct {

SystemDefaultRegistry string

KubeAPIServer string

KubeControllerManager string

KubeProxy string

KubeScheduler string

pkg/podexecutor/staticpod.go

briandowns · 2021-07-27T17:10:55Z

Can you provide a few examples of what the data passed in would look like to the newly added arguments?

brandond · 2021-07-27T17:23:29Z

pkg/rke2/rke2_linux.go

+		CloudControllerManager: cfg.ExtraEnv.CloudControllerManager.Value(),
+	}
+
+	extraMounts := podexecutor.ControlPlaneMounts{


Should we do input validation on these? Later on we do mount := strings.Split(rawMount, ":") and reference mount[1] but we don't actually validate that they contain at least one semicolon, so that could cause a runtime panic.

Yeah, I've added some basic input validation to check that the length of the resultant slice is 2, but this doesn't actually validate that the paths are valid or anything. I'm not sure how far we want to go with the validations, as the kubelet will block the static pods from running itself as well if the volume mounts/source don't make sense

Signed-off-by: Chris Kim <oats87g@gmail.com>

pkg/rke2/rke2_linux.go

Signed-off-by: Chris Kim <oats87g@gmail.com>

codecov-commenter · 2021-07-30T22:36:40Z

Codecov Report

Merging #1463 (ad5af17) into master (6c1df6c) will decrease coverage by 0.75%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master   #1463      +/-   ##
=========================================
- Coverage    8.52%   7.77%   -0.76%     
=========================================
  Files          20      21       +1     
  Lines        1689    1852     +163     
=========================================
  Hits          144     144              
- Misses       1523    1686     +163     
  Partials       22      22

Flag	Coverage Δ
unittests	`7.77% <0.00%> (-0.76%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/cli/cmds/root.go	`0.00% <0.00%> (ø)`
pkg/podexecutor/staticpod.go	`0.00% <0.00%> (ø)`
pkg/rke2/rke2.go	`0.00% <ø> (ø)`
pkg/rke2/rke2_linux.go	`0.00% <0.00%> (ø)`
pkg/staticpod/staticpod.go	`0.00% <0.00%> (ø)`
pkg/cli/cmds/agent.go	`0.00% <0.00%> (ø)`
pkg/windows/service_linux.go	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c1df6c...ad5af17. Read the comment docs.

pkg/staticpod/staticpod.go

Signed-off-by: Chris Kim <oats87g@gmail.com>

…quests/limits Signed-off-by: Chris Kim <oats87g@gmail.com>

brandond · 2021-08-03T18:26:32Z

pkg/staticpod/staticpod.go

+		if len(mount) != 2 || len(mount) != 3 {
+			logrus.Errorf("mount for pod %s %s was not valid", p.Name, rawMount)
+			continue
+		}
+		var ro bool
+		if len(mount) == 3 {
+			switch strings.ToLower(mount[2]) {
+			case "ro":
+				ro = true
+			case "rw":
+				ro = false
+			default:
+				logrus.Errorf("unknown mount option: %s encountered in extra mount %s for pod %s", mount[2], rawMount, p.Name)
+			}
+		}


I think this would be a lot tidier as a switch len(mount). case 2 is a noop, case 3 checks for ro/rw, default is an error.

I changed to using a switch, but am now doing a switch within a switch. I think this is acceptable.

I put a switch in your switch, so you can switch while you switch /xzibit

… for env var parsing Signed-off-by: Chris Kim <oats87g@gmail.com>

Signed-off-by: Chris Kim <oats87g@gmail.com>

pkg/staticpod/staticpod.go

briandowns · 2021-08-03T18:48:04Z

pkg/staticpod/staticpod.go

-			Requests: v1.ResourceList{
-				v1.ResourceCPU: *resource.NewMilliQuantity(args.CPUMillis, resource.DecimalSI),
-			},
+	p.Spec.Containers[0].Resources = v1.ResourceRequirements{}


Are these not already initialized to their 0 value for that type?

Signed-off-by: Chris Kim <oats87g@gmail.com>

Oats87 · 2021-08-03T19:41:47Z

#1233

…ol plane static pod components (rancher#1463) * Add customization of cpu/memory limits/requests and extra binds and env

…ol plane static pod components (rancher#1463)

…ol plane static pod components (#1463) (#1526)

Oats87 added 5 commits July 26, 2021 18:01

Initial attempt to add customization of cpu/memory limits/requests an…

0b84f14

…d extra binds and env Signed-off-by: Chris Kim <oats87g@gmail.com>

goimports

cc39afe

Signed-off-by: Chris Kim <oats87g@gmail.com>

fix import cycle

58f5b6d

Signed-off-by: Chris Kim <oats87g@gmail.com>

Don't parse if the string is empty

80bb702

Signed-off-by: Chris Kim <oats87g@gmail.com>

Only set request/limits if defined

cd42aed

Signed-off-by: Chris Kim <oats87g@gmail.com>

Oats87 requested a review from a team as a code owner July 27, 2021 03:22

Oats87 added 4 commits July 26, 2021 20:41

fix formatting

445991b

Signed-off-by: Chris Kim <oats87g@gmail.com>

fix formatting

e2f744a

Signed-off-by: Chris Kim <oats87g@gmail.com>

change to mount from bind

a691a0a

Signed-off-by: Chris Kim <oats87g@gmail.com>

Merge branch 'issues/rke2/1046' of github.com:Oats87/rke2 into issues…

1d3e485

…/rke2/1046

briandowns reviewed Jul 27, 2021

View reviewed changes

pkg/cli/cmds/root.go Outdated Show resolved Hide resolved

briandowns reviewed Jul 27, 2021

View reviewed changes

pkg/podexecutor/staticpod.go Outdated Show resolved Hide resolved

brandond requested changes Jul 27, 2021

View reviewed changes

Oats87 added 2 commits July 27, 2021 12:47

add rudimentary input validation for env and volumes

0f6e9e2

Signed-off-by: Chris Kim <oats87g@gmail.com>

optimization corrections to formatting

6e1eed4

Signed-off-by: Chris Kim <oats87g@gmail.com>

brandond requested changes Jul 28, 2021

View reviewed changes

pkg/rke2/rke2_linux.go Show resolved Hide resolved

Oats87 added 4 commits July 30, 2021 13:23

better parsing of requests/limits

0b850dd

Signed-off-by: Chris Kim <oats87g@gmail.com>

minor logic correction

454c257

Signed-off-by: Chris Kim <oats87g@gmail.com>

more optimization

08bccf0

Signed-off-by: Chris Kim <oats87g@gmail.com>

gofmt

fe6b8d4

Signed-off-by: Chris Kim <oats87g@gmail.com>

brandond approved these changes Jul 30, 2021

View reviewed changes

Oats87 added 6 commits July 30, 2021 14:29

more optimization

0587227

Signed-off-by: Chris Kim <oats87g@gmail.com>

import cycle correction

eed8dd5

Signed-off-by: Chris Kim <oats87g@gmail.com>

move default

c1d8b47

Signed-off-by: Chris Kim <oats87g@gmail.com>

fix import cycle

9c90fd5

Signed-off-by: Chris Kim <oats87g@gmail.com>

remove unnecessary import

467cc38

Signed-off-by: Chris Kim <oats87g@gmail.com>

missed import issue

ad5af17

Signed-off-by: Chris Kim <oats87g@gmail.com>

brandond reviewed Aug 2, 2021

View reviewed changes

pkg/staticpod/staticpod.go Outdated Show resolved Hide resolved

Add RO/RW options to extraMount

4be20c1

Signed-off-by: Chris Kim <oats87g@gmail.com>

Oats87 force-pushed the issues/rke2/1046 branch from 7d91757 to 4be20c1 Compare August 3, 2021 18:00

Add basic documentation for extra volumes, extra env, and resource re…

5b40c1d

…quests/limits Signed-off-by: Chris Kim <oats87g@gmail.com>

brandond requested changes Aug 3, 2021

View reviewed changes

Oats87 added 2 commits August 3, 2021 11:30

Optimize volume mount ro/rw detection and use SplitN instead of Split…

680f7d6

… for env var parsing Signed-off-by: Chris Kim <oats87g@gmail.com>

Fix imports

bc172b6

Signed-off-by: Chris Kim <oats87g@gmail.com>

Oats87 changed the title ~~WIP: Custom resource requests/limits, extra binds, and extra env for control plane static pod components~~ Custom resource requests/limits, extra binds, and extra env for control plane static pod components Aug 3, 2021

briandowns reviewed Aug 3, 2021

View reviewed changes

pkg/staticpod/staticpod.go Outdated Show resolved Hide resolved

briandowns reviewed Aug 3, 2021

View reviewed changes

pkg/staticpod/staticpod.go Outdated Show resolved Hide resolved

briandowns reviewed Aug 3, 2021

View reviewed changes

pkg/staticpod/staticpod.go Show resolved Hide resolved

briandowns reviewed Aug 3, 2021

View reviewed changes

Address PR comments

2e1d275

Signed-off-by: Chris Kim <oats87g@gmail.com>

briandowns approved these changes Aug 3, 2021

View reviewed changes

Oats87 requested a review from brandond August 3, 2021 18:57

brandond approved these changes Aug 3, 2021

View reviewed changes

Oats87 merged commit 959247b into rancher:master Aug 3, 2021

Oats87 added a commit to Oats87/rke2 that referenced this pull request Aug 3, 2021

Custom resource requests/limits, extra binds, and extra env for contr…

bb61c8f

…ol plane static pod components (rancher#1463) * Add customization of cpu/memory limits/requests and extra binds and env

Oats87 mentioned this pull request Aug 3, 2021

[release-1.21] Custom resource requests/limits, extra binds, and extra env for control plane static pod components #1526

Merged

Oats87 added a commit to Oats87/rke2 that referenced this pull request Aug 3, 2021

Custom resource requests/limits, extra binds, and extra env for contr…

e1bf18d

…ol plane static pod components (rancher#1463)

Oats87 added a commit that referenced this pull request Aug 3, 2021

Custom resource requests/limits, extra binds, and extra env for contr…

52a7adf

…ol plane static pod components (#1463) (#1526)

This was referenced Aug 13, 2021

Make docs for new flags more clear #1620

Merged

Change the Kube-API Audit.log output file path #1653

Closed

dweomer mentioned this pull request Aug 24, 2021

Update server_config.md #925

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom resource requests/limits, extra binds, and extra env for control plane static pod components #1463

Custom resource requests/limits, extra binds, and extra env for control plane static pod components #1463

Oats87 commented Jul 27, 2021 •

edited by brandond

briandowns Jul 27, 2021

brandond Jul 27, 2021 •

edited

briandowns commented Jul 27, 2021

brandond Jul 27, 2021

Oats87 Jul 27, 2021

codecov-commenter commented Jul 30, 2021

brandond Aug 3, 2021

Oats87 Aug 3, 2021

brandond Aug 3, 2021

briandowns Aug 3, 2021

Oats87 commented Aug 3, 2021

	type ImageOverrideConfig struct {
	SystemDefaultRegistry string
	KubeAPIServer string
	KubeControllerManager string
	KubeProxy string
	KubeScheduler string

Custom resource requests/limits, extra binds, and extra env for control plane static pod components #1463

Custom resource requests/limits, extra binds, and extra env for control plane static pod components #1463

Conversation

Oats87 commented Jul 27, 2021 • edited by brandond

Proposed Changes

Types of Changes

Verification

Linked Issues

Further Comments

briandowns Jul 27, 2021

Choose a reason for hiding this comment

brandond Jul 27, 2021 • edited

Choose a reason for hiding this comment

briandowns commented Jul 27, 2021

brandond Jul 27, 2021

Choose a reason for hiding this comment

Oats87 Jul 27, 2021

Choose a reason for hiding this comment

codecov-commenter commented Jul 30, 2021

Codecov Report

brandond Aug 3, 2021

Choose a reason for hiding this comment

Oats87 Aug 3, 2021

Choose a reason for hiding this comment

brandond Aug 3, 2021

Choose a reason for hiding this comment

briandowns Aug 3, 2021

Choose a reason for hiding this comment

Oats87 commented Aug 3, 2021

Oats87 commented Jul 27, 2021 •

edited by brandond

brandond Jul 27, 2021 •

edited