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

Enabling actions/cache for GHES based on presence of AC service #774

Merged
merged 23 commits into from
Mar 30, 2022

Conversation

tiwarishub
Copy link
Contributor

@tiwarishub tiwarishub commented Mar 23, 2022

This PR enables actions/cache in GHES based on the presence of the Actions cache service in the GHES instance. It checks the presence of the GHES instance using the recent toolkit changes published in 2.0.0 actions/toolkit#1028. This same presence can also be applied to the dotcom scenario.

When AC service is not present
image

When AC service is present
image

image

Linked items: https://github.com/github/c2c-actions/issues/4065

src/restore.ts Outdated
utils.logWarning(
"Cache action is not supported on GHES. See https://github.com/actions/cache/issues/505 for more details"
);
if (!cache.isAvailable()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename isAvailable to isFeatureAvailable otherwise it is looks like we are talking about a cache item being available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

src/save.ts Outdated
"Something is going wrong with ArtifactCache service which supports cache actions. Please check https://www.githubstatus.com/ for any ongoing issue in actions."
);
}
utils.setCacheHitOutput(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this during save?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed it

src/save.ts Outdated
Comment on lines 14 to 24
if (!cache.isAvailable()) {
if (utils.isGhes()){
utils.logWarning(
"Cache action is only supported on GHES version >= 3.5. If you are on version >=3.5 Please check with GHES admin if ArtifactCache service is enabled or not."
);
}
else{
utils.logWarning(
"Something is going wrong with ArtifactCache service which supports cache actions. Please check https://www.githubstatus.com/ for any ongoing issue in actions."
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like big enough repeated code and can think about moving to utils

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to function

@tiwarishub tiwarishub marked this pull request as ready for review March 28, 2022 10:28
@tiwarishub tiwarishub requested a review from a team as a code owner March 28, 2022 10:28
README.md Outdated
- `action/cache` is currently not supported on GitHub Enterprise Server. <https://github.com/github/roadmap/issues/273> is tracking this.

Since GitHub Enterprise Server uses self-hosted runners, dependencies are typically cached on the runner by whatever dependency management tool is being used (npm, maven, etc.). This eliminates the need for explicit caching in some scenarios.

## Changelog schedule and history

| Status | Version | Date | Highlights |
Copy link
Contributor

Choose a reason for hiding this comment

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

Add to this changelog with version number which can be updated when publishing.
Also move this changelog to a separate file named CHANGELOG

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a file with the name RELEASE.md because found this nomenclature is used toolkit as well.

"Cache action is not supported on GHES. See https://github.com/actions/cache/issues/505 for more details"
});

test("save on ghes without AC available should no=op", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

no-op

@aparna-ravindra
Copy link
Contributor

How do we update both version actions/cache@v2 and actions/cache@v3 ?

@@ -0,0 +1,7 @@
# Releases

### 3.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

we support actions/cache@v2 as well. We could add the latest version of v2 here in the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that i will update on when i will raise PR for actions/cache@v2

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we will only support v3 for ghes 3.5 as that has runner with node 16. @ashwinsangem do we support v2 on 3.5?

Copy link
Contributor

Choose a reason for hiding this comment

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

We won't support v2 in GHES 3.5 as the minimum runner version is updated to node16. Full discussion here.

Copy link
Contributor Author

@tiwarishub tiwarishub Mar 29, 2022

Choose a reason for hiding this comment

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

okay, then we will not port these changes on v2 version.

@tiwarishub
Copy link
Contributor Author

tiwarishub commented Mar 28, 2022

How do we update both version actions/cache@v2 and actions/cache@v3 ?

I am planning to create a release branch releases/v2.1.8 which will be branched out from #651 PR commit. And then will cherry-pick my PR changes to this branch

@tiwarishub tiwarishub merged commit 136d96b into main Mar 30, 2022
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.

None yet

5 participants