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

scylla_node: ignore scylla-tools config files if scylla-tools is missing #618

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

tchaikov
Copy link
Contributor

before this change, in ScyllaNode.copy_config_files()we always try to copy the jvm settings packaged by scylla-tools-java package, but we plan to drop this package. but if that package is installed, get_tools_java_dir() would return None. this breaks copy_config_files(), like:

21:44:43  Traceback (most recent call last):
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/bin/ccm", line 74, in <module>
21:44:43      cmd.run()
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/cmds/cluster_cmds.py", line 269, in run
21:44:43      cluster.populate(self.nodes, self.options.debug, use_vnodes=self.options.vnodes, ipprefix=self.options.ipprefix, ipformat=self.options.ipformat)
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/cluster.py", line 327, in populate
21:44:43      self.new_node(i, debug=debug, initial_token=tk, data_center=dc, rack=rack)
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/cluster.py", line 334, in new_node
21:44:43      node = self.create_node(name=f'node{i}',
21:44:43             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_cluster.py", line 93, in create_node
21:44:43      return ScyllaNode(name, self, auto_bootstrap, None,
21:44:43             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_node.py", line 51, in __init__
21:44:43      super().__init__(name, cluster, auto_bootstrap,
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/node.py", line 126, in __init__
21:44:43      self.import_config_files()
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_node.py", line 975, in import_config_files
21:44:43      self.copy_config_files()
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_node.py", line 981, in copy_config_files
21:44:43      conf_pattern = os.path.join(self.get_tools_java_dir(), 'conf', "jvm*.options")
21:44:43                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21:44:43    File "<frozen posixpath>", line 76, in join
21:44:43  TypeError: expected str, bytes or os.PathLike object, not NoneType

in this change, we enumerate the directory only if it exists.

Fixes #617
Signed-off-by: Kefu Chai [email protected]

before this change, in `ScyllaNode.copy_config_files()`we always try to
copy the jvm settings packaged by scylla-tools-java package, but we plan
to drop this package. but if that package is installed,
`get_tools_java_dir()` would return `None`. this breaks
`copy_config_files()`, like:

```
21:44:43  Traceback (most recent call last):
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/bin/ccm", line 74, in <module>
21:44:43      cmd.run()
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/cmds/cluster_cmds.py", line 269, in run
21:44:43      cluster.populate(self.nodes, self.options.debug, use_vnodes=self.options.vnodes, ipprefix=self.options.ipprefix, ipformat=self.options.ipformat)
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/cluster.py", line 327, in populate
21:44:43      self.new_node(i, debug=debug, initial_token=tk, data_center=dc, rack=rack)
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/cluster.py", line 334, in new_node
21:44:43      node = self.create_node(name=f'node{i}',
21:44:43             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_cluster.py", line 93, in create_node
21:44:43      return ScyllaNode(name, self, auto_bootstrap, None,
21:44:43             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_node.py", line 51, in __init__
21:44:43      super().__init__(name, cluster, auto_bootstrap,
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/node.py", line 126, in __init__
21:44:43      self.import_config_files()
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_node.py", line 975, in import_config_files
21:44:43      self.copy_config_files()
21:44:43    File "/jenkins/workspace/scylla-master/gating-dtest-release/scylla/.local/lib/python3.12/site-packages/ccmlib/scylla_node.py", line 981, in copy_config_files
21:44:43      conf_pattern = os.path.join(self.get_tools_java_dir(), 'conf', "jvm*.options")
21:44:43                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21:44:43    File "<frozen posixpath>", line 76, in join
21:44:43  TypeError: expected str, bytes or os.PathLike object, not NoneType
```

in this change, we enumerate the directory only if it exists.

Fixes scylladb#617
Signed-off-by: Kefu Chai <[email protected]>
Copy link
Contributor

@syuu1228 syuu1228 left a comment

Choose a reason for hiding this comment

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

LGTM

@fruch
Copy link
Contributor

fruch commented Sep 26, 2024

Would want to see on gating run with this change (with and without the change of removing java tools)

@yaronkaikov
Copy link
Contributor

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

@fruch
Copy link
Contributor

fruch commented Sep 26, 2024

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

Only after it proven to break current gating.

He would need to wait for it to pass next anyhow.

@yaronkaikov
Copy link
Contributor

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

Only after it proven to break current gating.

He would need to wait for it to pass next anyhow.

Sorry, I don't follow, how do you want to test it?

@fruch
Copy link
Contributor

fruch commented Sep 26, 2024

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

Only after it proven to break current gating.

He would need to wait for it to pass next anyhow.

Sorry, I don't follow, how do you want to test it?

Run a staging/BYO dtest job with gating tests

Next isn't a place to experiment with

@yaronkaikov
Copy link
Contributor

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

Only after it proven to break current gating.
He would need to wait for it to pass next anyhow.

Sorry, I don't follow, how do you want to test it?

Run a staging/BYO dtest job with gating tests

Next isn't a place to experiment with

I am running https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next/1568/ with a custom configuration.

@yaronkaikov
Copy link
Contributor

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

Only after it proven to break current gating.
He would need to wait for it to pass next anyhow.

Sorry, I don't follow, how do you want to test it?

Run a staging/BYO dtest job with gating tests
Next isn't a place to experiment with

I am running https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next/1568/ with a custom configuration.

Or even better https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/gating-dtest-release-with-consistent-topology-changes/144/

@yaronkaikov
Copy link
Contributor

@fruch can you merge this? we need it promoted before @syuu1228 can re-kick the CI

Only after it proven to break current gating.
He would need to wait for it to pass next anyhow.

Sorry, I don't follow, how do you want to test it?

Run a staging/BYO dtest job with gating tests
Next isn't a place to experiment with

I am running https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next/1568/ with a custom configuration.

Or even better https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/gating-dtest-release-with-consistent-topology-changes/144/

And https://jenkins.scylladb.com/job/scylla-master/job/byo/job/byo_build_tests_dtest/2501/ (which is @syuu1228 branch with @tchaikov branch for ccm)

@tchaikov
Copy link
Contributor Author

@yaronkaikov thank you for verifying the change.

@fruch
Copy link
Contributor

fruch commented Sep 26, 2024

@yaronkaikov
Copy link
Contributor

@tchaikov
Copy link
Contributor Author

@fruch ping ?

@fruch fruch merged commit 3a88640 into scylladb:next Sep 26, 2024
4 checks passed
@tchaikov tchaikov deleted the copy-if-exists branch September 26, 2024 14:14
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.

ccmlib should work when scylla-tools not installed
4 participants