Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Add step to place workflow_dispatch inputs in env #31424

Merged

Conversation

learnitall
Copy link
Contributor

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

This commit adds a new job to the beginning of workflows that have a workflow_dispatch trigger whose inputs provide important context to the run. The job saves the inputs to the environment so they can be viewed in the logs by users.

A majority of the workflows in the cilium repository have a workflow_dispatch trigger, which is used by users and Ariane to run workflows as needed. These workflows take a variety of inputs that may not always be made available in the logs, as they may not be used in a place that warrants their value being outputted. This obfuscation can make debugging these workflows difficult at times, as these inputs provide important context to the origin of the workflow's trigger.

For instance, the PR-number input is used to determine the concurrency group for a workflow run. It is ignored everywhere else. The value of the concurrency group is not available via GitHub's UI or API, therefore this input obfuscated. However, the PR-number input provides valuable metadata, being used to describe the branch or PR that a workflow was triggered for.

This commit modifies workflows which have non-obvious workflow_dispatch input values

Fixes: #31207

Modify GitHub Actions Workflows to echo the inputs they are given when triggered by a `workflow_dispatch` event.

@learnitall learnitall added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Mar 15, 2024
@learnitall
Copy link
Contributor Author

/test

@learnitall learnitall added dont-merge/preview-only Only for preview or testing, don't merge it. area/CI-improvement Topic or proposal to improve the Continuous Integration workflow labels Mar 15, 2024
@learnitall learnitall force-pushed the pr/learnitall/echo-workflow-inputs branch from 55e606c to a256092 Compare March 26, 2024 15:24
@learnitall learnitall marked this pull request as ready for review March 26, 2024 15:24
@learnitall learnitall requested review from a team as code owners March 26, 2024 15:24
@learnitall learnitall force-pushed the pr/learnitall/echo-workflow-inputs branch from a256092 to 1d10aec Compare March 26, 2024 15:43
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only reviewed the two IPsec workflows.

Copy link
Contributor

@brlbil brlbil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Steps names require change.

.github/workflows/build-images-beta.yaml Outdated Show resolved Hide resolved
.github/workflows/build-images-beta.yaml Outdated Show resolved Hide resolved
.github/workflows/build-images-beta.yaml Show resolved Hide resolved
@learnitall learnitall force-pushed the pr/learnitall/echo-workflow-inputs branch from 1d10aec to 73a79ac Compare March 27, 2024 22:35
@learnitall learnitall requested a review from a team as a code owner March 27, 2024 22:35
@learnitall learnitall force-pushed the pr/learnitall/echo-workflow-inputs branch from 73a79ac to 422af3d Compare March 27, 2024 22:38
@learnitall learnitall removed the request for review from a team March 27, 2024 22:38
@learnitall learnitall force-pushed the pr/learnitall/echo-workflow-inputs branch from 422af3d to 5cb9329 Compare March 27, 2024 22:42
@learnitall learnitall requested a review from brlbil March 27, 2024 22:43
@brlbil
Copy link
Contributor

brlbil commented Mar 28, 2024

/test

Copy link
Contributor

@brlbil brlbil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since PR is from a fork added code is not tested properly. But changes are straightforward forward so it should be okay.

@learnitall learnitall removed the dont-merge/preview-only Only for preview or testing, don't merge it. label Mar 28, 2024
@learnitall learnitall force-pushed the pr/learnitall/echo-workflow-inputs branch from b59a5e2 to 383c14e Compare April 4, 2024 18:33
@learnitall
Copy link
Contributor Author

Well, my apologies folks. A sanity check was a good idea indeed. After trying out the job in a fork on Conformance AKS (ci-aks), an error was returned stating the workflow file was invalid: https://github.com/learnitall/cilium/actions/runs/8559471668. I pushed a fix and now the new job runs successfully: https://github.com/learnitall/cilium/actions/runs/8559529975/job/23456474426.

I ran each modified workflow to verify the job works for each. In these runs, the workflows failed early due to the bad inputs that I provided to the workflow dispatch trigger (ie invalid context-ref) or due to missing variables in my fork. I've double-checked that the workflow failures are unrelated to the changes in this PR.

