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

acceptance: update how we build the python compose image #81313

Merged
merged 1 commit into from
May 17, 2022

Conversation

rickystewart
Copy link
Collaborator

Up until this point we were using apt install postgresql-client to
install the psql binary in this image. Because the postgresql-client
apt package was apparently updated to a later version of postgres
last week that includes this change,
the TestComposeGSSPython acceptance test started failing with the
following error:

psql: error: private key file "/certs/client.root.key" must be owned by the current user or root

Less recent versions of psql are more permissive about cert
permissions, so we work around this by simply copying a less recent
version of psql from the older postgres:11 image.

Release note: None
Release justification: Test-only change

@rickystewart rickystewart requested a review from a team May 16, 2022 18:59
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart rickystewart force-pushed the pythonfix branch 3 times, most recently from f99d7aa to cac79d6 Compare May 16, 2022 22:37
Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

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

thanks for looking into this!!!

@otan
Copy link
Contributor

otan commented May 16, 2022

hmm, looks like it's failing with


        python_1     | Creating a k5s token...
        kdc_1        | krb5kdc: starting...
        python_1     | Password for [email protected]: 
        python_1     | Creating test user
        python_1     | Error: You must install at least one postgresql-client-<version> package

@rickystewart
Copy link
Collaborator Author

Yep, I think I'll have to do a couple more iterations on this. 🙄 Very annoying. Stay tuned.

Up until this point we were using `apt install postgresql-client` to
install the `psql` binary in this image. Because the `postgresql-client`
`apt` package was apparently updated to a later version of `postgres`
last week that includes [this change](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa),
the `TestComposeGSSPython` `acceptance` test started failing with the
following error:

```
psql: error: private key file "/certs/client.root.key" must be owned by the current user or root
```

Less recent versions of `psql` are more permissive about cert
permissions, so we work around this by manually installing postgres 11.

Release note: None
Release justification: Test-only change
@rickystewart
Copy link
Collaborator Author

bors r=otan

@craig
Copy link
Contributor

craig bot commented May 17, 2022

Build succeeded:

@blathers-crl
Copy link

blathers-crl bot commented May 17, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from ecf90bc to blathers/backport-release-21.1-81313: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.1.x failed. See errors above.


error creating merge commit from ecf90bc to blathers/backport-release-21.2-81313: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.2.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

rickystewart added a commit to rickystewart/cockroach that referenced this pull request May 19, 2022
`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see cockroachdb#81313, cockroachdb#81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
craig bot pushed a commit that referenced this pull request May 19, 2022
81388: authors: add chinemeremchigbo to authors r=lassenordahl a=ChinemeremChigbo

authors: add chinemeremchigbo to authors

Release note: None
Release justification: non-production code change

81460: acceptance: run `python`, `psql` containers as current uid r=knz a=rickystewart

`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see #81313, #81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None

81464: ui: Add tests for the TimeScaleDropdown component r=jocrl a=jocrl

This commit splits out tests in `timescale.spec.tsx` and adds additional tests
for the TimeScaleDropdown component, including testing that the custom time
picker is initialized to the currently selected time. This also adds
TimeScaleDropdown stories.

Release note: None

81486: sql/backfill: fix bug adding new columns to new index with volatile default r=ajwerner a=ajwerner

In #58990 we added support for the index backfiller to evaluate expressions. This unblocked the usage of virtual computed columns in secondary expressions, so wasn't a totally bogus change, but probably was trying to do too much without understanding all of the implications. The original motivation for that change was to unblock the then nascent declarative schema changer prototype which wanted to only implement #47989 as a column change protocol. In order to do that, it needed to evaluate default expressions when adding new columns to a new primary index. For whatever reason, probably lack of understanding, I made it such that all default expressions for columns which were in the adding state were evaluated. This is wrong in cases where the column has already been backfilled into the current primary index; it's wrong because volatile expressions will evaluate to different values causing the newly backfilled secondary index to be bogus and corrupt. 

This change addresses the failure of the above change by being more thoughtful about whether it's sane to evaluate a default expression when backfilling into an index. Note that it's still insane to backfill a volatile expression for a new column into the key of a new primary index. As of writing, there is no way to do this. 

Backports will address #81448.

Release note (bug fix): In 21.1 a bug was introduced whereby default values
were recomputed when populating data in new secondary indexes for columns
which were added in the same transaction as the index. This arises, for example
in cases like `ALTER TABLE t ADD COLUMN f FLOAT8 UNIQUE DEFAULT (random())`.
If the default expression is not volatile, then the recomputation is harmless.
If, however, the default expression is volatile, the data in the secondary
index will not match the data in the primary index: a corrupt index will have
been created. This bug has now been fixed.

81504: acceptance/roachtest: update ORM versions under test r=ecwall a=rafiss

See DXTPT-35

Release note: None

81523: sql: fix typo in warning on using DateStyle/IntervalStyle r=rafiss a=otan

Release note (sql change): Fixed a small typo when using DateStyle and
IntervalStyle.

Co-authored-by: Chinemerem <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Josephine Lee <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
blathers-crl bot pushed a commit that referenced this pull request May 19, 2022
`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see #81313, #81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this pull request May 19, 2022
`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see cockroachdb#81313, cockroachdb#81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this pull request May 24, 2022
Manual cherry-pick from cockroachdb#81460.

`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see cockroachdb#81313, cockroachdb#81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
andrewbaptist pushed a commit to andrewbaptist/cockroach that referenced this pull request May 25, 2022
`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see cockroachdb#81313, cockroachdb#81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this pull request Jun 23, 2022
`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see cockroachdb#81313, cockroachdb#81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
yuzefovich pushed a commit to yuzefovich/cockroach that referenced this pull request Jun 30, 2022
`postgres`'s permission checking for certificates has gotten more
rigorous since [this commit](https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=a59c79564bdc209a5bc7b02d706f0d7352eb82fa).
This has broken a couple `acceptance` tests which do not pin to any
specific `postgres` version (see cockroachdb#81313, cockroachdb#81437).

Here we attempt to solve the problem "once and for all" by ensuring that
these containers run with a UID that is equal to the one that created
the certificates.

Release note: None
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