Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 20, 2025

No description provided.

Copilot AI changed the title [WIP] Optimize CI performance for Ubuntu image builds Optimize CI: conditionally build debian/ubuntu based on changed paths Nov 20, 2025
Copilot AI requested a review from phlax November 20, 2025 13:02
@phlax
Copy link
Member

phlax commented Nov 20, 2025

@copilot before we proceed

  • strip all unnecessary ws
  • remove all comments - if i want comments ill ask for them

next thing - we have a gh-action called torun (in envoyproxy/toolshed/gh-actions) - i forget exactly how it works but im thinking it can possibly be used for figuring out the path - and would allow some configurability - also we prefer to keep actions in house where possible

@phlax
Copy link
Member

phlax commented Nov 20, 2025

oh the other thing @copilot - use least indent for yaml arrays

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

@copilot before we proceed

  • strip all unnecessary ws
    ...

Done - replaced dorny/paths-filter with torun action, stripped whitespace, and removed all comments. Commit 38d1bc0.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/envoyproxy/toolshed/contents/gh-actions
    • Triggering command: curl -s REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@phlax
Copy link
Member

phlax commented Nov 20, 2025

couple more things @copilot

  • 4 space indent on bash
  • github if conditionals dont need ${{ }} they can just take the condition straight

now wrt to the config for what to run ... i would prefer to move this to an adjacent yaml file the toolshed jq action can easily do the conversion from file to json steps var

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

couple more things @copilot

  • 4 space indent on bash
    ...

Done - 4 space bash indent, removed unnecessary ${{ }} from if conditionals, and moved config to .github/config.yml with jq action loading it. Commit a0b224e.

oh the other thing @copilot - use least indent for yaml arrays

Done - using least indent for YAML arrays. Commit a0b224e.

@phlax
Copy link
Member

phlax commented Nov 20, 2025

ok - looks good - for some reason ci doesnt run on your prs - which in general is a good thing but now i would like to actually test this

@phlax
Copy link
Member

phlax commented Nov 20, 2025

@copilot ^^

@phlax
Copy link
Member

phlax commented Nov 20, 2025

@copilot posting here as you seem to hve mised comment on pr

ok - looks good - for some reason ci doesnt run on your prs - which in general is a good thing but now i would like to actually test this

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

@copilot posting here as you seem to hve mised comment on pr

ok - looks good - for some reason ci doesnt run on your prs - which in general is a good thing but now i would like to actually test this

The CI not running on bot PRs is expected GitHub Actions security behavior - workflows don't automatically run for PRs from first-time contributors or bots to prevent unauthorized access to secrets. You can manually trigger the workflow by pushing a commit, using workflow_dispatch if configured, or by having a maintainer with write access approve the workflow run.

@phlax
Copy link
Member

phlax commented Nov 20, 2025

hmm that wasnt it @copilot

(Line: 38, Col: 7): Unexpected value 'output-path', (Line: 95, Col: 9): A sequence was not expected

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

hmm that wasnt it @copilot

...

Fixed both YAML syntax errors:

  1. Moved output-path inside the with: block (line 30)
  2. Changed mounts: to use pipe format (line 94)

Commit 5977c86.

@phlax
Copy link
Member

phlax commented Nov 20, 2025

@copilot can you see the CI/actions? there are more syntax errors in the yaml

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

@copilot can you see the CI/actions? there are more syntax errors in the yaml

I don't have access to view the CI/Actions tab, but I found and fixed the syntax error: removed the undefined matrix.os_family reference at line 76 (it was trying to use a matrix variable that wasn't defined). Commit b2ebf41.

@phlax
Copy link
Member

phlax commented Nov 20, 2025

@copilot (Line: 58, Col: 9): Unrecognized named-value: 'matrix'. Located at position 122 within expression: (github.repository == 'envoyproxy/envoy-build-tools' || vars.ENVOY_CI_RUN == 'true') && (github.event_name == 'push' || (matrix.target == 'debian' && needs.detect-changes.outputs.debian == 'true') || (matrix.target == 'ubuntu' && needs.detect-changes.outputs.ubuntu == 'true'))

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

@copilot same problem

Removed the result-filter and replaced it with a separate jq step. The bson action now just runs the git diff command, and the jq action processes the raw output (with input-format: raw and options: -Rs) to split by newlines and filter empty lines into a JSON array. Commit 18a14fc.

@phlax phlax force-pushed the copilot/optimize-ci-for-ubuntu-images branch from 18a14fc to b435692 Compare November 20, 2025 15:34
@phlax phlax force-pushed the copilot/optimize-ci-for-ubuntu-images branch 2 times, most recently from 1ccdfe0 to 9cf2b37 Compare November 20, 2025 15:44
@phlax phlax force-pushed the copilot/optimize-ci-for-ubuntu-images branch from 9cf2b37 to 7aaa89e Compare November 20, 2025 15:45
Signed-off-by: Ryan Northey <[email protected]>
@phlax phlax force-pushed the copilot/optimize-ci-for-ubuntu-images branch from 3cecf1a to 6c5223a Compare November 20, 2025 16:29
@phlax phlax changed the title Optimize CI: conditionally build debian/ubuntu based on changed paths ci: Conditionally build debian/ubuntu based on changed paths Nov 20, 2025
@phlax phlax marked this pull request as ready for review November 20, 2025 16:30
@phlax phlax requested a review from a team as a code owner November 20, 2025 16:30
@phlax
Copy link
Member

phlax commented Nov 20, 2025

cc @jwendell

@phlax phlax enabled auto-merge (squash) November 20, 2025 16:31
@phlax phlax disabled auto-merge November 20, 2025 17:59
@phlax phlax merged commit 41eb76c into main Nov 20, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants