-
Notifications
You must be signed in to change notification settings - Fork 27
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
packer/build_images.sh: get version and release values from pipline #558
packer/build_images.sh: get version and release values from pipline #558
Conversation
1675f57
to
b59fce3
Compare
packer/build_image.sh
Outdated
if [ -z "$TARGET" ]; then | ||
echo "Missing --target parameter. Please specify target cloud (aws/gce/azure)" | ||
exit 1 | ||
fi | ||
|
||
SSH_USERNAME=ubuntu | ||
|
||
SCYLLA_FULL_VERSION="$VERSION-$SCYLLA_RELEASE" | ||
SCYLLA_MACHINE_IMAGE_VERSION="$VERSION-$SCYLLA_MACHINE_IMAGE_RELEASE" | ||
|
||
if [ $LOCALDEB -eq 1 ]; then | ||
INSTALL_ARGS="$INSTALL_ARGS --localdeb" |
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.
BTW, what about the option of --localdeb
I'm not sure it will work now. Actually I'm not sure we are using 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.
We are not using it for sure. it's for someone who builds locally and then needs an AMI
We can obliterate it as we have today BYO can do that instead, WDYT?
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 BYO it is a good alternative.
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 will add another commit to remove it
related to changes in scylladb/scylla-pkg#4635, modifying the script to get `version` and `release` values from the pipeline to remove the constrain between dpackager OS and image OS Should be merged together with scylladb/scylla-pkg#4635
b59fce3
to
ee479de
Compare
@Annamikhlin Added another commit with the cleanup |
@Annamikhlin let me know if you have any more comments |
packer/build_image.sh
Outdated
@@ -36,19 +34,13 @@ print_usage() { | |||
echo " --ec2-instance-type Set EC2 instance type to use while building the AMI. If empty will use defaults per architecture" | |||
exit 1 | |||
} | |||
LOCALDEB=0 | |||
DOWNLOAD_ONLY=0 |
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.
DOWNLOAD_ONLY
- can be removed as well
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.
removed
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.
small comment, other than LGTM
Since both option are not really in use anymore, and in case we need to build a custom image we can do it using BYO
7d6d53a
to
264dd07
Compare
related to changes in https://github.com/scylladb/scylla-pkg/pull/4635, modifying the script to get
version
andrelease
values from the pipeline to remove the constrain between dpackager OS and image OSShould be merged with https://github.com/scylladb/scylla-pkg/pull/4635
Testing