feat(cluster): add terminatedPodGcThreshold (podGcControllerConfig) support - #252
Conversation
ack-prow
Bot
added
the
needs-ok-to-test
|
Hi @chithreshazad. Thanks for your PR. I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
| kubeControllerManagerConfig: | ||
| horizontalPodAutoscalerControllerConfig: | ||
| horizontalPodAutoscalerSyncPeriod: "$HPA_SYNC_PERIOD" | ||
| # terminatedPodGcThreshold is valid only on PCP (Provisioned Control Plane) |
There was a problem hiding this comment.
If that's the case, you'll need to update this manifest with the PCP configuration, otherwise the e2e tests might fail.
There was a problem hiding this comment.
The test is already using the PCP configuration. The test set the controlPlaneScalingConfig to tier-xl which is a PCP tier.
controlPlaneScalingConfig:
tier: "tier-xl"
|
/ok-to-test |
ack-prow
Bot
added
ok-to-test
michaelhtm
left a comment
michaelhtm
left a comment
There was a problem hiding this comment.
Thanks @chithreshazad
left two comments below
|
/label release/minor |
ack-prow
Bot
added
the
release/minor
Adds a `Gc` -> `GC` initialism so garbage-collection field names render the abbreviation uppercased, consistent with existing AWS abbreviations (DB, AZ, ARN, ...). ### Motivation The EKS controller is adding `controlPlaneComponentConfig.kubeControllerManagerConfig.podGcControllerConfig.terminatedPodGcThreshold` (aws-controllers-k8s/eks-controller#252). Without this rule ACK renders `podGcControllerConfig` / `terminatedPodGcThreshold`; with it they become `podGCControllerConfig` / `terminatedPodGCThreshold`. ### Safety / scope The rule matches `Gc` only when an uppercase letter follows (`Gc(?=[A-Z])`), so it does **not** touch `Gcp` (DMS `GcpMySQLSettings` = Google Cloud Platform), `Gcm` (Pinpoint/Chime GCM channels, KMS AES-GCM), `Gce`, or `Gcs`. Scanned all 432 services in `aws-sdk-go-v2`: the only real API-model fields with a `Gc`+uppercase token are the EKS podGc family. The only other matches are randomized protocol-test fixtures under `service/internal` (not a generated service), so there is no fleet-wide impact. ### Testing `go test ./names/names_test.go` passes, including new cases for the uppercased podGc fields and `Gcp`/`Gcm` guards.
065f0cb to
13ae600
Compare
|
/retest-required |
|
@chithreshazad these failures are mainly caused by your go version. Can you try updating yours to 1.27 and regenerate the controller? |
…upport Exposes KubeControllerManagerConfig.podGCControllerConfig.terminatedPodGCThreshold (new in aws-sdk-go-v2/service/eks v1.95.1) on the Cluster resource. - generator.yaml: un-ignore the field and add nested late_initialize (skip_incomplete_check) for PodGCControllerConfig and its TerminatedPodGCThreshold leaf. The backend injects tier defaults on read, so nested late-init prevents false drift on the sibling/parent fields. - Regenerated with code-generator main, which includes the "GC" garbage-collection initialism (aws-controllers-k8s/pkg#51 -> pkg v0.0.27 -> code-generator#741), so the k8s-facing fields render as podGCControllerConfig / terminatedPodGCThreshold. (The AWS API wire name remains terminatedPodGcThreshold.) - No hook change: updates flow through the existing updateComponentConfig, which sends the whole KubeControllerManagerConfig via the regenerated newCreateRequestPayload when Spec.KubeControllerManagerConfig differs; delta.go compares the field. - e2e: create sets terminatedPodGCThreshold=12500 and asserts it via describe (AWS wire name terminatedPodGcThreshold), updates the CR to 12000, re-asserts. Uses tier-xl (valid only on PCP clusters, range 10000-12500).
13ae600 to
94a268a
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chithreshazad, michaelhtm The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
Description of changes:
Exposes
KubeControllerManagerConfig.podGcControllerConfig.terminatedPodGcThreshold(new inaws-sdk-go-v2/service/eks v1.95.1) on the Cluster resource.generator.yaml: un-ignore the field and add nested late_initialize (skip_incomplete_check) forPodGcControllerConfigand itsTerminatedPodGcThresholdleaf, matching the existing component-config pattern. The backend injects tier defaults on read, so nested late-init prevents false drift on the sibling/parent fields.updateComponentConfig, which sends the wholeKubeControllerManagerConfig(now including podGc) via the regeneratednewCreateRequestPayloadwheneverSpec.KubeControllerManagerConfigdiffers; delta.go now compares the field.terminatedPodGcThreshold=12500, update to12000, re-assert. Usestier-xl(terminatedPodGcThresholdis valid only on PCP clusters, range10000-12500).CertificateAuthorityandCluster.certificateAuthority.activeremain ignored (separate features).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.