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

cdh-web QA deploy playbook #49

Merged
merged 54 commits into from
Dec 15, 2020
Merged

cdh-web QA deploy playbook #49

merged 54 commits into from
Dec 15, 2020

Conversation

thatbudakguy
Copy link
Contributor

@thatbudakguy thatbudakguy commented Dec 4, 2020

I'm currently able to run the playbook successfully start to finish, but http://cdh-test-web1.princeton.edu/ times out (I am on VPN). You need to have the community.general ansible collection installed to run it; see the README for the command to do that. You can run the playbook with: ansible-playbook playbooks/cdh-web_qa.yml.

Tons of changes, since this involved touching virtually every role in some way, although for many of them it was just adding privilege escalation (and maybe a default or two). I also haven't messed with the tests for the postgresql role yet. Hoping more eyes on it will help clean up this massive PR.

thatbudakguy and others added 30 commits December 1, 2020 15:52
@rlskoeser
Copy link
Contributor

@thatbudakguy cdh-test-web1 is the server name, not the url — the ones we'd planned to use are documented here:
pulibrary/princeton_ansible#1798

— That reminds me, we'll need to update the allowed hostnames config.

Copy link
Contributor

@rlskoeser rlskoeser left a comment

Choose a reason for hiding this comment

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

Some comments and questions, but generally I think this looks good. Was able to pull the latest version of the branch and run the molecule tests successfully.

You tested the new deploy, but might be good to test an old qa deploy also to make sure they still work.

README.md Show resolved Hide resolved
group_vars/cdhweb_qa/vars.yml Outdated Show resolved Hide resolved
playbooks/cdh-web_qa.yml Show resolved Hide resolved
roles/postgresql/tasks/main.yml Show resolved Hide resolved
roles/postgresql/tasks/main.yml Outdated Show resolved Hide resolved
roles/postgresql/tasks/backup_db.yml Outdated Show resolved Hide resolved
roles/postgresql/tasks/backup_db.yml Show resolved Hide resolved
roles/deploy_user/molecule/default/verify.yml Outdated Show resolved Hide resolved
roles/postgresql/molecule/default/verify.yml Outdated Show resolved Hide resolved
roles/postgresql/molecule/default/verify.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@rlskoeser rlskoeser left a comment

Choose a reason for hiding this comment

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

Updates since last time all look good to me.

Copy link
Collaborator

@kayiwa kayiwa left a comment

Choose a reason for hiding this comment

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

Love your deploy user tests!
postgresql looks good also.

@thatbudakguy
Copy link
Contributor Author

@rlskoeser two things still need a decision before merging, I think:

  • should we try to add tests for the postgres role? there still aren't any that use the PUL approach of running postgres locally to test against.
  • should we try your suggestion of not making database backups if the database has only just been created? seems like low to medium effort, but we would also probably want to test it.

@rlskoeser
Copy link
Contributor

@thatbudakguy thanks for identifying what's left and where you need me to weigh in.

At this point, I don't want to hold up the migration work any further.

@rlskoeser two things still need a decision before merging, I think:

  • should we try to add tests for the postgres role? there still aren't any that use the PUL approach of running postgres locally to test against.

I'd like to put this off. Would you make an issue to add a test? Can we set up a time down the road to pair with Francis on that, or borrow from the example hello world postrges role work he was doing for us?

  • should we try your suggestion of not making database backups if the database has only just been created? seems like low to medium effort, but we would also probably want to test it.

I don't think it's worth holding up the deploy, since it's pretty minor and not a huge downside (not a heavy cost for backing up an empty database). Should we make an issue to document it? I don't think it's a huge priority, but might be easy to fold in and test once we have the postgres role tested.

@thatbudakguy
Copy link
Contributor Author

created #52 and #53 to track the above. for the postgres tests (#52), i think we know how to do it, so we can implement it and then tag francis for review.

@rlskoeser rlskoeser merged commit 967f927 into main Dec 15, 2020
@rlskoeser rlskoeser deleted the cdh-web-qa-pul branch December 15, 2020 17:18
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