ROSAENG-60886: Migrate from EKS Auto Mode to OSS Karpenter - #698
Open
theautoroboto wants to merge 34 commits into
Open
ROSAENG-60886: Migrate from EKS Auto Mode to OSS Karpenter#698theautoroboto wants to merge 34 commits into
theautoroboto wants to merge 34 commits into
Conversation
openshift-ci
Bot
added
the
needs-rebase
|
[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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR migrates EKS provisioning to OSS Karpenter, adds Karpenter-aware bootstrap and validation flows, introduces AWS Load Balancer Controller infrastructure, updates Kubernetes integrations and Argo CD drift handling, and expands design and module documentation. ChangesKarpenter platform migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
Author
|
/test on-demand-e2e |
theautoroboto
marked this pull request as draft
July 23, 2026 16:41
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
Author
|
/test on-demand-e2e |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
terraform/modules/eks-cluster/iam.tf (1)
262-289: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRemove the unused instance-profile write permissions.
EC2NodeClass.spec.instanceProfileis used in both nodeclass templates, and this policy already assumes the profile is pre-created; keepiam:GetInstanceProfile,iam:ListInstanceProfiles, andiam:PassRole, but dropiam:CreateInstanceProfile,iam:TagInstanceProfile,iam:AddRoleToInstanceProfile,iam:DeleteInstanceProfile, andiam:RemoveRoleFromInstanceProfile.terraform/modules/eks-cluster/iam.tf:262-289🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terraform/modules/eks-cluster/iam.tf` around lines 262 - 289, Remove the IAMInstanceProfileCreate and IAMInstanceProfileModify statements from the policy, eliminating CreateInstanceProfile, TagInstanceProfile, AddRoleToInstanceProfile, DeleteInstanceProfile, and RemoveRoleFromInstanceProfile permissions. Preserve the existing iam:GetInstanceProfile, iam:ListInstanceProfiles, and iam:PassRole permissions.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@argocd/config/management-cluster/eks-nodepool/templates/00-nodeclass.yaml`:
- Around line 7-8: Update the amiSelectorTerms configuration in the NodeClass to
use the bottlerocket-fips@latest alias instead of bottlerocket@latest,
preserving the existing AMI selector structure.
In `@argocd/config/regional-cluster/eks-nodepool/templates/00-nodeclass.yaml`:
- Around line 1-18: Update the EC2NodeClass resource named fips so
amiSelectorTerms selects the approved FIPS Bottlerocket AMI instead of the
standard bottlerocket@latest alias. Preserve the existing FIPS pool contract and
leave the cluster, subnet, security group, and metadata settings unchanged.
In `@argocd/config/shared/argocd/values.yaml`:
- Around line 172-185: Update the redis-ha configuration block in values.yaml to
explicitly set redis-ha.enabled to true, and verify its HA replica settings
remain aligned with the intended Argo CD HA configuration while preserving the
existing tolerations.
In `@argocd/config/shared/storageclass/templates/gp3.yaml`:
- Line 7: Replace the existing gp3 StorageClass rather than mutating its
immutable provisioner: add a new StorageClass with a distinct name using
ebs.csi.aws.com, update management and regional consumers/defaulting to
reference it, and retain the old class until the new driver is available before
retiring it.
In `@docs/design/fips-eks-compute.md`:
- Around line 77-79: Revise the compliance statement in the Bottlerocket
workload description to scope it to node-level FIPS readiness and FIPS-validated
cryptographic modules. Remove the implication that this alone satisfies FedRAMP
High/Moderate requirements for the cluster or customer-bearing workloads, unless
the statement explicitly references the broader required control set.
In `@docs/design/karpenter-node-provisioning.md`:
- Around line 27-32: Update the rationale in
docs/design/karpenter-node-provisioning.md:27-32 to remove the claim that Pod
Identity requires a separate admission webhook, retaining only the bootstrap and
compatibility reasons for choosing IRSA. Also update
docs/design/zoa-trusted-actions.md:816 to state that IRSA remains supported and
compare Pod Identity as an option that primarily avoids per-cluster OIDC
provider management; do not describe IRSA as deprecated.
In `@docs/design/rate-limiting-architecture.md`:
- Line 242: Update the Provisioned Concurrency note so each struck-through value
uses its own matched Markdown delimiters: `~~200 req/s~~` for traffic and
`~~$30-45/month~~` for cost. Keep the surrounding explanation unchanged.
In `@scripts/validate-rc-aws.sh`:
- Around line 135-151: Update the node-group validation flow around the
ng_desired and ng_ready queries to run these detail queries only when the node
group is confirmed ACTIVE, matching the guarded structure in validate-mc-aws.sh.
Preserve safe fallback handling for AWS or jq failures so missing or unavailable
node groups do not abort the script or leave ng_ready empty before the numeric
comparison.
In `@terraform/modules/aws-load-balancer-controller/README.md`:
- Around line 14-16: Update the upstream recommended policy URL in the README
IAM role description to reference version v2.17.1, matching the versions used by
iam.tf and Chart.yaml.
In `@terraform/modules/ecs-bootstrap/README.md`:
- Line 77: Update the karpenter_version entry in the Inputs table to document
the module’s actual default of "1.13.0", matching the default defined by the
karpenter_version variable.
In `@terraform/modules/eks-cluster/README.md`:
- Line 102: Update the ami_kms_key_arn documentation row in the module README so
kms:CreateGrant is described as granted only to the Karpenter controller role,
while preserving the existing kms:Decrypt role description.
In `@terraform/modules/eks-cluster/variables.tf`:
- Around line 84-88: Update the ami_kms_key_arn variable description to
accurately state that setting it adds the implemented KMS permissions to the
Karpenter controller role only, specifically kms:CreateGrant and
kms:DescribeKey, and remove claims about kms:Decrypt or node-role permissions.
---
Nitpick comments:
In `@terraform/modules/eks-cluster/iam.tf`:
- Around line 262-289: Remove the IAMInstanceProfileCreate and
IAMInstanceProfileModify statements from the policy, eliminating
CreateInstanceProfile, TagInstanceProfile, AddRoleToInstanceProfile,
DeleteInstanceProfile, and RemoveRoleFromInstanceProfile permissions. Preserve
the existing iam:GetInstanceProfile, iam:ListInstanceProfiles, and iam:PassRole
permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0775ea04-2597-48b3-ad3f-5c3825efa1a2
📒 Files selected for processing (60)
Makefileargocd/config/management-cluster/eks-nodepool/templates/00-nodeclass.yamlargocd/config/management-cluster/eks-nodepool/templates/10-nodepool.yamlargocd/config/management-cluster/eks-nodepool/values.yamlargocd/config/management-cluster/hypershift/templates/05-job.yamlargocd/config/management-cluster/monitoring/values.yamlargocd/config/regional-cluster/aws-load-balancer-controller/Chart.yamlargocd/config/regional-cluster/aws-load-balancer-controller/values.yamlargocd/config/regional-cluster/eks-nodepool/templates/00-nodeclass.yamlargocd/config/regional-cluster/eks-nodepool/templates/10-nodepool.yamlargocd/config/regional-cluster/eks-nodepool/values.yamlargocd/config/regional-cluster/loki/templates/targetgroupbinding.yamlargocd/config/regional-cluster/platform-api/templates/targetgroupbinding.yamlargocd/config/regional-cluster/thanos/templates/targetgroupbinding.yamlargocd/config/shared/argocd/values.yamlargocd/config/shared/storageclass/templates/gp3.yamlci/ephemeral-provider/__init__.pyconfig/templates/argocd-bootstrap/applicationset.yaml.j2deploy/ephemeral/us-east-1/argocd-bootstrap-management-cluster/applicationset.yamldeploy/ephemeral/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yamldeploy/integration/us-east-1/argocd-bootstrap-management-cluster/applicationset.yamldeploy/integration/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yamldocs/README.mddocs/design/fips-eks-compute.mddocs/design/fully-private-eks-bootstrap.mddocs/design/gitops-cluster-configuration.mddocs/design/karpenter-node-provisioning.mddocs/design/logging-platform.mddocs/design/rate-limiting-architecture.mddocs/design/thanos-metrics-infrastructure.mddocs/design/zoa-trusted-actions.mdscripts/buildspec/provision-infra-mc.shscripts/buildspec/register.shscripts/validate-mc-aws.shscripts/validate-mc-k8s.shscripts/validate-rc-aws.shscripts/validate-rc-k8s.shterraform/config/management-cluster/main.tfterraform/config/pipeline-management-cluster/main.tfterraform/config/pipeline-regional-cluster/main.tfterraform/config/regional-cluster/imports.shterraform/config/regional-cluster/main.tfterraform/modules/aws-load-balancer-controller/README.mdterraform/modules/aws-load-balancer-controller/iam.tfterraform/modules/aws-load-balancer-controller/main.tfterraform/modules/aws-load-balancer-controller/outputs.tfterraform/modules/aws-load-balancer-controller/variables.tfterraform/modules/aws-load-balancer-controller/versions.tfterraform/modules/ecs-bootstrap/README.mdterraform/modules/ecs-bootstrap/main.tfterraform/modules/ecs-bootstrap/variables.tfterraform/modules/eks-cluster/README.mdterraform/modules/eks-cluster/data.tfterraform/modules/eks-cluster/iam.tfterraform/modules/eks-cluster/locals.tfterraform/modules/eks-cluster/main.tfterraform/modules/eks-cluster/outputs.tfterraform/modules/eks-cluster/variables.tfterraform/modules/eks-cluster/versions.tfterraform/modules/hyperfleet-infrastructure/amazonmq.tf
theautoroboto
force-pushed
the
remove-auto-mode-v2
branch
from
July 23, 2026 16:55
98def0f to
23b650e
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
/test on-demand-e2e |
2 similar comments
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
theautoroboto
marked this pull request as ready for review
July 23, 2026 19:51
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
Author
|
/test on-demand-e2e |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/zoa-trusted-actions.md`:
- Line 825: Update the IRSA rationale in the trusted-actions design section to
remove the claim that IRSA is deprecated. State instead that the platform
standardizes on EKS Pod Identity for new workload service accounts and avoids
per-cluster OIDC provider management, while preserving the existing comparison
context.
In `@scripts/validate-mc-k8s.sh`:
- Around line 138-184: Qualify all HyperShift NodePool kubectl resource
references in the NodePool validation block: update the list command, per-object
get commands, and process-substitution command to use
nodepools.hypershift.openshift.io, while preserving the existing counting and
readiness checks.
In `@terraform/modules/eks-cluster/iam.tf`:
- Around line 395-409: Add an aws:SourceArn condition to the AllowEventBridge
statement in aws_sqs_queue_policy.karpenter_interruption, restricting
sqs:SendMessage to the Karpenter EventBridge rule ARN(s); optionally also
constrain aws:SourceAccount. Reuse the existing Karpenter EventBridge rule
resource symbols and preserve the queue policy’s enablement and permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ece5d3d8-316a-4df0-98b5-a9949422062f
📒 Files selected for processing (57)
Makefileargocd/config/management-cluster/eks-nodepool/templates/00-nodeclass.yamlargocd/config/management-cluster/eks-nodepool/templates/10-nodepool.yamlargocd/config/management-cluster/eks-nodepool/values.yamlargocd/config/management-cluster/hypershift/templates/05-job.yamlargocd/config/management-cluster/monitoring/values.yamlargocd/config/regional-cluster/aws-load-balancer-controller/Chart.yamlargocd/config/regional-cluster/aws-load-balancer-controller/values.yamlargocd/config/regional-cluster/eks-nodepool/templates/00-nodeclass.yamlargocd/config/regional-cluster/eks-nodepool/templates/10-nodepool.yamlargocd/config/regional-cluster/eks-nodepool/values.yamlargocd/config/regional-cluster/loki/templates/targetgroupbinding.yamlargocd/config/regional-cluster/platform-api/templates/targetgroupbinding.yamlargocd/config/regional-cluster/thanos/templates/targetgroupbinding.yamlargocd/config/shared/argocd/values.yamlargocd/config/shared/storageclass/templates/gp3.yamlci/ephemeral-provider/__init__.pyconfig/templates/argocd-bootstrap/applicationset.yaml.j2deploy/ephemeral/us-east-1/argocd-bootstrap-management-cluster/applicationset.yamldeploy/ephemeral/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yamldeploy/integration/us-east-1/argocd-bootstrap-management-cluster/applicationset.yamldeploy/integration/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yamldocs/README.mddocs/design/fips-eks-compute.mddocs/design/fully-private-eks-bootstrap.mddocs/design/karpenter-node-provisioning.mddocs/design/logging-platform.mddocs/design/thanos-metrics-infrastructure.mddocs/design/zoa-trusted-actions.mdscripts/buildspec/provision-infra-mc.shscripts/buildspec/register.shscripts/validate-mc-aws.shscripts/validate-mc-k8s.shscripts/validate-rc-aws.shscripts/validate-rc-k8s.shterraform/config/management-cluster/main.tfterraform/config/pipeline-management-cluster/main.tfterraform/config/pipeline-regional-cluster/main.tfterraform/config/regional-cluster/imports.shterraform/config/regional-cluster/main.tfterraform/modules/aws-load-balancer-controller/README.mdterraform/modules/aws-load-balancer-controller/iam.tfterraform/modules/aws-load-balancer-controller/main.tfterraform/modules/aws-load-balancer-controller/outputs.tfterraform/modules/aws-load-balancer-controller/variables.tfterraform/modules/aws-load-balancer-controller/versions.tfterraform/modules/ecs-bootstrap/README.mdterraform/modules/ecs-bootstrap/main.tfterraform/modules/ecs-bootstrap/variables.tfterraform/modules/eks-cluster/README.mdterraform/modules/eks-cluster/data.tfterraform/modules/eks-cluster/iam.tfterraform/modules/eks-cluster/locals.tfterraform/modules/eks-cluster/main.tfterraform/modules/eks-cluster/outputs.tfterraform/modules/eks-cluster/variables.tfterraform/modules/eks-cluster/versions.tf
🚧 Files skipped from review as they are similar to previous changes (46)
- argocd/config/regional-cluster/platform-api/templates/targetgroupbinding.yaml
- terraform/modules/aws-load-balancer-controller/versions.tf
- argocd/config/regional-cluster/eks-nodepool/values.yaml
- argocd/config/regional-cluster/eks-nodepool/templates/10-nodepool.yaml
- argocd/config/management-cluster/eks-nodepool/values.yaml
- argocd/config/management-cluster/eks-nodepool/templates/10-nodepool.yaml
- ci/ephemeral-provider/init.py
- terraform/config/pipeline-regional-cluster/main.tf
- terraform/modules/eks-cluster/locals.tf
- terraform/config/pipeline-management-cluster/main.tf
- terraform/config/management-cluster/main.tf
- Makefile
- config/templates/argocd-bootstrap/applicationset.yaml.j2
- terraform/modules/aws-load-balancer-controller/main.tf
- argocd/config/regional-cluster/aws-load-balancer-controller/values.yaml
- docs/design/logging-platform.md
- deploy/integration/us-east-1/argocd-bootstrap-management-cluster/applicationset.yaml
- deploy/ephemeral/us-east-1/argocd-bootstrap-management-cluster/applicationset.yaml
- argocd/config/shared/storageclass/templates/gp3.yaml
- terraform/modules/aws-load-balancer-controller/variables.tf
- docs/README.md
- terraform/modules/aws-load-balancer-controller/outputs.tf
- docs/design/thanos-metrics-infrastructure.md
- terraform/modules/eks-cluster/data.tf
- terraform/modules/aws-load-balancer-controller/README.md
- argocd/config/management-cluster/monitoring/values.yaml
- deploy/integration/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yaml
- terraform/modules/eks-cluster/versions.tf
- argocd/config/regional-cluster/aws-load-balancer-controller/Chart.yaml
- terraform/modules/ecs-bootstrap/variables.tf
- deploy/ephemeral/us-east-1/argocd-bootstrap-regional-cluster/applicationset.yaml
- argocd/config/regional-cluster/thanos/templates/targetgroupbinding.yaml
- terraform/modules/eks-cluster/variables.tf
- terraform/config/regional-cluster/main.tf
- argocd/config/shared/argocd/values.yaml
- argocd/config/regional-cluster/loki/templates/targetgroupbinding.yaml
- terraform/modules/eks-cluster/outputs.tf
- scripts/validate-mc-aws.sh
- terraform/modules/eks-cluster/main.tf
- terraform/modules/aws-load-balancer-controller/iam.tf
- docs/design/fips-eks-compute.md
- terraform/modules/ecs-bootstrap/main.tf
- scripts/validate-rc-aws.sh
- docs/design/karpenter-node-provisioning.md
- argocd/config/management-cluster/hypershift/templates/05-job.yaml
- terraform/modules/eks-cluster/README.md
theautoroboto
changed the title
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. |
Contributor
Author
|
/test on-demand-e2e |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/ephemeral-provider/orchestrator.py`:
- Around line 233-244: Update the docstring describing the override-directory
handling to reflect the logic around load_and_merge: region YAML files in
config/ephemeral are replaced, while defaults.yaml is retained and merged with
the override to preserve existing environment settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 966663f8-4dcf-4747-9d6a-905bda0cf917
📒 Files selected for processing (1)
ci/ephemeral-provider/orchestrator.py
Contributor
Author
|
/test on-demand-e2e |
openshift-ci
Bot
added
the
needs-rebase
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
argocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml (1)
2-2: 📐 Maintainability & Code Quality | 🔵 TrivialValidate the shared rendered configuration.
Run Helm lint and validate rendered output for both management and regional rendering contexts before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@argocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml` at line 2, Validate the shared rendered configuration for sre-targetgroupbinding by running Helm lint and checking the rendered output in both management and regional rendering contexts before merging.Source: Coding guidelines
argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yaml (1)
2-2: 📐 Maintainability & Code Quality | 🔵 TrivialRun Helm lint and validate rendered configuration for every changed chart.
argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yaml#L2-L2: lint and render the Grafana chart.argocd/config/regional-cluster/loki/templates/targetgroupbinding.yaml#L2-L2: lint and render the Loki chart.argocd/config/regional-cluster/loki/templates/targetgroupbinding.yaml#L20-L20: validate the query-frontend rendering.argocd/config/regional-cluster/loki/templates/targetgroupbinding.yaml#L38-L38: validate the SRE query-frontend rendering.argocd/config/regional-cluster/monitoring/templates/sre-targetgroupbinding.yaml#L2-L2: lint and render the monitoring chart.argocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml#L2-L2: validate both management and regional renderings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yaml` at line 2, Run Helm lint and validate rendered output for every affected chart: Grafana at argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yaml:2-2; Loki at argocd/config/regional-cluster/loki/templates/targetgroupbinding.yaml:2-2, including query-frontend at :20-20 and SRE query-frontend at :38-38; monitoring at argocd/config/regional-cluster/monitoring/templates/sre-targetgroupbinding.yaml:2-2; and both management and regional renderings for argocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml:2-2. Fix any lint or rendering errors exposed by these validations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yaml`:
- Line 2: Run Helm lint and validate rendered output for every affected chart:
Grafana at
argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yaml:2-2;
Loki at
argocd/config/regional-cluster/loki/templates/targetgroupbinding.yaml:2-2,
including query-frontend at :20-20 and SRE query-frontend at :38-38; monitoring
at
argocd/config/regional-cluster/monitoring/templates/sre-targetgroupbinding.yaml:2-2;
and both management and regional renderings for
argocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml:2-2. Fix any
lint or rendering errors exposed by these validations.
In `@argocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml`:
- Line 2: Validate the shared rendered configuration for sre-targetgroupbinding
by running Helm lint and checking the rendered output in both management and
regional rendering contexts before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 09a325f2-c261-44a8-a248-39f9ce5e3f17
📒 Files selected for processing (5)
argocd/config/regional-cluster/grafana/templates/sre-targetgroupbinding.yamlargocd/config/regional-cluster/loki/templates/targetgroupbinding.yamlargocd/config/regional-cluster/monitoring/templates/sre-targetgroupbinding.yamlargocd/config/regional-cluster/thanos/templates/targetgroupbinding.yamlargocd/config/shared/argocd/templates/sre-targetgroupbinding.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- argocd/config/regional-cluster/thanos/templates/targetgroupbinding.yaml
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
/test on-demand-e2e |
3 similar comments
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
Resolves conflicts between the Karpenter migration work on this branch and the ElastiCache Valkey rate-limiting additions merged to main. Both sets of changes are independent: kept all Karpenter env vars/variables alongside the new REDIS_ENDPOINT env var, redis_endpoint variable, and rc_aws_account_id / redis_endpoint module inputs in regional-cluster main.tf. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
/test on-demand-e2e |
Adds temporary diagnostic instrumentation (marked TODO(dns-troubleshooting)) to identify the root cause of missing HCP API DNS records in e2e tests: - ci/e2e-tests.sh: Route53 background poller (30s interval, A record timeline), CloudTrail queries for ChangeResourceRecordSets and AssumeRole to dns-zone-operator, pre/post-failure diag_dns() calls - scripts/dev/collect-cluster-logs.sh: thread DIAG_BASE_DOMAIN through to ECS log-collector task as BASE_DOMAIN - terraform/modules/bastion/log-collection-task.tf: DNS probe from inside VPC (Route53 A records + dig NS delegation), BASE_DOMAIN env var, route53:List* IAM policy on log-collector role - terraform/modules/ecs-bootstrap/main.tf: expand hypershift-install DIAG block to show logs from all pods (not just first) and add external-dns pod status + logs - argocd/config/management-cluster/hypershift/templates/05-job.yaml: show external-dns pod phase/reason/message at the error exit path All diagnostic blocks are marked for removal before merging to main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The wait was blocking MC bootstrap when external-dns failed to reach availableReplicas >= 1 within 5 minutes. Removing the gate unblocks provisioning; external-dns readiness is addressed separately. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
/test on-demand-e2e |
The Red Hat external-dns image (registry.redhat.io/edo/external-dns-rhel9) does not have a binary at /ko-app/external-dns — that path is specific to the upstream ko-built image that was replaced in c68f7fc. commit 668d19e inadvertently re-introduced this command replacement when adding the CriticalAddonsOnly toleration patch. The override caused external-dns to crash immediately on start (exec not found), which meant no A records were ever written to Route53 shard zones, leaving HCP clusters stuck in Provisioning indefinitely. Keep the --aws-assume-role arg injection and CriticalAddonsOnly toleration; remove only the command replacement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
/test on-demand-e2e |
The existing diag_dns() only checked RC account CloudTrail for external-dns Route53 activity. But external-dns first assumes a Pod Identity role in the MC account before cross-account assuming the RC dns-zone-operator role. If external-dns is not running (root cause A) or Pod Identity is misconfigured, the RC CloudTrail stays silent — which matches the current CI failure pattern (25+ min, zero A records, cert-manager is the only actor in the RC shard zone). The new check looks for AssumeRoleWithWebIdentity events on *dns-operator roles in the MC account: - Empty → pod is dead (confirms root cause A) - AccessDenied → Pod Identity association missing or wrong role - OK → pod is running; investigate DNSEndpoint CRs or the RC cross-account step Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
/test on-demand-e2e |
Root cause (go directive version mismatch) was fixed in rosa-hyperfleet-api. Removing the diag_dns() function, Route53 background poller, and all associated TODO(dns-troubleshooting) blocks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
/test on-demand-e2e |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
/test on-demand-e2e |
- 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>
openshift-ci
Bot
added
the
needs-rebase
Contributor
Author
|
/test on-demand-e2e |
# Conflicts: # scripts/dev/dump-env.sh
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
/test on-demand-e2e |
3 similar comments
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
…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>
Contributor
Author
|
/test on-demand-e2e |
2 similar comments
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test on-demand-e2e |
Contributor
Author
|
/test images |
Contributor
Author
|
/test on-demand-e2e |
|
@theautoroboto: The following test 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. |
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 rolekubectlis not in the hypershift-operator image; replaced withcurlagainst the in-cluster API serveradmissionWebhooks.enabled: falseskips the certgen Job buttls.enableddefaults totrue, crashing PrometheusOperator on the missing TLS secret mount; now sets both tofalseRoot causes fixed (from CI run 2080263806742695936)
Job CRD wait: The
hypershift-installJob pod usesquay.io/cbusse_openshift/hypershift-operator:aws-iam-authwhich does not includekubectl. The original CRD wait loop silently failed every iteration, the Job timed out, and hypershift never installed. The ECS bootstrap (which runs on a different container withkubectl) could see the CRDs fine — the Job pod could not.monitoring TLS:
kube-prometheus-stack72.6.2 gates thetls-secretvolume mount onprometheusOperator.tls.enabled(notadmissionWebhooks.enabled). Disabling admission webhooks stops the certgen Job from creating the secret, while the Deployment still tries to mount it → pod crash → monitoring Degraded → hypershift sync blocks.Test plan
on-demand-e2eon this PRmc01-bootstrapCodeBuild log shows=== Prometheus Operator CRDs present — proceeding with hypershift install ===🤖 Generated with Claude Code
Summary by CodeRabbit