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

kubelet: add setting for configuring cpuManagerPolicy #1638

Merged
merged 4 commits into from
Jul 7, 2021

Conversation

gthao313
Copy link
Member

@gthao313 gthao313 commented Jun 29, 2021

Issue number:
Part of #1628

Description of changes:
Fluent-bit does not work with cpuPolicyManager: static, and it does work with cpuPolicyManager: none. We expose kubernetes.cpu-manager-policy as a setting, so customers can specify cpuManagerPolicy to none and not be blocked by this issue while running fluent bit.

  • expose kubernetes.cpu-manager-policy as a setting
  • expose kubernetes.cpu-manager-reconcile-policy as a setting
  • add migration for cpu-manager-manager-policy - deal with the issue that changing the CPU manager policy caused issues on upgrade/downgrade ( from none to static and from static to none)
  • add migration for cpu-manager-reconcile-period

Solution about deal with the issue that changing the CPU manager policy caused issues on upgrade/downgrade

error

2028 cpu_manager.go:203] [cpumanager] could not initialize checkpoint manager: could not restore state from checkpoint: configured policy "static" differs from state checkpoint policy "none", please drain this node and delete the CPU manager checkpoint file "/var/lib/kubelet/cpu_manager_state" before restarting Kubelet, please drain node and remove policy state file

The reason why upgrade/downgrade of CPU manager policy migration is configured policy "static" differs from state checkpoint policy "none". Therefore, I use migration tool to delete cpu-manager-policy checkpoint for every migration process. No orchestrated workload would be running when the migrator is running, so it should be safe to delete that file before kubelet starts.

Testing done:

  • Test behavioral effect of new setting cpu-manager-policy
  • Test behavioral effect of new setting cpu-manager-reconcile-policy
  • Test migration process - cpu-manager-policy
  • Test migration process - cpu-manager-reconcile-policy

cpu-manager-policy

launching instance with the AMI which contains new setting and check if cpu-manager-policy has been configured expectantly. Meanwhile, configure it as none and test if fluent-bit pod work as expected.

Step1 check if cpu-manager-policy has been configured expectantly:

Test with cpu-manager-policy= "none"

1726 policy_none.go:43] [cpumanager] none policy: Start

Test with default ("none")

1737 cpu_manager.go:193] [cpumanager] starting with none policy

Test with cpu-manager-policy= "static"

1723 cpu_manager.go:193] [cpumanager] starting with static policy

Step2 configure it as none and test if fluent-bit pod work as expected:

  1. kubectl run --image public.ecr.aws/aws-observability/aws-for-fluent-bit:2.16.0 --command -- sh
  2. kubectl exec -ti sh -- sh
  3. /fluent-bit/bin/fluent-bit -H -i cpu -o null
  4. while curl localhost:2020; do echo pass; done

result (web server works perfectly ):

COMMAND   PID USER   FD   TYPE DEVICE SIZE/OFF NODE NAME
fluent-bi   1 root   19u  IPv4  20229      0t0  TCP *:24224 (LISTEN)
fluent-bi  36 root   30u  IPv4  19367      0t0  TCP *:2020 (LISTEN)

Migration test:

upgrade
Step1: Upgrade to v1.1.3

bash-5.0# updog check-update -a --json
[
  {
    "variant": "aws-k8s-1.20",
    "arch": "x86_64",
    "version": "1.1.3",
    "max_version": "1.1.3",
.....

cat var/lib/kubelet/cpu_manager_state
{"policyName":"static","defaultCpuSet":"0-3","checksum":611748604}

bash-5.0# updog update -i 1.1.3 -r -n
Starting update to 1.1.3

Step2: check if cpu-manager-policy works as expected after upgrading

cat var/lib/kubelet/cpu_manager_state
{"policyName":"none","defaultCpuSet":"","checksum":1353318690}
journalctl - u kubelet
1701 policy_none.go:43] [cpumanager] none policy: Start

Step3: Specify new setting cpu-manage-policythrough control container

apiclient set -j '{"kubernetes": {"cpu-manager-policy": "none"}}'
bash-5.0# cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-policy
"none"

downgrade

Step1: Check migration binary

ls -al /var/lib/bottlerocket-migrations
total 492
drwx------.  2 root root   4096 Jun 29 21:51 .
drwxr-xr-x. 14 root root   4096 Jun 29 22:22 ..
-rw-r--r--.  1 root root 488744 Jun 29 22:21 0b168f448fed442fc3be61059256d4c12a423aa6afba5ec825596522dd015d41.migrate_v1.1.3_cpu-manager-policy.lz4

Step2: Downgrade to previous verison

signpost rollback-to-inactive
reboot

Step3: check if cpu-manager-policy works as expected after downgrading

cat var/lib/kubelet/cpu_manager_state
{"policyName":"static","defaultCpuSet":"0-3","checksum":611748604}
journalctl - u kubelet
1700 cpu_manager.go:193] [cpumanager] starting with static policy

