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

Remove aliases created by buildx when installing by default #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hashhar
Copy link

@hashhar hashhar commented Apr 25, 2022

If the action is configured to install buildx by default using the
input then docker buildx sets up docker build as an alias for buildx
making all docker build calls use the buildx builder instead of
traditional builders. The action didn't perform cleanup in this case to
uninstall the aliases which meant that any future workflows running on
same GitHub Actions runner would get the buildx builders even if it did
not explicitly request it.

This commit tracks if the aliases were installed and removes them during
post step of the action if so.

Fixes #125

If the action is configured to install buildx by default using the
input then docker buildx sets up docker build as an alias for buildx
making all docker build calls use the buildx builder instead of
traditional builders. The action didn't perform cleanup in this case to
uninstall the aliases which meant that any future workflows running on
same GitHub Actions runner would get the buildx builders even if it did
not explicitly request it.

This commit tracks if the aliases were installed and removes them during
post step of the action if so.

Signed-off-by: Ashhar Hasan <hashhar_dev@outlook.com>
@hashhar
Copy link
Author

hashhar commented Apr 26, 2022

Thanks for approving the CI run @tonistiigi. Looks green (kind of expected since no tests exist for the cleanup part and I couldn't figure out how to add one).

Let me know if there's something more you'd like me to do.

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! Needs a rebase and adapt following changes that have been made recently, thanks!

Comment on lines +130 to 143
if (stateHelper.IsBuildxDefaultBuilder) {
core.startGroup('Uninstalling build aliased to buildx');
await exec
.getExecOutput('docker', ['buildx', 'uninstall'], {
ignoreReturnCode: true
})
.then(res => {
if (res.stderr.length > 0 && res.exitCode != 0) {
core.warning(res.stderr.trim());
}
});
core.endGroup();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Since #204 and #213 we should have this in the post hook and handled through cleanup input.

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.

Cleanup aliases created by this action in post-step of action
2 participants