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

Switching from staging to archive module #243

Merged
merged 2 commits into from
Mar 24, 2016

Conversation

hopperd
Copy link
Contributor

@hopperd hopperd commented Mar 17, 2016

Since the staging module won't be receiving any TLC anymore we'd probably do well to upgrade to the module that replaces it, the puppet/archive module. This will provide checksum support down the road (currently somewhat experimental with urls).

@hopperd hopperd self-assigned this Mar 17, 2016
@hopperd hopperd mentioned this pull request Mar 17, 2016
@hopperd hopperd changed the title Fixes 242 by switching archive module Fixes 242 by switching to archive module Mar 17, 2016
@hopperd hopperd changed the title Fixes 242 by switching to archive module Switching from staging to archive module Mar 17, 2016
@solarkennedy
Copy link
Contributor

This looks great! I'm happy we can finally move to the archive module.

@aj-jester had previously attempted this and found some gotchas:
#178

It sounds like this may be potentially disruptive to some, but I think long term we have to do it.

@hopperd
Copy link
Contributor Author

hopperd commented Mar 18, 2016

The only gotcha I think we need to be concerned with is the first run one. And I think we can get around this by specifying the install path to be the same one that is used previously by staging /opt/staging. This is specified by the install_path variable that is set. So I think that would probably get us around that issue if that is the only major concern.

It would be possible for us to also possibly build in a check for the staging folder path and if that version exists let everything download and relink but just not restart. That should be adequate as well.

I'm open to either solutions you guys want to go with. It might be more challenging changing the notify as it might not be in quote and accessible, but we could see about cancelling the refresh (not sure I've done that before though)

@hopperd
Copy link
Contributor Author

hopperd commented Mar 18, 2016

Another option would be to use the consul_version fact we have to see if it is the same version that is being downloaded and relinked and not reload if it is.

@hopperd hopperd force-pushed the feature/switch-to-archive-242 branch from 060b0b6 to 67e0d57 Compare March 18, 2016 14:34
@hopperd
Copy link
Contributor Author

hopperd commented Mar 18, 2016

Ok added additional tests and rebased.

@hopperd
Copy link
Contributor Author

hopperd commented Mar 18, 2016

Also tested this on a few systems and archive downloaded the artifact and extracted, and updated the symlink but didn't restart the service. So I think we've covered the biggest concern.

@solarkennedy
Copy link
Contributor

@Split3 I've uploaded a new commit to this branch that fixes the integration tests. (`bundle exec rake beaker)

It looks like thew web_ui download gets installed into /opt/puppet-archive/consul-0.6.4_web_ui can you confirm this is correct? If so I say we ship.

@hopperd
Copy link
Contributor Author

hopperd commented Mar 21, 2016

@solarkennedy Yes I kept the same functionality we had previously to use the symlinks for both the executable along with the ui so that is expected.

@hopperd
Copy link
Contributor Author

hopperd commented Mar 21, 2016

@solarkennedy Thanks for fixing the acceptance tests, any reason we don't run those are part of travis builds? Sorry wasn't running those locally since I didn't see them as part of travis.

@hopperd
Copy link
Contributor Author

hopperd commented Mar 23, 2016

@solarkennedy so are we good here now?

@solarkennedy
Copy link
Contributor

I'm very sick right now and I don't have a recovery eta. Please continue
and use standard best judgment. Thank you for helping in my absence.
Zzzzzz....
On Mar 23, 2016 5:49 AM, "Daniel Hopper" notifications@github.com wrote:

@solarkennedy https://github.com/solarkennedy so are we good here now?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#243 (comment)

@hopperd
Copy link
Contributor Author

hopperd commented Mar 24, 2016

Sorry to hear @solarkennedy, rest up and get better. I'll go ahead and merge this in and we can make adjustments as needed.

@hopperd hopperd merged commit 433702f into master Mar 24, 2016
@hopperd hopperd deleted the feature/switch-to-archive-242 branch March 24, 2016 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants