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

Enhance readme #30

Merged
merged 1 commit into from
Feb 5, 2020
Merged

Enhance readme #30

merged 1 commit into from
Feb 5, 2020

Conversation

SaitTalhaNisanci
Copy link
Contributor

No description provided.


## Introduction

This repository contains the source code for docker images that are used in [citus testing](https://github.com/citusdata/citus/blob/master/.circleci/config.yml). The images are pushed to [docker hub](https://hub.docker.com/u/citus). There is no hooking logic between this repository and the docker hub account. It is used purely for the storage of our images source code.
Copy link
Member

Choose a reason for hiding this comment

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

Can we make an issue to track a task to automate the image creation from a CI job? Running upgrades on developer machines have proven to be troublesome, as we had to create a new docker hub account due to access restrictions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#33.

- The script (`install-and-test-ext`) takes the target name as an argument, and the following command is used to run the test:
While building the image:

* Install postgres major which is taken as an argument `PG_MAJOR`.
Copy link
Member

Choose a reason for hiding this comment

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

most images would and should be taken from the original postgres releases, I think this line does not reflect that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well if we install pg12 from the postgres base image, when we try to install libcurl, it installs libcurl4 and citus does not want libcurl4 but libcurl3. We can add libcurl3 package source to our apt/source.list but in the future another package can cause a problem too. And in the extbuilder image we also dont install from pg base image as we have multiple pg majors. So installing the same way as extbuilder seemed okay to me, what do you think?


Because of the specific logic that we have to run tests faster, we have a docker image `debbuilder` that generates a debian package for `postgresql-server-dev-{pg-major}`. The generated package is in the built docker image, specifically in `{container-name}/home/circleci/debs`. The package is pushed to [the-process](https://packagecloud.io/citus-bot/the-process). This package place is added to apt source list and pinned with with the highest priority so that it is chosen over the standard `postgresql-server-dev-{pg-major}` package.

The installation of postgres packages is at the time of building the image to reduce testing time. However this means that once the image is built the postgres version is fixed. You will need to rebuild the image if you want to upgrade the image, upgrading the image for a minor version is easier compared to upgrading a major version.
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't changing the major version for most containers entail to rebuild with a different PG_MAJOR build arg? I think only for pg upgrade tests, where we need multiple postgres versions, and the extbuilder, where we also need multiple postgres versions, the build is slightly harder as we need to change some of the scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For many images yes. What should be done is explained for each image, is there something that you think is redundant or can be simplified?

README.md Outdated

### 1.1. How circleci uses our images in testing

Our [dockerhub account](https://hub.docker.com/u/citus) has all the images here expect the `debbuilder`. When a change is committed, circleci:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Our [dockerhub account](https://hub.docker.com/u/citus) has all the images here expect the `debbuilder`. When a change is committed, circleci:
Our [dockerhub account](https://hub.docker.com/u/citus) has all the images here except the `debbuilder`. When a change is committed, circleci:

@JelteF JelteF merged commit bf23be7 into master Feb 5, 2020
@JelteF JelteF deleted the enhance/README branch February 5, 2020 13:13
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