-
Couldn't load subscription status.
- Fork 25
Add multiple k8s settings #104
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,7 +58,7 @@ bottlerocket-template-helper = { path = "./bottlerocket-template-helper", versio | |
|
|
||
| # Settings Models | ||
| bottlerocket-model-derive = { path = "./bottlerocket-settings-models/model-derive", version = "0.1" } | ||
| bottlerocket-modeled-types = { path = "./bottlerocket-settings-models/modeled-types", version = "0.12" } | ||
| bottlerocket-modeled-types = { path = "./bottlerocket-settings-models/modeled-types", version = "0.13" } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have commits for the other version bumps, are they not needed for this bump? |
||
| bottlerocket-scalar = { path = "./bottlerocket-settings-models/scalar", version = "0.1" } | ||
| bottlerocket-scalar-derive = { path = "./bottlerocket-settings-models/scalar-derive", version = "0.1" } | ||
| bottlerocket-string-impls-for = { path = "./bottlerocket-settings-models/string-impls-for", version = "0.1" } | ||
|
|
@@ -76,7 +76,7 @@ settings-extension-dns = { path = "./bottlerocket-settings-models/settings-exten | |
| settings-extension-ecs = { path = "./bottlerocket-settings-models/settings-extensions/ecs", version = "0.1" } | ||
| settings-extension-host-containers = { path = "./bottlerocket-settings-models/settings-extensions/host-containers", version = "0.2" } | ||
| settings-extension-kernel = { path = "./bottlerocket-settings-models/settings-extensions/kernel", version = "0.1" } | ||
| settings-extension-kubernetes = { path = "./bottlerocket-settings-models/settings-extensions/kubernetes", version = "0.5" } | ||
| settings-extension-kubernetes = { path = "./bottlerocket-settings-models/settings-extensions/kubernetes", version = "0.6" } | ||
| settings-extension-kubelet-device-plugins = { path = "./bottlerocket-settings-models/settings-extensions/kubelet-device-plugins", version = "0.3" } | ||
| settings-extension-metrics = { path = "./bottlerocket-settings-models/settings-extensions/metrics", version = "0.1" } | ||
| settings-extension-motd = { path = "./bottlerocket-settings-models/settings-extensions/motd", version = "0.1" } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |
|
|
||
| - See [unreleased changes here] | ||
|
|
||
| [unreleased changes here]: https://github.com/bottlerocket-os/bottlerocket-settings-sdk/compare/bottlerocket-settings-models-v0.16.0...HEAD | ||
| [unreleased changes here]: https://github.com/bottlerocket-os/bottlerocket-settings-sdk/compare/bottlerocket-settings-models-v0.17.0...HEAD | ||
|
|
||
| ## [0.17.0] - 2025-10-10 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should also update the date of this release. |
||
|
|
||
| ## Model Changes | ||
|
|
||
| ### Added | ||
|
|
||
| - Added `image-minimum-gc-age` and `image-maximum-gc-age` kubernetes settings ([#87]) - Thanks @parnniti! | ||
| - Added `ids-per-pod` and `max-parallel-image-pulls` kubernetes settings ([#104]) | ||
|
|
||
| [#87]:https://github.com/bottlerocket-os/bottlerocket-settings-sdk/pull/87 | ||
| [#104]:https://github.com/bottlerocket-os/bottlerocket-settings-sdk/pull/104 | ||
|
|
||
| [0.17.0]: https://github.com/bottlerocket-os/bottlerocket-settings-sdk/compare/bottlerocket-settings-models-v0.16.0...bottlerocket-settings-models-v0.17.0 | ||
|
|
||
| ## [0.16.0] - 2025-09-19 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1492,6 +1492,54 @@ mod test_kubernetes_memory_swap_behavior { | |
|
|
||
| // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= | ||
|
|
||
| /// KubernetesIdsPerPodValue represents an integer that contains a valid Kubernetes idsPerPod value. | ||
| /// Must be a multiple of 65536 and less than 1<<32. | ||
| #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)] | ||
| #[serde(try_from = "i64", into = "i64")] | ||
| pub struct KubernetesIdsPerPodValue { | ||
| inner: i64, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason why we are not using u32 instead of i64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked the other integer settings we have, and I saw that historically we have matched the kubelet config docs. For IdsPerPod, it's listed as int64. |
||
| } | ||
|
|
||
| impl TryFrom<i64> for KubernetesIdsPerPodValue { | ||
| type Error = error::Error; | ||
|
|
||
| fn try_from(input: i64) -> Result<Self, Self::Error> { | ||
| ensure!( | ||
| input % 65536 == 0 && input < (1i64 << 32), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add a note on this validation for future references? It would be great If you can find the code in the kubelet that performs that validation. |
||
| error::InvalidKubernetesIdsPerPodValueSnafu { input } | ||
| ); | ||
| Ok(KubernetesIdsPerPodValue { inner: input }) | ||
| } | ||
| } | ||
|
|
||
| impl From<KubernetesIdsPerPodValue> for i64 { | ||
| fn from(val: KubernetesIdsPerPodValue) -> Self { | ||
| val.inner | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod test_kubernetes_ids_per_pod_value { | ||
| use super::KubernetesIdsPerPodValue; | ||
| use std::convert::TryFrom; | ||
|
|
||
| #[test] | ||
| fn good_values() { | ||
| for ok in &[0, 65536, 131072, 196608, 4294901760] { | ||
| KubernetesIdsPerPodValue::try_from(*ok).unwrap(); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn bad_values() { | ||
| for err in &[1, 65535, 65537, 4294967296] { | ||
| KubernetesIdsPerPodValue::try_from(*err).unwrap_err(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= | ||
|
|
||
| /// NvidiaDevicePluginSettings contains the device sharing and partitioning related settings for Nvidia gpu. | ||
| #[model(impl_default = true)] | ||
| pub struct NvidiaDevicePluginSettings { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -159,6 +159,9 @@ pub mod error { | |||||
|
|
||||||
| #[snafu(display("Invalid memory swap behavior value '{}'", input))] | ||||||
| InvalidMemorySwapBehavior { input: String }, | ||||||
|
|
||||||
| #[snafu(display("Invalid Kubernetes ids per pod value '{}'", input))] | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| InvalidKubernetesIdsPerPodValue { input: i64 }, | ||||||
| } | ||||||
|
|
||||||
| /// Creates a `ValidationError` with a consistent message for strings with regex validations | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [package] | ||
| name = "settings-extension-kubernetes" | ||
| version = "0.5.0" | ||
| version = "0.6.0" | ||
| authors = ["Sean P. Kelly <[email protected]>"] | ||
| license = "Apache-2.0 OR MIT" | ||
| edition = "2021" | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,11 +5,11 @@ use bottlerocket_modeled_types::{ | |||||||
| KubernetesAuthenticationMode, KubernetesBootstrapToken, KubernetesCPUManagerPolicyOption, | ||||||||
| KubernetesCloudProvider, KubernetesClusterDnsIp, KubernetesClusterName, | ||||||||
| KubernetesDurationValue, KubernetesEvictionKey, KubernetesHostnameOverrideSource, | ||||||||
| KubernetesLabelKey, KubernetesLabelValue, KubernetesMemoryManagerPolicy, | ||||||||
| KubernetesMemoryReservation, KubernetesMemorySwapBehavior, KubernetesQuantityValue, | ||||||||
| KubernetesReservedResourceKey, KubernetesTaintValue, KubernetesThresholdValue, | ||||||||
| NonNegativeInteger, SingleLineString, TopologyManagerPolicy, TopologyManagerScope, Url, | ||||||||
| ValidBase64, ValidLinuxHostname, | ||||||||
| KubernetesIdsPerPodValue, KubernetesLabelKey, KubernetesLabelValue, | ||||||||
| KubernetesMemoryManagerPolicy, KubernetesMemoryReservation, KubernetesMemorySwapBehavior, | ||||||||
| KubernetesQuantityValue, KubernetesReservedResourceKey, KubernetesTaintValue, | ||||||||
| KubernetesThresholdValue, NonNegativeInteger, SingleLineString, TopologyManagerPolicy, | ||||||||
| TopologyManagerScope, Url, ValidBase64, ValidLinuxHostname, | ||||||||
| }; | ||||||||
| use bottlerocket_settings_sdk::{GenerateResult, SettingsModel}; | ||||||||
|
|
||||||||
|
|
@@ -96,6 +96,8 @@ pub struct KubernetesSettingsV1 { | |||||||
| node_ip: IpAddr, | ||||||||
| pod_infra_container_image: SingleLineString, | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This commit is missing its |
||||||||
| hostname_override: ValidLinuxHostname, | ||||||||
| ids_per_pod: KubernetesIdsPerPodValue, | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We use i64 directly in the shape. bottlerocket-settings-sdk/bottlerocket-settings-models/settings-extensions/kubernetes/src/lib.rs Line 74 in 9306966
For consistency, let's just do:
Suggested change
And we can drop the custom model type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This API has a restriction on the value so the type seems right:
|
||||||||
| max_parallel_image_pulls: i32, | ||||||||
| } | ||||||||
|
|
||||||||
| type Result<T> = std::result::Result<T, Infallible>; | ||||||||
|
|
@@ -199,6 +201,8 @@ mod test { | |||||||
| device_ownership_from_security_context: None, | ||||||||
| single_process_oom_kill: None, | ||||||||
| static_pods_enabled: None, | ||||||||
| ids_per_pod: None, | ||||||||
| max_parallel_image_pulls: None, | ||||||||
| }) | ||||||||
| ); | ||||||||
| } | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [package] | ||
| name = "bottlerocket-settings-models" | ||
| version = "0.16.0" | ||
| version = "0.17.0" | ||
| authors = ["Tom Kirchner <[email protected]>"] | ||
| license = "Apache-2.0 OR MIT" | ||
| edition = "2021" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit is missing its signed-off tag