Skip to content

CASSSIDECAR-400- Fixing sidecar’s ProcessLifecycleProviderIntegrationTest() failure in root mode#313

Merged
frankgh merged 2 commits intoapache:trunkfrom
shruti-p-s:CASSSIDECAR-400
Feb 4, 2026
Merged

CASSSIDECAR-400- Fixing sidecar’s ProcessLifecycleProviderIntegrationTest() failure in root mode#313
frankgh merged 2 commits intoapache:trunkfrom
shruti-p-s:CASSSIDECAR-400

Conversation

@shruti-p-s
Copy link
Contributor

Patch by Shruti Sekaran; Reviewed by TBD for CASSSIDECAR-400

Copy link
Contributor

@frankgh frankgh left a comment

Choose a reason for hiding this comment

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

I have some concerns about adding the flag by default.

startCassandraCmd.add(cassandraBin().toString());
startCassandraCmd.add("-p");
startCassandraCmd.add(pidFileLocation);
startCassandraCmd.add("-R"); // Allow running as root (required for Cassandra 5.0+ when running as root)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding this flag to production code is dangerous. Looking at CASSANDRA-8142 the idea is to prevent this behavior by default for good reasons. I would propose that we alternatively look into having the ability to provide optional extra args that we can configure for testing purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I've updated the argument as a test override

@shruti-p-s shruti-p-s requested a review from frankgh February 4, 2026 18:01
Copy link
Contributor

@sarankk sarankk left a comment

Choose a reason for hiding this comment

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

+1 Thanks Shruti, Lgtm

Copy link
Contributor

@frankgh frankgh left a comment

Choose a reason for hiding this comment

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

+1 thanks for the patch

@frankgh frankgh merged commit a201f3a into apache:trunk Feb 4, 2026
75 of 84 checks passed
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.

4 participants