Skip to content

ci: switch to custom setup-go action in windows #15623

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

Merged
merged 4 commits into from
Feb 13, 2025
Merged

Conversation

kruskall
Copy link
Member

@kruskall kruskall commented Feb 7, 2025

Motivation/summary

windows runner has significantly slower disks and restoring the cache is wasting a huge amount of time

Checklist

For functional changes, consider:

  • Is it observable through the addition of either logging or metrics?
  • Is its use being published in telemetry to enable product improvement?
  • Have system tests been added to avoid regression?

How to test these changes

Related issues

See actions/setup-go#495
Slightly related to #15581

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time
@kruskall kruskall requested a review from a team as a code owner February 7, 2025 19:37
Copy link
Contributor

mergify bot commented Feb 7, 2025

This pull request does not have a backport label. Could you fix it @kruskall? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-7.17 is the label to automatically backport to the 7.17 branch.
  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • backport-9./d is the label to automatically backport to the 9./d branch. /d is the digit.
  • backport-8.x is the label to automatically backport to the 8.x branch.
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@kruskall kruskall added the backport-active-all Automated backport with mergify to all the active branches label Feb 7, 2025
@kruskall kruskall changed the title ci: update GOCACHE vars on windows runner to improve performance ci: switch to custom setup-go action in windows Feb 10, 2025
@kruskall kruskall requested a review from a team February 10, 2025 17:10
@kruskall
Copy link
Member Author

kruskall commented Feb 10, 2025

tested on my fork, test windows time went down from ~5m to ~1m: https://github.com/kruskall/apm-server/actions/runs/13245308706/job/36972215506

@@ -45,7 +45,13 @@ jobs:
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: antontroshin/setup-go@bda02de8887c9946189f81e7e59512914aeb9ea4
Copy link
Member

Choose a reason for hiding this comment

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

To play safe, this is a fork commit, and somehow I feel we could raise the fix to the upstream maybe, instead of using the fork from someone else.

I understand the benefits, but I'm a bit concerned about opening this

Copy link
Member

Choose a reason for hiding this comment

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

OTOH, we are using permissions: contents: read by default. But according to our guideline, we need to be cautious to use third-party actions.

Copy link
Member Author

Choose a reason for hiding this comment

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

there's already a PR open for it upstream, the action is from that PR 😭

See actions/setup-go#515

OTOH, we are using permissions: contents: read by default. But according to our guideline, we need to be cautious to use third-party actions.

fully agree with this. I pinned the action to a commit to avoid tampering. It should be enough to avoid such issues. WDYT ?

Copy link
Member

Choose a reason for hiding this comment

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

We can give a go; the only thing could be the one whether the commit vanished

@kruskall kruskall enabled auto-merge (squash) February 13, 2025 16:18
@kruskall kruskall merged commit 211ccc8 into main Feb 13, 2025
13 of 14 checks passed
@kruskall kruskall deleted the ci/update-gocache-vars branch February 13, 2025 16:23
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)

# Conflicts:
#	.github/workflows/ci.yml
mergify bot added a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)

Co-authored-by: kruskall <99559985+kruskall@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)

Co-authored-by: kruskall <99559985+kruskall@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Feb 13, 2025
* ci: update GOCACHE vars on windows runner to improve performance

windows runner has significantly slower disks and restoring the cache is
wasting a huge amount of time

* Update ci.yml

(cherry picked from commit 211ccc8)

Co-authored-by: kruskall <99559985+kruskall@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-active-all Automated backport with mergify to all the active branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants