feat(ci): add scheduled oci integration-test workflows - #6969
Conversation
fe9cbc3 to
a6fc21c
Compare
holmanb
left a comment
There was a problem hiding this comment.
File naming scheme matches the pattern, thanks for that. This looks pretty good.
I have a couple of minor questions related to organization and reuse.
| run: | | ||
| sh -c 'echo "$REQUIRED_SECRET" | base64 -d > "$PYCLOUDLIB_CONFIG"' | ||
| - name: Setup OCI credentials | ||
| if: ${{ env.CLOUD_INIT_PLATFORM == 'oci' }} |
There was a problem hiding this comment.
Cloud-specific code in a common workflow seems messy. I worry that this shared file will become increasingly difficult to understand due to cloud-specific conditional code. Can this be done outside of the shared workflow? Some question applies for LXD as well.
There was a problem hiding this comment.
A github actions job which calls a reusable workflow via workflow_call cannot also contain steps: because the entire job is delegated to the called workflow. So we cannot prepend a step like setting up lxd or writing an OCI file, and expect that LXD is configured or that a file is persisted for the common pycloudlib integration test run because they are completely separate jobs. Called jobs appear to only obtain data files via actions/download-artifact and the preparatory job or calling job would use actions/upload-artifact to setup whatever payload we want to pass in, but that would potentially expose our jobs to transporting senstive files between jobs which may be more risky.
Our alternatives appear to be to create a separate 100-dispatch-common-oci.yml job which would would have the inline step and mandatory secret checks for OCI stuff. This would contain nearly the same duplicated steps as our 100-dispatch-common.yml job, with the exception of the mandatory secret check for PYCLOUDLIB_OCI_* secrets and the writing of the oci config and pem files.
If we decide to tease out 100-dispatch-common-oci, we'd also want to tease out lxd setup into 100-dispatch-common-lxd.yml. At that point we would have 3 nearly duplicated 100-dispatchyml actions to differentiate truly common, lxd and oci pre-requisite steps. So, it looks like we either carry around 135 lines duplicated in 3 dispatch-common.yml workflows or cope with the 40-50 lines of platform conditionals inside the single 100-dispatch-common.yml.
If we decided to stay with platform conditionals inside 100-dispatch-common.yml we can further reduce the amount of vertical space used to represent those steps by creation a composite action such as uses: .github/actions/setup-oci-keys which could encapsulate the oci secrets validation and writing keys so we don't have to grok that detail in 100-dispatch-common.yml.
I'm leaning toward keeping the 100-dispatch-common.yml, adding small composite actions to reduce the reading load on the truly shared 100-dispatch-common.yml. But, if you feel strongly that we should differentiate 100-dispatch-common-oci and common-lxd I can propose that difference for a quick look.
There was a problem hiding this comment.
Our alternatives appear to be to create a separate 100-dispatch-common-oci.yml job
This adds quite a bit of both indirection and almost fully duplicates the current common file (plus it duplicates one of our number prefixes which breaks an existing pattern).
Is there any reason we couldn't have our OCI workflows do something like this:
- name: Setup OCI credentials
env:
PYCLOUDLIB_OCI_CONFIG_FILE_PATH: ${{ runner.temp }}/oci/config
PYCLOUDLIB_OCI_KEY_FILE_PATH: ${{ runner.temp }}/oci/key.pem
run: |
mkdir -p "$(dirname "$PYCLOUDLIB_OCI_CONFIG_FILE_PATH")"
sh -c 'printf "%s\n" "$PYCLOUDLIB_OCI_CONFIG_B64" | base64 -d > "$PYCLOUDLIB_OCI_CONFIG_FILE_PATH"'
sh -c 'printf "%s\n" "$PYCLOUDLIB_OCI_KEY_B64" | base64 -d | install -m 600 /dev/stdin "$PYCLOUDLIB_OCI_KEY_FILE_PATH"'
- name: Run integration Tests
uses: ./.github/workflows/100-dispatch-oci.yml
with:
...There was a problem hiding this comment.
There are a couple of things blocking here:
- We cannot add preliminary steps in a single job and call a reusable workflow because the reusable workflow uses a different runner and hence a difference filesystem. this results in not seeing the file we created in the separate oci workflow's steps
- Even if we converted all the 'steps' into composite actions which can be called by multiple jobs. The oci-specific or lxd-specific job is delegated to a separate runner to call the initial steps (setup-lxd or setup-oci-cred-files) and that job filesystem/environment is not reused, so we'd have to use upload-artifacts from setup oci workflow and download-artifacts from the common workflow.
That said, if we want to reduce the size of such platform-specific steps in the common workflow, I think migrating our platform-specific steps into composite actions will make it easier to read the 100-dispatch-common.yml because we will have something like:
- name: Setup OCI creds
uses: ./.github/actions/setup-oci-creds
instead of
- name: Setup OCI credentials
env:
PYCLOUDLIB_OCI_CONFIG_FILE_PATH: ${{ runner.temp }}/oci/config
PYCLOUDLIB_OCI_KEY_FILE_PATH: ${{ runner.temp }}/oci/key.pem
run: |
mkdir -p "$(dirname "$PYCLOUDLIB_OCI_CONFIG_FILE_PATH")"
sh -c 'printf "%s\n" "$PYCLOUDLIB_OCI_CONFIG_B64" | base64 -d > "$PYCLOUDLIB_OCI_CONFIG_FILE_PATH"'
sh -c 'printf "%s\n" "$PYCLOUDLIB_OCI_KEY_B64" | base64 -d | install -m 600 /dev/stdin "$PYCLOUDLIB_OCI_KEY_FILE_PATH"'
There was a problem hiding this comment.
@holmanb I made further progress when I converted the entire common workflow 100-common-dispatch.yml into a composite action. The supplemental 3 commits to make that change are here:
- convert common dispatch to common composite action
- Ensure checkout is performed in scheduled workflows before trying to reference composite action
- Fix unavailable runner.temp template variables in composite action
With these three additional commits, we have a successful oci workflow run
The common structure will be the following then:
- scheduled platform-specific jobs:
- duplicated repo assertion (only run on upstream repo)
- platform specific secret assertions
- platform specific setup steps (and initial checkout) in 15X-*oci.yml
- initial checkout
- call common composite action integration-tests
- common composite action integration tests:
- obtain inputs for runner_temp, platform, image_type, install_source, and filter_tests(name to be changed in next PR)
- perform common required secret assertions
- setup-lxd (TODO: move out into lxd-specific scheduled runners in next PR)
- remaining integration steps
If this approach is agreeable, I'll pull in those 3 commits and push to this branch for final review. Then I can followup with two small PRs:
- rename of
filter_tests->integration_test_params - move setup-lxd out to platform specific scheduled workflows
I have pushed the proposed commits mentioned above to thisa PR as I believe that best aligns with your earlier review suggestions.
There was a problem hiding this comment.
Per review comments on #7047 I had missed converting the azure jobs to use composite action in this PR. I have remedied that issue here so it's central to the review and migration away from 100-dispatch-common.yml to .github/actions/integration-test/action.yml.
Successful dispatch of Azure here
| options: | ||
| - generic | ||
| - minimal | ||
| filter_tests: |
There was a problem hiding this comment.
filter_tests doesn't seem like the best name for this. That is one thing that can be done, but it seems like one might add other arguments to tox via this variable name.
We can fix this in a followup.
Create oci workflow integration test coverage for Oracle on Ubuntu Jammy, Noble, Resolute and Stonking scheduled twice-weekly: - Mon & Thu, cron '2 22 * * 1,4 Extend the shared 100-dispatch-common.yml workflow: - add 'oci' to the workflow_dispatch platform choice list - add two optional secrets PYCLOUDLIB_OCI_CONFIG_B64 / PYCLOUDLIB_OCI_KEY_B64 to workflow_call.secrets - pass PYCLOUDLIB_OCI_CONFIG_FILE_PATH / PYCLOUDLIB_OCI_KEY_FILE_PATH to the Run integration Tests step env - clean up the oci dir in the Clean pycloudlib step
1b0e193 to
892da6c
Compare
Replace the reusable workflow .github/workflows/100-dispatch-common.yml with a composite action .github/actions/integration-test that contains the shared integration-test steps (secret assertion, checkout, LXD/SSH/ pycloudlib setup, tox run, report publish, cleanup). OCI-specific secret validation and credential setup are pulled out of the common path and moved into steps within each 15*oci.yml scheduled workflow, which write the OCI config/key to $RUNNER_TEMP/oci before invoking the composite action. All 16 scheduled integration workflows (4 OCI, 4 EC2, 8 LXD) now call the composite action directly with job-level env secret mappings, matching the repo's existing composite-action convention (.github/actions/create-sru-bug). Behavior is preserved; the canonical/cloud-init repository guard moves from the reusable workflow job onto each calling job.
The 15*oci.yml workflows run OCI-specific steps before invoking the local composite action ./.github/actions/integration-test. GitHub requires action.yml to exist on the runner to load a local action, so actions/checkout must run first. Add a Checkout step at the top of each OCI workflow's job, ahead of the OCI secret assertion and credential setup steps.
The runner context is not available inside a composite action's runs
section at manifest-load time, so ${{ runner.temp }} expressions in
.github/actions/integration-test/action.yml caused workflow dispatch to
fail with "Unrecognized named-value: 'runner'". Add a required
runner_temp input to the composite action and replace its internal
${{ runner.temp }} references with ${{ inputs.runner_temp }}. All 16
caller workflows now pass runner_temp: ${{ runner.temp }} (the runner
context is available in workflow jobs).
Drop now unused 100-common-dispatch.yml as it is is unreferenced now.
0e3f1d4 to
4eff4d1
Compare
|
Per review comments on 7047 I had forgotten to address azure dropping the use of 100-dispatch-common.yml. Top most commit handles that unaddressed aspect of the migration to composite action for azure. |
holmanb
left a comment
There was a problem hiding this comment.
Can we split this in two? It both adds OCI support and refactors all of the existing integration actions.
Separated out #7070 to review just the refactor. |
Blocked by #7070
Proposed Commit Message
Additional Context
Success run on individual oci creds: https://github.com/blackboxsw/cloud-init/actions/runs/30711239669
Test Steps
[oci]section documenting your configCreate an oci config file via new API Keys from cloud.oracle.com
PYCLOUDLIB_OCI_CONFIG_B64 # Copy value from: cat oci.config | base64 -w 0
Grab private key pem from cloud.oracle.com
PYCLOUDLIB_OCI_KEY_B64 # Copy value from: cat your-team-login-date*pem | base64 -w 0
PYCLOUDLIB_TOML_B64 # copy value from cat pycloudlib.toml | base64 -w 0
SSH_PRIVATE_KEY # Copy shared CI private key used for all integration tests
SSH_PUBLIC_KEY # ditto public key
Merge recommendations