Step4: Check if cpu-manager-policy have been removed

cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-policy
cat: /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-policy: No such file or directory

cpu-manager-reconcile-period

launching instance with the AMI which contains new setting and check if cpu-manager-reconcile-period has been configured expectantly.

Step1 check if cpu-manager-reconcile-period has been configured expectantly:

Test with cpu-manager-reconcile-period = 100s

cpu_manager.go:194] [cpumanager] reconciling every 100s

Test with cpu-manager-reconcile-period = 10m

1730 cpu_manager.go:194] [cpumanager] reconciling every 10m0s

Test with cpu-manager-reconcile-period = 1h

1736 cpu_manager.go:194] [cpumanager] reconciling every 1h0m0s

Test with cpu-manager-reconcile-period = 1h0m20s

cpu_manager.go:194] [cpumanager] reconciling every 1h0m20s

Test with cpu-manager-reconcile-period = 10m20s

1726 cpu_manager.go:194] [cpumanager] reconciling every 10m20s

Test with cpu-manager-reconcile-period = 1h10m2s

1727 cpu_manager.go:194] [cpumanager] reconciling every 1h10m2s

Migration test:

upgrade
Step1: Upgrade to v1.1.3

bash-5.0# updog check-update -a --json
[
  {
    "variant": "aws-k8s-1.20",
    "arch": "x86_64",
    "version": "1.1.3",
    "max_version": "1.1.3",
.....

bash-5.0# updog update -i 1.1.3 -r -n
Starting update to 1.1.3

Step2: Specify new setting cpu-manager-reconcile-periodthrough control container

apiclient set -j '{"kubernetes": {"cpu-manager-reconcile-period": "10m"}}'
apiclient set -j '{"kubernetes": {"cpu-manager-policy": "none"}}'
bash-5.0# cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-reconcile-period
"10m"
cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-policy
"none"
journalctl -u kubelet
cpu_manager.go:194] [cpumanager] start with none policy
cpu_manager.go:194] [cpumanager] reconciling every 10m0s

downgrade
Step1: Check migration binary

ls -al /var/lib/bottlerocket-migrations
0b168f448fed442fc3be61059256d4c12a423aa6afba5ec825596522dd015d41.migrate_v1.1.3_cpu-manager-reconcile-period.lz4
0b168f448fed442fc3be61059256d4c12a423aa6afba5ec825596522dd015d41.migrate_v1.1.3_cpu-manager-policy.lz4

Step2: Downgrade to previous verison

signpost rollback-to-inactive
reboot

Step3: Check if cpu-manager-policy have been removed

cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-reconcile-period
cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-reconcile-period: No such file or directory
cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-policy
cat /var/lib/bottlerocket/datastore/current/live/settings/kubernetes/cpu-manager-policy: No such file or directory
journalctl -u kubelet
cpu_manager.go:194] [cpumanager] start with static policy
1702 cpu_manager.go:194] [cpumanager] reconciling every 10s

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@gthao313 gthao313 marked this pull request as draft June 29, 2021 22:30
@gthao313 gthao313 changed the title kubelet: add setting for configuring cpuPolicyManager kubelet: add setting for configuring cpuManagerPolicy Jun 29, 2021
@gthao313 gthao313 marked this pull request as ready for review June 29, 2021 22:45
Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

I'd like to add a setting for cpu-manager-reconcile-period while we're at it.

sources/models/src/modeled_types/mod.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
packages/kubernetes-1.20/kubelet-config Outdated Show resolved Hide resolved
@samjo-nyang
Copy link
Contributor

Could you backport this option to k8s 1.18 and 1.19? I think the migration process will not make any problems if you set "none" as default value.

@gthao313 gthao313 marked this pull request as draft July 2, 2021 19:35
@gthao313 gthao313 force-pushed the kubelet-settings branch 2 times, most recently from 2ff61ea to a259e4c Compare July 2, 2021 21:02
@gthao313
Copy link
Member Author

gthao313 commented Jul 2, 2021

Push above

  • add a new setting cpu-manager-reconcile-period and it migration.
  • add a custom migration for cpu-manger-policy to deal with the issue that changing the CPU manager policy caused issues on upgrade/downgrade
  • Default cpu-manger-policy to none.
  • Support cpu-manger-policy on 1.16, 1.17, 1.18 and 1.19 variant.

cpu-manger-policy migration logic: The reason why upgrade/downgrade of CPU manager policy migration is configured policy "static" differs from state checkpoint policy "none". Therefore, I use migration tool to delete cpu-manager-policy checkpoint during migration process. No orchestrated workload would be running when the migrator is running, so it should be safe to delete that file before kubelet starts.

