This repository has been archived by the owner on Sep 26, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2k
Alpine provisioner #4529
Closed
Closed
Alpine provisioner #4529
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2ca6cc5
Add Alpine provisioner, re docker/machine#3951.
pwhelan 09a83ce
Install community repo if docker is not found in installed repos
publicarray ac1a6ff
Alpine: Fix 'remove' is not an apk command
publicarray 2f8587b
Infer the community repo from the main repo URL
frebib cdf2165
fix add community repo to /etc/apk/repositories
publicarray File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this could be better for detecting whether community is enabled. Strictly speaking docker could be installed but community not although I'm not entirely sure in this case that it matters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you prefer
if ! which docker >/dev/null && ! apk info docker >/dev/null; ...
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll think about it. It's probably fine for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I've added the check in the last commit do you want me to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to summarise what (the updated) script does:
If the
docker
command is not found andapk
can't find docker in any installed repositories viaapk info docker
we continue to install the community repo. Next we retrieve the major and minor version from/etc/alpine-release
. The official mirror (CDN) for the appropriate version is appended to the/etc/apk/repositories
file. Finallyapk update
is used to refresh the local repository database.I think that this is pretty safe but if you have any ideas to improve this I'm open to improvements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just spent some time putting this together, tell me what you think:
This checks for the existence of an enabled community repo. If it's not present, it copies the
main
repository ands/main/community/
.From what I have tested this works in pretty much every case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think this is much better. Would you like to make the commit so you get credit for the script?
The only way i can think of where this would not work in the highly unlikely case where when 'main' is not installed. Maybe a user tries to run this script from the alpine.iso directly? from memory the iso is mounted as a cdrom and is the sole repository until setup-alpine is run. But again I don't think that will happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hum. I didn't consider that case. Ideally this would be more complex to account for that too. Maybe I should add the extra check in if the
main
repo doesn't exist either. I'll make a PR to your fork in a minuteThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to commit this later, but here is the revised statement: