Skip to content

Comments

Updated input parameters and added jmxPasswordFilePath variable to in…#183

Closed
Scharlotten wants to merge 1 commit intodatastax:masterfrom
Scharlotten:jmxfilepathfix
Closed

Updated input parameters and added jmxPasswordFilePath variable to in…#183
Scharlotten wants to merge 1 commit intodatastax:masterfrom
Scharlotten:jmxfilepathfix

Conversation

@Scharlotten
Copy link
Contributor

@Scharlotten Scharlotten commented Jun 16, 2025

…put files and updated nodetoolCredentials logic in the ds-collector bash script added the option to use the file

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

…put files and updated nodetoolCredentials logic in the ds-collector bash script added the option to use the file
export jmxPort
export jmxUsername
export jmxPassword
export jmxPasswordFilePath
Copy link
Member

Choose a reason for hiding this comment

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

this isn't needed, as that variable is not used inside collect-info.rs

if [[ -n ${jmxUsername} ]] && [[ -n ${jmxPassword} ]] ; then
nodetoolCredentials="-u $jmxUsername -pw $jmxPassword"
elif [[ -n ${jmxUsername} ]] && [[ -n ${jmxPasswordFilePath} ]] ; then
nodetoolCredentials="-u $jmxUsername -pwf $jmxPasswordFilePath"
Copy link
Member

Choose a reason for hiding this comment

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

this works for nodetool executions, but there are other places jmxPassword is used that will now not work (both in this script and in the rust file ds-collector/rust-commands/collect-info.rs).

so i think we still have to read and set jmxPassword
e.g.

Suggested change
nodetoolCredentials="-u $jmxUsername -pwf $jmxPasswordFilePath"
while IFS= read -r line; do
# Skip empty lines
[[ -z "$line" ]] && continue
# Read first two fields from the line
read -r _role _password <<< "$line"
if [[ "${_role}" == "${jmxUsername}" ]]; then
jmxPassword="${_password}"
break
fi
done < "${jmxPasswordFilePath}"
nodetoolCredentials="-u $jmxUsername -pwf $jmxPasswordFilePath"

@michaelsembwever
Copy link
Member

i couldn't push to your fork @Scharlotten , so i've updated this PR here: #190

@michaelsembwever
Copy link
Member

merged in 87b1072

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