-
Notifications
You must be signed in to change notification settings - Fork 55
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
[WIP] vmpublish.rb: publish public images #154
Conversation
This script publishes builds from a previous vmbuild.rb invocation to any of the configurated public directories. Currently this means Atlas (for Vagrant images) and GCE (for GCE images). This code could probably use a similar metadata driven approach as vmbuild.rb. However for now I believe the current approach of hardcoding some of the calling conventions and parameters for each of the supported targets works.
Checked commit https://github.com/geertj/manageiq-appliance-build/commit/19ff7a8fcbf18f92f754e201bd2abdba7ee98734 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 scripts/vmbuild.rb
scripts/vmpublish.rb
|
FILE_SERVER = ENV["BUILD_FILE_SERVER"] # SSH Server to host files | ||
FILE_SERVER_ACCOUNT = ENV["BUILD_FILE_SERVER_ACCOUNT"] # Account to SSH as | ||
FILE_SERVER_BASE = Pathname.new(ENV["BUILD_FILE_SERVER_BASE"] || ".") # Subdirectory of Account where to store builds | ||
|
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.
@geertj from a reviewer perspective, I have to scroll up and down to see if this code was just moved or moved and modified. It looks identical, but moved. Can you do the move to a new file in a single commit so I can review commit by commit and not have to keep all of these changes in my head?
|
||
FILE_SERVER = ENV["BUILD_FILE_SERVER"] # SSH Server to host files | ||
FILE_SERVER_ACCOUNT = ENV["BUILD_FILE_SERVER_ACCOUNT"] # Account to SSH as | ||
FILE_SERVER_BASE = Pathname.new(ENV["BUILD_FILE_SERVER_BASE"] || ".") # Subdirectory of Account where to store builds |
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.
So, we need to put these constants within the Build
namespace. Meaning, they should reside in module Build
. We can do this later but we really should have them in that namespace and all code that uses it, if it's in that namespace, can access it without needing to specify the namespace, meaning we can use BUILD_BASE
within the namespace instead of Build::BUILD_BASE
. When we put it in the Build
namespace, it needs to live in a build directory, so scripts/build/config.rb
...
Because it's relevant here but not required... I created #158 for the following comment:
Future PR: Note, we really need to move files to the right places and re-namespace things...
vmbuild.rb -> build.rb
cli.rb -> build/cli.rb
git_checkout.rb -> build/git_checkout.rb
kickstart_generator.rb -> build/kickstart_generator.rb
productization.rb -> build/productization.rb
target.rb -> build/target.rb
productization.rb -> build/productization.rb
uploader.rb -> build/uploader.rb
Also, rename scripts to lib.
@@ -0,0 +1,80 @@ | |||
require 'logger' |
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.
Can we name this publish.rb
? We really need to rename vmbuild.rb to build.rb since that file adds the Build
module.
Sorry for the delay guys. Let's try to get resolution on the design issue first: whether this in the first instance needs to be a separate executable program, or be part of I proposed three reasons in favor of a separate program, and some of those were rightfully challenged. But taking a step back, I think the reason behind all of these is my intuition shaped by the Unix philosophy: small utilities that do one thing and do it well, and that communicate via pipes (or files). The benefits are that the individual program become easier to develop and debug. Errors are localized, and everything becomes easier to test (I realize we have unit tests and stuff.. but still) And we can also use the best tool for the job. Take for example #161 where I add support for uploading to EC2. I decided to use Ansible for that given that I needed a mix of API and secure shell actions. Ansible makes that very easy to do. The smaller program can then be tied together with a script if we need to. One consequence is that we need to be less aggressive cleaning up the files in |
This pull request is not mergeable. Please rebase and repush. |
This pull request has been automatically closed because it has not been updated for at least 6 months. Feel free to reopen this pull request if these changes are still valid. Thank you for all your contributions! |
This script publishes builds from a previous vmbuild.rb invocation to
any of the configurated public directories. Currently this means Atlas
(for Vagrant images) and GCE (for GCE images).
This code could probably use a similar metadata driven approach as
vmbuild.rb. However for now I believe the current approach of hardcoding
some of the calling conventions and parameters for each of the supported
targets works.