2028 cpu_manager.go:203] [cpumanager] could not initialize checkpoint manager: could not restore state from checkpoint: configured policy "static" differs from state checkpoint policy "none", please drain this node and delete the CPU manager checkpoint file "/var/lib/kubelet/cpu_manager_state" before restarting Kubelet, please drain node and remove policy state file

@gthao313 gthao313 marked this pull request as ready for review July 2, 2021 21:04
@gthao313
Copy link
Member Author

gthao313 commented Jul 2, 2021

Push above improving error handle for fileremove according to Matt's idea.

@gthao313 gthao313 requested review from webern and etungsten July 2, 2021 23:23
Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

For the CPU state migration, it might be better to just always remove the state file on boot, when we're sure no pods are running.

I'd recommend trying it without the migration, and adding this line to all the kubernetes-tmpfiles.conf snippets instead:

r! /var/lib/kubelet/cpu_manager_state

That still doesn't make the setting safe to switch on a running system, but it at least means a reboot will always fix it.

@bcressey
Copy link
Contributor

bcressey commented Jul 6, 2021

I'd recommend trying it without the migration, and adding this line to all the kubernetes-tmpfiles.conf snippets instead:

Oops - we'll still need the migration to fix up the downgrade path when going back to a version that didn't have the tmpfiles line, so we should do both.

@gthao313
Copy link
Member Author

gthao313 commented Jul 6, 2021

Push above

  • rename variables like KubernetesDurationValue
  • improve cpu-manager-policy unusual checkpoint to For upgrade, add r! /var/lib/kuberlet/cpu_manager_state and for downgrade we should still use migration backward
  • typo improvement


impl Migration for CpuManagerPolicyCleaner {
fn forward(&mut self, input: MigrationData) -> Result<MigrationData> {
println!("CpuManagerPolicyCleaner forward has no work to do on upgrade.");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's best to explain why it doesn't have work to do on upgrade. We're not changing the value of a setting or anything. It's just that we're adding a behavior where we always remove the cpu_manager_state checkpoint file, therefore we don't need to explicitly remove the file during forward migration.

Copy link
Member Author

@gthao313 gthao313 left a comment

Choose a reason for hiding this comment

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

Push above improve KubernetesDurationValue regex and improve comments/variable name

sources/models/src/modeled_types/kubernetes.rs Outdated Show resolved Hide resolved
@gthao313 gthao313 requested a review from bcressey July 6, 2021 22:41
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sources/api/migration/migration-helpers/src/error.rs Outdated Show resolved Hide resolved
packages/kubernetes-1.16/kubelet-config Show resolved Hide resolved
sources/models/src/modeled_types/kubernetes.rs Outdated Show resolved Hide resolved
sources/models/src/modeled_types/kubernetes.rs Outdated Show resolved Hide resolved
sources/models/src/modeled_types/kubernetes.rs Outdated Show resolved Hide resolved
@gthao313
Copy link
Member Author

gthao313 commented Jul 7, 2021

Push above:

  1. Improve code according to Tom's comments
  2. Improve Kubernetes duration regex and unit test.
  3. Done configuring setting tests and migration test for kubernetes 1.16 to 1.19 variant. All tests have been passed.

README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -358,6 +358,11 @@ The following settings are optional and allow you to further configure your clus
* `settings.kubernetes.kube-api-burst`: The burst to allow while talking with kubernetes.
* `settings.kubernetes.container-log-max-size`: The maximum size of container log file before it is rotated.
* `settings.kubernetes.container-log-max-files`: The maximum number of container log files that can be present for a container.
* `settings.kubernetes.cpu-manager-policy`: Specifies the CPU manager policy. Possible values are `static` and `none`. Defaults to `none`. If you want to allow
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We usually wrap at the sentence break. This seems like kind of a wide wrapping.

@gthao313
Copy link
Member Author

gthao313 commented Jul 7, 2021

push above improve README.

@gthao313 gthao313 requested review from bcressey and webern July 7, 2021 16:34
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
pass cpu-manager-policy argument to kubelet
upgrade: remove the cpu manager policy state file on boot
downgrade: add a migration `kubelet-cpu-manager-state` to remove the
cpu manager policy state file
pass cpu-manager-reconcile-period argument to kubelet
…oncile-period

adds migration for two new settings `settings.kubernetes.cpu-manager-policy`
and `settings.kubernetes.cpu-manager-reconcile-period`
@gthao313
Copy link
Member Author

gthao313 commented Jul 7, 2021

Push above improve README according to Tom's comments

@gthao313 gthao313 requested a review from tjkirch July 7, 2021 17:50
@gthao313 gthao313 merged commit 0904a8a into bottlerocket-os:develop Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants