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

Patch fix 3.3.3 #4033

Merged
merged 5 commits into from
Sep 12, 2024
Merged

Patch fix 3.3.3 #4033

merged 5 commits into from
Sep 12, 2024

Conversation

karol-kokoszka
Copy link
Collaborator

@karol-kokoszka karol-kokoszka commented Sep 11, 2024

Fixes regression https://github.com/scylladb/scylla-enterprise/issues/4692
#4029


Please make sure that:

  • Code is split to commits that address a single change
  • Commit messages are informative
  • Commit titles have module prefix
  • Commit titles have issue nr. suffix

Commit 5bf6b35 introduced a bug that when adding cluster to SM finished with error,
cluster was still saved in SM DB, but only with the ID and known hosts fields.
That's because method validateHostsConnectivity used discoverAndSetClusterHosts
instead of discoverClusterHosts, which happened before inserting cluster to SM DB.

Fixes #4028
E.g. clusters created before SM 3.2.6 can have empty --host SM DB column.
In case client was mistakenly initialized with "" host, getting host from the pool would panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x11da5ac]
goroutine 253 [running]:
github.com/scylladb/scylla-manager/v3/pkg/scyllaclient.NewClient.hostPool.func3(0xc0004345a0)
github.com/scylladb/scylla-manager/v3/pkg/scyllaclient/hostpool.go:32 +0xac

This should be replaced with a simple error.
This commit adds additional validation to client config,
which prohibits specifying empty host there.
It also makes host pool return an error instead of panic
in case of invalid host pool response.

Fixes scylladb/scylla-enterprise#4692
As it is possible that the --host field in SM DB is empty,
we should test that this scenario does not break anything in SM.

Tests scylladb/scylla-enterprise#4692
@karol-kokoszka
Copy link
Collaborator Author

@Michal-Leszczynski can you check operator using this branch as well ?

@Michal-Leszczynski
Copy link
Collaborator

Operator tests started here: scylladb/scylla-operator#2105 (comment).

@karol-kokoszka karol-kokoszka merged commit 5445449 into branch-3.3 Sep 12, 2024
45 of 51 checks passed
@karol-kokoszka karol-kokoszka deleted the patch_fix_3.3.3 branch September 12, 2024 07:43
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.

2 participants