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.

Sign up for GitHub

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

Jump to bottom

Add Patroni updates for new static_primary config option #224

Merged
merged 1 commit into from
May 18, 2022
Merged

Add Patroni updates for new static_primary config option #224

merged 1 commit into from
May 18, 2022

Conversation

thedodd
Copy link
Member

@thedodd thedodd commented Apr 5, 2022

@thedodd thedodd self-assigned this Apr 5, 2022
alexeyklyukin
alexeyklyukin approved these changes Apr 5, 2022
View reviewed changes
Copy link
Member

@alexeyklyukin alexeyklyukin 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!

@alexeyklyukin
Copy link
Member

I'd suggest to wait before merging it until Patroni PR is reviewed. One suggestion, if you don't plan to merge your Patroni PR to make it easy to submit it upstream, is to tag a final commit and use that tag here (immutable) instead of a pr (moving target).

@thedodd
Copy link
Member Author

thedodd commented Apr 5, 2022

@alexeyklyukin yes sir, that is actually exactly what I did in this case. The code that is copied into the container is based on a tag which I've added to our internal Patroni fork :). EG, the v2.2.0-beta.0 in https://raw.githubusercontent.com/timescale/patroni/v2.2.0-beta.0/patroni/ha.py being pulled into the container.

@thedodd thedodd force-pushed the patroni-updates branch 3 times, most recently from 07305f7 to 7408561 Compare April 5, 2022 19:19
@feikesteenbergen
Copy link
Member

@thedodd It seems it fails, as at the point you're doing the wget you are already the postgres user.

You should do this as the root user, so perhaps switch to the root user first and then switch back to postgres?

feikesteenbergen
feikesteenbergen reviewed Apr 6, 2022
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@thedodd thedodd force-pushed the patroni-updates branch 2 times, most recently from 8572f78 to 5b8f23f Compare April 6, 2022 13:50
@thedodd thedodd changed the title Add Patroni updates for static_single_node support Add Patroni updates for new static_primary config option Apr 22, 2022
@thedodd
Copy link
Member Author

thedodd commented Apr 22, 2022

Ok, this has now been updated based on the v2.2.0-beta.1 tag on the Patroni PR.

alexeyklyukin
alexeyklyukin reviewed Apr 26, 2022
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@thedodd thedodd force-pushed the patroni-updates branch 2 times, most recently from df49865 to b2495f1 Compare April 28, 2022 18:34
thedodd
thedodd commented Apr 28, 2022
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
feikesteenbergen
feikesteenbergen approved these changes May 9, 2022
View reviewed changes
Copy link
Member

@feikesteenbergen feikesteenbergen 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 to me, especially as we now do not touch the default Docker Image being built!

.github/workflows/publish_image.yaml Outdated Show resolved Hide resolved
@thedodd
Add Patroni updates for static_primary support
639ee96 
Use build arg to optionally include Patroni changes. Added a new release
matrix element to stamp-out a release image with the Patroni changes
enabled, bearing a tag suffix `-patroni-static-primary`
@thedodd thedodd merged commit 63246af into master May 18, 2022
@thedodd thedodd deleted the patroni-updates branch May 18, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexeyklyukin alexeyklyukin alexeyklyukin approved these changes

@feikesteenbergen feikesteenbergen feikesteenbergen approved these changes

@ncalibey ncalibey ncalibey approved these changes

Assignees

@thedodd thedodd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thedodd @alexeyklyukin @feikesteenbergen @ncalibey