ROSAENG-60886: Migrate from EKS Auto Mode to OSS Karpenter - #727
Closed
theautoroboto wants to merge 19 commits into
Closed
ROSAENG-60886: Migrate from EKS Auto Mode to OSS Karpenter#727theautoroboto wants to merge 19 commits into
theautoroboto wants to merge 19 commits into
Conversation
Replaces EKS Auto Mode with OSS Karpenter across all EKS clusters. Includes boot ordering fixes (HyperShift CRD wait, hypershift Application health gate), external-dns fixes (crash fix, CriticalAddonsOnly toleration, remove readiness wait), TargetGroupBinding API version corrections, and e2e test timeout tuning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix ami_kms_key_arn description (kms:CreateGrant + kms:DescribeKey on
controller role only; remove incorrect kms:Decrypt and node-role claims)
- Remove false admission webhook claim from karpenter-node-provisioning.md
- Remove IRSA deprecated claim from zoa-trusted-actions.md
- PIPELINE_COMPLETION_TIMEOUT: 5400 → 4500 (90 min → 75 min)
- register.sh MAX_RETRIES: 80 → 10 (revert Karpenter migration increase)
- Delete unused validate-{mc,rc}-{aws,k8s}.sh scripts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ctions The import used `2>/dev/null || true`, which swallowed all errors. When the import failed (e.g. state lock, resource already managed by a concurrent run sharing the same state key), execution continued to `terraform apply`, which found no connection in state and created a new PENDING one — requiring manual re-authorization on every affected run. Now the import output is captured and re-emitted on failure. The only tolerated failure is "Resource already managed" (idempotent import); all other errors exit non-zero with a visible message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ar connections" This reverts commit dc23021.
This reverts commit c5512dd.
…ng middleware" This reverts commit f32a48a.
Drop all temporary [DNS-DEBUG] and [DIAG] blocks added during external-dns root cause investigation: - provision-infra-rc.sh: DNS-DEBUG echo blocks around terraform apply - bastion/log-collection-task.tf: VPC-side DNS diag block, BASE_DOMAIN env var, and route53-read IAM policy - ecs-bootstrap/main.tf: periodic hypershift-install Job/pod dump in the ArgoCD health wait loop Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The readyReplicas check skipped reinstall only when the deployment was healthy, causing Karpenter to be reinstalled on any partial/pending state. Checking helm release status is the correct idempotency boundary — if the release is already deployed, skip. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Always-applying the NodePool conflicts with ArgoCD's SSA ownership on resync runs. The correct pattern is to seed once on first boot and let ArgoCD own the resource thereafter, matching main branch behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-warming a pause pod to surface EC2 API rate limiting is a CI concern, not a bootstrap responsibility. App pod scheduling naturally triggers Karpenter node provisioning. The 8-minute wait added unnecessary latency to every bootstrap run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ecovery and annotation re-stamping Both the broken-release recovery and annotation re-stamping only exist because ArgoCD was always upgraded via helm upgrade --install on every bootstrap run. Restoring skip-if-exists (matching main branch) makes those steps unreachable: a clean first install never ends in failed state, and Helm annotations are never stripped because ArgoCD does not run a competing upgrade on resources it does not yet own. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All ArgoCD component tolerations are already defined in values.yaml and applied automatically by helm install. The 27 redundant --set flags were duplicating that config. Only redisSecretInit tolerations remain as --set flags because that job is not covered by the ArgoCD chart's default toleration keys. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The wait is not a fundamental bootstrap responsibility — it exists because the E2E test runner starts immediately after bootstrap exits. Label it explicitly so future readers don't treat it as a correctness requirement for production deployments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The HCP e2e test fails with "no such host" for the cluster API hostname when external-dns hasn't written the Route53 A record yet. Add a diag_dns() function called pre-test and post-failure to surface: - external-dns pod state and recent log lines (errors/warnings) - DNSEndpoint CRs on the MC (confirms CPO output) - Pod Identity associations for the external-dns SA - Route53 hosted zones in the RC account - NS delegation for the base domain - Targeted A record probe for the specific cluster API host (on failure) This distinguishes IAM failures, missing DNSEndpoint CRs, Route53 zone misconfiguration, and pure propagation timing without requiring SSH access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ndling - Switch command shell from /bin/sh to /bin/bash - Log hypershift install start and exit code for easier CI triage - Log hypershift namespace HTTP check result - Change external-dns deployment and clusterrole patches from curl -sf (fail-fast) to curl -s with explicit HTTP code capture; log a WARNING on non-200/201 rather than aborting the Job — patch failures are non-fatal when the namespace already exists - Strip stale "(Auto Mode)" text from Valkey security group descriptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
validate-rc-k8s.sh and validate-mc-k8s.sh (and their AWS counterparts) require kubectl access to private EKS endpoints that Prow cannot reach. Remove all four scripts and the k8s validation block in ci/e2e-tests.sh that called them, fixing the validate=1 CI failure on PR openshift-online#698. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The hypershift-install Job was stuck in CreateContainerConfigError because it referenced env vars from Secret hypershift-config-env, which is only created by the CSI Secrets Store driver after a pod successfully mounts the CSI volume. Kubernetes validates Secret references before starting the pod, creating a deadlock: pod won't start without the Secret, Secret won't be created without the pod mounting the volume. Fix by reading OIDC configuration directly from CSI-mounted files (/mnt/secrets-store/*) instead of relying on the Kubernetes Secret object. The pod can now start (no Secret reference to validate), mount the CSI volume, read the files, and proceed. Changes: - Remove env vars OIDC_BUCKET_NAME, OIDC_BUCKET_REGION, OIDC_WRITER_ROLE_ARN that referenced secretKeyRef hypershift-config-env - Add shell commands to read values from CSI-mounted files at script start - Update hypershift install command to use shell variables instead of command substitution syntax Fixes: bda1a13 (feat: refactor OIDC S3/KMS access to assume-role pattern) Resolves: Management Cluster provision timeout in ephemeral CI runs Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
Collaborator
|
@theautoroboto: This pull request references ROSAENG-60886 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (73)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📝 WalkthroughWalkthroughThe change migrates EKS clusters from Auto Mode to OSS Karpenter, adds AWS Load Balancer Controller provisioning, updates bootstrap and Argo CD behavior, changes TargetGroupBinding APIs, and expands CI validation, diagnostics, and documentation. ChangesKarpenter infrastructure and bootstrap
AWS Load Balancer Controller and platform integration
Validation and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
|
|
@theautoroboto: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
kms:CreateGrantfrom node role to Karpenter controller rolekubectlnot in hypershift-operator image; replaced withcurlagainst in-cluster API servertls.enabledmust be disabled alongsideadmissionWebhooks.enabledto avoid missing TLS secret mount crashSupersedes #698 (CI integration broke after history squash).
Test plan
/test on-demand-e2emc01-bootstrapshows=== Prometheus Operator CRDs present — proceeding with hypershift install ===🤖 Generated with Claude Code
Summary by CodeRabbit