Workflow New Job Passes? Workflow Failure Reason Link
Conformance AKS (ci-aks) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699033/job/23457013307
Conformance AWS-CNI (ci-awscni) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699070/job/23457017880
Conformance Cluster Mesh (ci-clustermesh) Yes Missing variable GH_RUNNER_EXTRA_POWER https://github.com/learnitall/cilium/actions/runs/8559699162/job/23457006748
Conformance E2E (ci-e2e) Yes Hangs due to unavailability of ubuntu-latest-4cores-16gb runner https://github.com/learnitall/cilium/actions/runs/8559699253/job/23458326225
Conformance EKS (ci-eks) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699394/job/23457008845
Conformance Gateway API (ci-gateway-api) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699562/job/23457008859
Conformance External Workloads (ci-external-workloads) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699634/job/23457009142
Conformance Ginkgo (ci-ginkgo) Yes Bad context-ref and SHA (test2 and test3) https://github.com/learnitall/cilium/actions/runs/8559699685/job/23457009539
Conformance GKE (ci-gke) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699795/job/23457010523
Conformance Ingress (ci-ingress) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559699895/job/23457010801
Conformance IPsec E2E (ci-ipsec-e2e) Yes Hangs due to unavailability of ubuntu-latest-4cores-16gb runner https://github.com/learnitall/cilium/actions/runs/8559699986/job/23457011526
Conformance Multi Pool IPAM (ci-multi-pool) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559700136/job/23457012659
Conformance Runtime (ci-runtime) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559700181/job/23457013531
Integration Tests (ci-integration) Yes Hangs due to unavailability of ubuntu-22.04-arm64 runner https://github.com/learnitall/cilium/actions/runs/8559700346/job/23457013462
Cilium Cluster Mesh upgrade (ci-clustermesh) Yes Missing variable GH_RUNNER_EXTRA_POWER https://github.com/learnitall/cilium/actions/runs/8559700459/job/23457013588
Datapath BPF Complexity (ci-verifier) Yes Hangs due to unavailability of ubuntu-latest-4cores-16gb runner https://github.com/learnitall/cilium/actions/runs/8559700522/job/23457014793
Cilium E2E Upgrade (ci-e2e-upgrade) Yes Hangs due to unavailability of ubuntu-latest-4cores-16gb runner https://github.com/learnitall/cilium/actions/runs/8559700684/job/23457015960
Cilium IPsec upgrade (ci-ipsec-upgrade) Yes Hangs due to unavailability of ubuntu-latest-4cores-16gb runner https://github.com/learnitall/cilium/actions/runs/8559700778/job/23457016243
Cilium L4LB XDP (ci-l4lb) Yes Bad context-ref (test2) https://github.com/learnitall/cilium/actions/runs/8559700877/job/23457015935
Beta Image Release Build Yes Missing quay credentials https://github.com/learnitall/cilium/actions/runs/8560175776/job/23458547388

Thank you @julianwiedmann and @joestringer for pushing back and helping me learn a valuable lesson about testing, I appreciate it.

@learnitall learnitall removed the dont-merge/preview-only Only for preview or testing, don't merge it. label Apr 4, 2024
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing a sanity check there, that gives me a bit more confidence to click the merge button :)

@joestringer joestringer merged commit 77cafec into cilium:main Apr 4, 2024
44 checks passed
learnitall added a commit to learnitall/cilium-ci-opensearch that referenced this pull request May 6, 2024
This commit builds off of cilium/cilium#31424 by
adding the ability to parse inputs that are echoed in the logs of
workflow_dispatch triggered runs. This commit also performs some cleanup
based on changes in previous commits.

Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>
@learnitall learnitall added needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels May 9, 2024
@learnitall
Copy link
Contributor Author

Adding needs backport here. When we run scheduled workflows off of main through the Ariane Scheduled Workflows workflow, the workflow files from the target branch are used, not the workflow files from main.

@jibi jibi mentioned this pull request May 13, 2024
5 tasks
@jibi jibi added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels May 13, 2024
@jibi jibi mentioned this pull request May 13, 2024
2 tasks
@jibi jibi added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels May 13, 2024
@jibi jibi mentioned this pull request May 13, 2024
2 tasks
@jibi jibi added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels May 13, 2024
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commit displayed in GHA Workflow summary is not the commit which was tested.
8 participants