WIP Hypershift CI Serial job - #83038
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughAdds an optional HyperShift AWS OVN serial conformance job. The change introduces ipecho bastion provisioning and cleanup steps, registers the workflow, and configures a 20-hour job timeout. ChangesHyperShift AWS OVN conformance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CIJob
participant ConformanceWorkflow
participant HyperShift
participant IpechoProvision
participant AWS
participant IpechoDeprovision
CIJob->>ConformanceWorkflow: Start serial conformance job
ConformanceWorkflow->>HyperShift: Create management and AWS clusters
ConformanceWorkflow->>IpechoProvision: Provision ipecho bastion
IpechoProvision->>AWS: Create security group and EC2 instance
AWS-->>IpechoProvision: Return instance and endpoint details
ConformanceWorkflow->>ConformanceWorkflow: Wait and collect cluster state
ConformanceWorkflow->>HyperShift: Destroy cluster
ConformanceWorkflow->>IpechoDeprovision: Clean up bastion
IpechoDeprovision->>AWS: Terminate instance and delete security group
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: asood-rh 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 |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
openshift-ci
Bot
added
the
do-not-merge/invalid-owners-file
|
The OWNERS file contains untrusted users, which makes it INVALID. The following users are mentioned in OWNERS file(s) but are untrusted for the following reasons. One way to make the user trusted is to add them as members of the openshift org. You can then trigger verification by writing
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh`:
- Around line 49-52: Remove the TCP port 22 ingress authorization from the
provisioning commands, leaving only the ipecho port rule in the security group
setup. Do not add a replacement SSH rule; preserve the existing public ipecho
HTTP endpoint configuration.
- Around line 105-113: Remove the instance-side sed replacement near the
generated systemd unit and replace IPECHO_PORT_PLACEHOLDER exactly once before
launching the instance, preserving the configured IPECHO_PORT consistently for
the service, security group, and probe. Update the ipecho provisioning command
flow around the service heredoc and launch logic without changing unrelated
defaults.
- Around line 41-46: Make cleanup durable across partial provisioning failures:
in
ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh
lines 41-46, persist the security-group identifier and region immediately after
create-security-group; in the same file lines 126-135, persist the instance
identifier immediately after run-instances. In
ci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-commands.sh
lines 9-18, delete each resource whenever its own identifier is recorded,
allowing security-group cleanup without requiring ipecho_instance_id.
- Line 122: Update the user-data handling in the run-instances command to remove
USER_DATA_B64 and pass the raw USER_DATA value directly to --user-data, ensuring
cloud-init receives the ipecho script rather than pre-encoded content.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0bbd855e-e1c5-43bb-9dd9-a9e01379fa97
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/ovn-kubernetes/openshift-ovn-kubernetes-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (12)
ci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-main.yamlci-operator/step-registry/hypershift/aws/ovn/conformance/OWNERSci-operator/step-registry/hypershift/aws/ovn/conformance/hypershift-aws-ovn-conformance-workflow.metadata.jsonci-operator/step-registry/hypershift/aws/ovn/conformance/hypershift-aws-ovn-conformance-workflow.yamlci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/OWNERSci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-commands.shci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-ref.metadata.jsonci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-ref.yamlci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/OWNERSci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.shci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-ref.metadata.jsonci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-ref.yaml
| SG_ID=$(aws ec2 create-security-group \ | ||
| --group-name "ipecho-bastion-${NAMESPACE}-${UNIQUE_HASH}" \ | ||
| --description "ipecho bastion for CI job ${JOB_NAME}" \ | ||
| --vpc-id "${VPC_ID}" \ | ||
| --tag-specifications "ResourceType=security-group,Tags=[{Key=Name,Value=ipecho-bastion-${NAMESPACE}},{Key=expirationDate,Value=${EXPIRATION_DATE}}]" \ | ||
| --query "GroupId" --output text) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the ipecho cleanup protocol durable across partial provisioning failures.
Provisioning delays resource-marker writes. Deprovisioning requires an instance marker. A failed AWS call can therefore leave a security group or instance without cleanup.
ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh#L41-L46: Write the security-group identifier and region immediately aftercreate-security-group.ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh#L126-L135: Write the instance identifier immediately afterrun-instances.ci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-commands.sh#L9-L18: Clean up each resource when its own identifier exists. Do not requireipecho_instance_idbefore deleting a recorded security group.
📍 Affects 2 files
ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh#L41-L46(this comment)ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh#L126-L135ci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-commands.sh#L9-L18
🤖 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
`@ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh`
around lines 41 - 46, Make cleanup durable across partial provisioning failures:
in
ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh
lines 41-46, persist the security-group identifier and region immediately after
create-security-group; in the same file lines 126-135, persist the instance
identifier immediately after run-instances. In
ci-operator/step-registry/hypershift/aws/ovn/ipecho/deprovision/hypershift-aws-ovn-ipecho-deprovision-commands.sh
lines 9-18, delete each resource whenever its own identifier is recorded,
allowing security-group cleanup without requiring ipecho_instance_id.
| aws ec2 authorize-security-group-ingress --group-id "${SG_ID}" \ | ||
| --protocol tcp --port "${IPECHO_PORT}" --cidr "0.0.0.0/0" >/dev/null | ||
| aws ec2 authorize-security-group-ingress --group-id "${SG_ID}" \ | ||
| --protocol tcp --port 22 --cidr "0.0.0.0/0" >/dev/null |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the Internet-wide SSH rule.
The supplied workflow only requires the public ipecho HTTP endpoint. TCP/22 is not used by the supplied step contract. This rule exposes SSH on a public instance to every Internet address.
Remove the TCP/22 ingress rule. If SSH is required later, restrict its CIDR to an authenticated diagnostic source.
🤖 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
`@ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh`
around lines 49 - 52, Remove the TCP port 22 ingress authorization from the
provisioning commands, leaving only the ipecho port rule in the security group
setup. Do not add a replacement SSH rule; preserve the existing public ipecho
HTTP endpoint configuration.
| ExecStart=/usr/bin/python3 /usr/local/bin/ipecho.py IPECHO_PORT_PLACEHOLDER | ||
| Restart=always | ||
| RestartSec=3 | ||
|
|
||
| [Install] | ||
| WantedBy=multi-user.target | ||
| SVCEOF | ||
|
|
||
| sed -i "s/IPECHO_PORT_PLACEHOLDER/${IPECHO_PORT:-9095}/" /etc/systemd/system/ipecho.service |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the configured IPECHO_PORT.
The replacement at line 120 changes both IPECHO_PORT_PLACEHOLDER occurrences. If IPECHO_PORT is 8080, the generated sed command replaces 8080 with its instance-side default of 9095. The service then listens on 9095, while the security group and probe use 8080.
Remove the instance-side sed command. Replace the placeholder once before launch.
Proposed fix
- sed -i "s/IPECHO_PORT_PLACEHOLDER/${IPECHO_PORT:-9095}/" /etc/systemd/system/ipecho.service
systemctl daemon-reload
systemctl enable --now ipecho.serviceAlso applies to: 119-120
🤖 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
`@ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh`
around lines 105 - 113, Remove the instance-side sed replacement near the
generated systemd unit and replace IPECHO_PORT_PLACEHOLDER exactly once before
launching the instance, preserving the configured IPECHO_PORT consistently for
the service, security group, and probe. Update the ipecho provisioning command
flow around the service heredoc and launch logic without changing unrelated
defaults.
| # Replace the placeholder with the actual port in user-data | ||
| USER_DATA="${USER_DATA//IPECHO_PORT_PLACEHOLDER/${IPECHO_PORT}}" | ||
|
|
||
| USER_DATA_B64=$(echo "${USER_DATA}" | base64 -w0) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
aws ec2 run-instances help | sed -n '/--user-data/,/^ --/p'Repository: openshift/release
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=$(fd -t f 'hypershift-aws-ovn-ipecho-provision-commands\.sh$' . | head -n1)
test -n "$file"
printf '%s\n' "FILE=$file"
sed -n '90,145p' "$file"
printf '%s\n' '--- related user-data and run-instances references ---'
rg -n -C 3 'USER_DATA(_B64)?|run-instances|--user-data|ipecho' "$file" ci-operator/step-registry/hypershift/aws/ovn/ipecho || trueRepository: openshift/release
Length of output: 47796
🌐 Web query:
AWS CLI ec2 run-instances --user-data base64 encoding AWS documentation
💡 Result:
When using the AWS CLI to launch EC2 instances with the run-instances command, you do not need to manually base64-encode your user data [1][2][3]. The AWS CLI automatically performs the required base64 encoding for you before sending the data to the Amazon EC2 API [1][3]. You can provide the user data in a few ways: 1. As a plain text string directly on the command line: aws ec2 run-instances --image-id --user-data "echo Hello World" 2. As the contents of a local file by using the file:// prefix, which instructs the AWS CLI to load the text from a file: aws ec2 run-instances --image-id --user-data file://path/to/your/script.txt It is important to note that the AWS CLI expects the input to be plain text, not pre-encoded base64 data [4]. If you provide already encoded base64 data, the CLI will treat that encoded string as the raw content and encode it again, resulting in the instance receiving the base64-encoded string rather than your original script [4][5]. While the underlying Amazon EC2 API requires base64-encoded user data [6][3], the AWS CLI abstracts this requirement away from the user for convenience [1][2].
Citations:
- 1: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/user-data.html
- 2: https://awscli.amazonaws.com/v2/documentation/api/2.4.18/reference/ec2/run-instances.html
- 3: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_UserData.html
- 4: aws ec2 run-instances, option user-data and base64 string aws/aws-cli#1708
- 5: ec2 run-instance incorrect description for --user-data aws/aws-cli#366
- 6: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RunInstances.html
Pass raw user data to aws ec2 run-instances.
The AWS CLI base64-encodes --user-data input. Remove USER_DATA_B64 and pass --user-data "${USER_DATA}"; otherwise cloud-init receives encoded data instead of the ipecho script.
🤖 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
`@ci-operator/step-registry/hypershift/aws/ovn/ipecho/provision/hypershift-aws-ovn-ipecho-provision-commands.sh`
at line 122, Update the user-data handling in the run-instances command to
remove USER_DATA_B64 and pass the raw USER_DATA value directly to --user-data,
ensuring cloud-init receives the ipecho script rather than pre-encoded content.
|
/pj-rehearse pull-ci-openshift-ovn-kubernetes-main-e2e-aws-ovn-hypershift-serial |
|
@asood-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-ovn-kubernetes-main-e2e-aws-ovn-hypershift-serial |
|
@asood-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@asood-rh: 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. |
1 participant
Hypershift on AWS with ipecho server
Summary by CodeRabbit
ipechobastion workflow that provisions AWS resources, exposes a client-IP endpoint, supports debugging, and performs cleanup.