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

Bump scylla version to 5.4.6 and use fixed version for all of the nodes #187

Merged
merged 1 commit into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
name: Build
runs-on: ubuntu-latest
env:
SCYLLA_IMAGE: scylladb/scylla:5.2.9
SCYLLA_IMAGE: scylladb/scylla:5.4.6
steps:
- uses: actions/checkout@v2
- uses: actions/setup-go@v2
Expand Down
6 changes: 3 additions & 3 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ services:
timeout: 5s
retries: 18
node_2:
image: scylladb/scylla-nightly
image: scylladb/scylla-nightly:6.0.0-rc2-0.20240602.cbf47319c1f7
Copy link
Collaborator

Choose a reason for hiding this comment

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

@roydahan , @sylwiaszunejko , WDYT does it work to have somethign like SCYLLA_TABLET_IMAGE, instead of hardcoded one?

Only problem I see here is that driver-matrix test can use this either docker-compose.yaml or integration.sh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure, when we have 6.0 we would not need to have different scylla version for tablet/non-tablet tests and then there will be no need for SCYLLA_TABLET_IMAGE, we will just bump SCYLLA_IMAGE to scylladb/scylla:6.0.0 and use it for all of the nodes

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed

command: |
--experimental-features consistent-topology-changes
--experimental-features tablets
Expand All @@ -49,7 +49,7 @@ services:
timeout: 5s
retries: 18
node_3:
image: scylladb/scylla-nightly
image: scylladb/scylla-nightly:6.0.0-rc2-0.20240602.cbf47319c1f7
command: |
--experimental-features consistent-topology-changes
--experimental-features tablets
Expand All @@ -68,7 +68,7 @@ services:
node_2:
condition: service_healthy
node_4:
image: scylladb/scylla-nightly
image: scylladb/scylla-nightly:6.0.0-rc2-0.20240602.cbf47319c1f7
command: |
--experimental-features consistent-topology-changes
--experimental-features tablets
Expand Down
2 changes: 2 additions & 0 deletions testdata/config/scylla.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ client_encryption_options:
keyfile: /etc/scylla/db.key
truststore: /etc/scylla/ca.crt
require_client_auth: true
# when using 5.4.x we have to specify force_schema_commit_log option
force_schema_commit_log: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to explicitly mention this param?
What about other params that are set by default in the Scylla.yaml?
Are we overriding all of them using this yaml?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this the container fails to start. There is an issue about that when using 5.4.x we have to specify that option: scylladb/scylladb#16427 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sylwiaszunejko , can I ask you to add comment to the file so that we could link this option to the context

Loading