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

Consolidate build and deploy scripts fixes #249 #321

Closed
wants to merge 1 commit into from

Conversation

rgee0
Copy link
Contributor

@rgee0 rgee0 commented Oct 21, 2017

Signed-off-by: rgee0 [email protected]

Description

Previously the build and deploy shell scripts were distinct entities for each supported hardware architecture. This change seeks to bring the separate scripts together and uses uname -m to switch the deploy script and build Dockerfile accordingly.

  • Also added queue package to the armhf Dockerfile to provide consistency with the x86_64 & arm64 gateways.

  • Added the ability to deploy docker-compose.extended.yml on x86_64 by passing in the argument of extended into ./deploy_stack.sh

  • Removed the defunct *.armhf.sh files.

Motivation and Context

Simplifies the build and deploy activities which consequently improves the user experience.
Fixes issue #249

  • I have raised an issue to propose this change (required)

How Has This Been Tested?

Tested that the shell scripts correctly switch to the relevant architecture type on x86_64, armv7 and aarch64.
Tried the extended argument on all three architectures to ensure that it only had a material effect on x86_64.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Looks good. Would also suggest a point of contact like an issue in the event of an unsupported configuration.

esac

if [ "$ARCH" = "x86_64" ] && [ "$1" = "extended" ] ; then
COMPOSEFILE="docker-compose.extended.yml"
Copy link
Member

Choose a reason for hiding this comment

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

Is there no extended compose file for arm yet? We should look at that because NATS has support as a multi-arch image now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#322 logged to look at extended compose file(s) for arm

@rgee0 rgee0 force-pushed the consolidateDeployScript branch from 57ab9ac to 77b4c06 Compare October 21, 2017 15:40
@rgee0
Copy link
Contributor Author

rgee0 commented Oct 21, 2017

@alexellis I've amended the not found messages to include a request that an issue be raised on GH.

@alexellis
Copy link
Member

Thank you. I'll merge into master once we have someone else test. What blogs and documentation do we need to update?

@alexellis
Copy link
Member

@rgee0 @JockDaRock @johnmccabe please can you help?

@alexellis
Copy link
Member

@rgee0 what remains to be done here? See also merge conflicts.

@rgee0
Copy link
Contributor Author

rgee0 commented Jan 1, 2018

@alexellis suggest closing this. Too long has passed. Things move on.

@rgee0 rgee0 closed this Jan 7, 2018
@alexellis
Copy link
Member

OK can you summarise where we were with it and how I should go about moving forward if I'm going to be handling this now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants