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

fixes #15014 - restore pulp db init flag #145

Merged
merged 1 commit into from Jun 7, 2016

Conversation

stbenjam
Copy link
Member

@stbenjam stbenjam commented May 11, 2016

This brings back the init flag so that the puppet installer only
runs the migration once or when the user has run --reset (which
deletes the init.flag file)

@ekohl
Copy link
Member

ekohl commented May 11, 2016

ACK. I recall there was an issue where someone was complaining the module wasn't idempotent because mongodb was restarted on every run. I can't find it now, but it looks like this should fix it.

@stbenjam
Copy link
Member Author

stbenjam commented May 13, 2016

Hm, not sure if this will fix that, this will cause a migrate whenever the service was down and puppet started it. Since the yum upgrade could happen out of band, this is the only way I could think of to ensure the database schema is up-to-date when we run it.

@ehelms
Copy link
Member

ehelms commented May 13, 2016

On the surface, this seems like a strange paradigm, every time the database is started migrate it. Does the foreman module for example follow this paradigm?

@stbenjam
Copy link
Member Author

On the surface, this seems like a strange paradigm, every time the database is started migrate it. Does the foreman module for example follow this paradigm?

It is a very strange to do, yes. Do note, it's not every time the database is started, only when puppet does it.

The problem with --reset is the mongodb is never getting migrated. So, my idea was to leave it off in the reset pre-hook, so puppet starts it and triggers the migrate.

Do you think there's some other thing we could subscribe to that indicates the DB needs migrating? Maybe re-introduce /var/lib/pulp/init.flag which seems to have been removed?

@stbenjam
Copy link
Member Author

(init.flag and the reset stuff was removed in #65)

@beav
Copy link
Contributor

beav commented May 18, 2016

I got bit by this bug yesterday, 👍 from me for this patch. Running pulp-manage-db is pretty quick, especially after a --reset 🐨

@ehelms
Copy link
Member

ehelms commented May 20, 2016

If @cristifalcas can tell us why he removed it (whether by accident or with an intent) we could maybe bring back the init file otherwise the only thing I could think of would be tying the migrate to the creation of the dbpath so that it migrates whenever that is refreshed/recreated. Failing any of that I'll be OK with this change.

@cristifalcas
Copy link
Contributor

Hi, from what I remember, I did it because I didn't see any purpose for it. Also it was stopping future migrations if nobody removed the file. It may be useful for tests, but in production not so much.

@stbenjam
Copy link
Member Author

I did it because I didn't see any purpose for it.

It's used in production by Katello, the --reset command to the installer removes the file. Perhaps the database path for mongo makes more sense than the init flag file

Also it was stopping future migrations if nobody removed the file.

This will never happen now anyway, a change on a require doesn't trigger refresh on puppet resource, AFAIK.

@@ -23,7 +23,7 @@
logoutput => 'on_failure',
user => 'apache',
refreshonly => true,
require => File['/etc/pulp/server.conf'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is still need it because the migration should run after all needed information is populated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose, is there a reason this is an include, and not between config + service?

https://github.com/Katello/puppet-pulp/blob/master/manifests/init.pp#L377

@stbenjam
Copy link
Member Author

stbenjam commented Jun 1, 2016

If there's no better suggestions, I think I'll go with just putting the init flag back.

@stbenjam stbenjam changed the title fixes #15014 - pulp db migration on mongo start fixes #15014 - restore pulp db init flag Jun 1, 2016
This brings back the init flag so that the puppet installer only
runs the migration once or when the user has run --reset (which
deletes the init.flag file).
@stbenjam
Copy link
Member Author

stbenjam commented Jun 7, 2016

Ready for another look

@jlsherrill
Copy link
Contributor

ACK, tested and worked for me.

@stbenjam stbenjam merged commit 9584d7b into theforeman:master Jun 7, 2016
@stbenjam
Copy link
Member Author

stbenjam commented Jun 7, 2016

Thanks!

@stbenjam stbenjam deleted the fix-reset branch June 7, 2016 19:47
cegeka-jenkins pushed a commit to cegeka/puppet-pulp that referenced this pull request Oct 23, 2017
This brings back the init flag so that the puppet installer only
runs the migration once or when the user has run --reset (which
deletes the init.flag file).
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.

None yet

6 participants