-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add GA workflow to update AtoM's Vagrant box #305
Conversation
description: | ||
description: Description | ||
required: true |
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'd love to see a textarea input in here but it's currently not possible. The easiest solution I found is to add <br/>
directly in the input and those are represented in Vagrant Cloud's GUI as expected, but they break the Markdown support. We could try to add some extra formatting in the Ruby script to accept other linebreaks (like \n
) but I'm not sure if it worths the effort.
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.
One potential improvement for the future is to maintain a CHANGELOG.md document (there are many changelog parsers or plain markdown parsers) in this repo that is read by this workflow to extract the description field from there - it's somewhat more reproducible too since the build inputs will be versioned.
jobs: | ||
vagrant-box-atom: | ||
name: Build and upload | ||
runs-on: macos-10.15 |
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.
MacOS required for nested virtualization:
actions/runner-images#183
actions/runner-images#433
It also comes with all the required dependencies (Virtualbox, Vagrant, Packer, Bundler).
bundle install | ||
ruby upload.rb \ | ||
'${{ github.workspace }}/atom-vagrant-${{ github.event.inputs.version }}.box' \ | ||
'${{ secrets.VAGRANT_CLOUD }}' \ |
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.
Already generated and added to the repo.
client = VagrantCloud::Client.new(access_token: token) | ||
|
||
client.box_version_create( | ||
username: ORG, | ||
name: BOX, | ||
version: version, | ||
description: description | ||
) | ||
|
||
client.box_version_provider_create( | ||
username: ORG, | ||
name: BOX, | ||
version: version, | ||
provider: PROVIDER | ||
) | ||
|
||
upload_url = client.box_version_provider_upload( | ||
username: ORG, | ||
name: BOX, | ||
version: version, | ||
provider: PROVIDER | ||
) |
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.
If anything fails here (authentication, version already exists, etc.) the script ends with error. However, this may leave residual data in Vagrant Cloud that won't be considered in a a new run. For example, if the box fails to upload below and the workflow is triggered again with the same version it will fail because the version already exists.
We could consider those cases in the script but I'm not sure if we should, as it may lead to versions overwrite. We could document the situation so, in case of failure, if the version has been created it has to be deleted from Vagrant Cloud's GUI before trying again with the same version.
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 guess we could check if the version is fully released to avoid that overwrite, but I still have my doubts about if it's worth it, feedback is welcomed.
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.
For error handling, maybe one option is a workflow job with a status condition that does the necessary clean-up?
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.
Thanks @sevein, I think it may be easier directly in the script, but it is an enhancement that could be added later on. For now I'll add a note in the internal wiki about failure/retry.
require 'vagrant_cloud' | ||
|
||
ORG = 'artefactual' | ||
BOX = 'atom' |
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.
At the moment it's all named for AtoM's Vagrant box, but we could accept this as a script argument and reuse the same script for Archivematica's Vagrant box, if we want to implement a similar workflow.
path = ARGV[0] | ||
token = ARGV[1] | ||
version = ARGV[2] | ||
description = ARGV[3] |
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.
We could use Ruby's option parser to improve the script arguments/options management and document the script usage if needed.
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.
This is useful! A future improvement could be to push a git tag as soon as you're done with the workflow and it executed successfully, if we wanted to track that sort of thing in this repository.
A common git tag naming pattern in monorepos is subproject/version
, e.g. vagrant-box-atom/${version}
, e.g. gopls/v0.6.10
. I don't think we really need another action just to push a git tag, but there's action-push-tag for reference.
I'm aware we don't have a lot of documentation in this repo, but a tools/vagrant-box-atom/README.md shortly describing its purpose would be perfect.
tools/vagrant-box-atom/upload.rb
Outdated
@@ -0,0 +1,59 @@ | |||
#!/usr/bin/ruby |
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.
If it's not a big deal I'd probably change it to:
#!/usr/bin/env ruby
That ensures compatibility with other platforms where the binary sits in other locations.
description: | ||
description: Description | ||
required: true |
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.
One potential improvement for the future is to maintain a CHANGELOG.md document (there are many changelog parsers or plain markdown parsers) in this repo that is read by this workflow to extract the description field from there - it's somewhat more reproducible too since the build inputs will be versioned.
|
||
PLATFORMS | ||
ruby | ||
x86_64-linux |
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 guess it doesn't really matter since you're not publishing anything but shouldn't this platform be a "darwin" something? Feel free to ignore.
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.
TBH, I have no idea, this is what bundle install
created and seems to be working in both Linux and MacOS.
client = VagrantCloud::Client.new(access_token: token) | ||
|
||
client.box_version_create( | ||
username: ORG, | ||
name: BOX, | ||
version: version, | ||
description: description | ||
) | ||
|
||
client.box_version_provider_create( | ||
username: ORG, | ||
name: BOX, | ||
version: version, | ||
provider: PROVIDER | ||
) | ||
|
||
upload_url = client.box_version_provider_upload( | ||
username: ORG, | ||
name: BOX, | ||
version: version, | ||
provider: PROVIDER | ||
) |
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.
For error handling, maybe one option is a workflow job with a status condition that does the necessary clean-up?
Thanks @sevein! About the changelog and the tag, I'm not sure due to how this repo is managed/named and all the other repositories that are involved, a new Vagrant box may be needed for many reasons and most of them won't involve this repository. |
7dfa318
to
e6ef297
Compare
Add a manually triggered Github actions workflow, accepting `version` and `description` as inputs, to build and upload AtoM's Vagrant box. Use a Ruby script to upload the box with Hashicorp's Ruby client for the Vagrant Cloud API.
e6ef297
to
d7ac09a
Compare
Add a manually triggered Github actions workflow, accepting
version
and
description
as inputs, to build and upload AtoM's Vagrant box. Usea Ruby script to upload the box with Hashicorp's Ruby client for the
Vagrant Cloud API.
Refs https://trello.com/c/vMINa4M8/323-atom-vagrant-box-automated-build-and-upload.