Skip to content

JAVA-2622: Add Cassandra 4.0 to build matrix for ITs #1390

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.

Sign up for GitHub

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

Merged
emerkle826 merged 3 commits into from
Jan 24, 2020
Merged

JAVA-2622: Add Cassandra 4.0 to build matrix for ITs #1390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d5729d
JAVA-2622: Add Cassandra 4.0 to build matrix for ITs
emerkle826 Jan 22, 2020
bac8050
[fixup] fix build.yaml
emerkle826 Jan 22, 2020
f2b3c9e
[fixup] Fix Version.toString() for C* 4.0 pre-releases
emerkle826 Jan 23, 2020
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
34 changes: 18 additions & 16 deletions build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ cassandra:
- '2.1'
- '3.0'
- '3.11'
- '4.0'
- 'dse-4.8'
- 'dse-5.0'
- 'dse-5.1'
Expand All @@ -27,41 +28,45 @@ schedules:
- jabba: openjdk_jdk11
- jabba: openjdk_jdk12
- jabba: openjdk_jdk13
disable_commit_status: true
notify:
slack: java-driver-dev-bots
nightly:
schedule: nightly
matrix:
exclude:
# No excludes for JDK8
# Exclude JDK11 for all but the latest Cassandra and DSE 6.7+
- jabba: openjdk_jdk11
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', dse-6.8']
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', 'dse-6.8']
# Exclude JDK12 for all but the latest Cassandra and DSE 6.7+
- jabba: openjdk_jdk12
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', dse-6.8']
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', 'dse-6.8']
# Exclude JDK13 for all but the latest Cassandra and DSE 6.7+
- jabba: openjdk_jdk13
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', dse-6.8']
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', 'dse-6.8']
disable_commit_status: true
notify:
slack: java-driver-dev-bots

adhoc:
schedule: adhoc
matrix:
exclude:
# No excludes for JDK8
# Exclude JDK11 for all but the latest Cassandra and DSE 6.7+
- jabba: openjdk_jdk11
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', dse-6.8']
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', 'dse-6.8']
# Exclude JDK12 for all but the latest Cassandra and DSE 6.7+
- jabba: openjdk_jdk12
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', dse-6.8']
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', 'dse-6.8']
# Exclude JDK13 for all but the latest Cassandra and DSE 6.7+
- jabba: openjdk_jdk13
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', dse-6.8']
cassandra: ['2.1', '3.0', 'dse-4.8', 'dse-5.0', 'dse-5.1', 'dse-6.0', 'dse-6.8']
disable_commit_status: true
notify:
slack: java-driver-dev-bots
build:
- type: maven
- properties: |
ccm.version=$CCM_CASSANDRA_VERSION
ccm.dse=$CCM_IS_DSE
proxy.path=$HOME/proxy
maven.javadoc.skip=true
- script: |
# Jabba default should be a JDK8 for now
jabba use default
Expand All @@ -72,11 +77,8 @@ build:
# Use the matrix JDK for testing
jabba use $JABBA_JDK_NAME
# Run tests against matrix JDK
mvn -B -V verify --batch-mode --show-version
mvn -B -V verify --batch-mode --show-version -Dccm.version=$CCM_CASSANDRA_VERSION -Dccm.dse=$CCM_IS_DSE -Dproxy.path=$HOME/proxy -Dmaven.javadoc.skip=true
- xunit:
- "**/target/surefire-reports/TEST-*.xml"
- "**/target/failsafe-reports/TEST-*.xml"
- jacoco: true
disable_commit_status: true
notify:
slack: java-driver-dev-bots
20 changes: 19 additions & 1 deletion test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/CcmBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,24 @@ public Version getCassandraVersion() {
}
}

private String getCcmVersionString(Version version) {
// for 4.0 pre-releases, the CCM version string needs to be "4.0-alpha1" or "4.0-alpha2"
// Version.toString() always adds a patch value, even if it's not specified when parsing.
if (version.getMajor() == 4
&& version.getMinor() == 0
&& version.getPatch() == 0
&& version.getPreReleaseLabels() != null) {
Comment on lines +192 to +195

@emerkle826 emerkle826 Jan 23, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This check is specifically to catch a Version object for something like 4.0-alpha2. When you use Version.parse(String), it sets the patch value to 0 if not specified.

Passing a string of "4.0-alpha2" yields a major of 4, minor of 0, patch of 0 and preRelease of "alpha2". Version.toString() always appends the patch version to the String. So if you parse "4.0-alpha2", the toString() method returns "4.0.0-alpha2". The way CCM is installed on the Jenkins images, it looks for a released version of CCM, or a tagged pre-release version. So the ccm create command for C* 4.0 pre-release builds needs to specify -v 4.0-alpha2, not -v 4.0.0-alpha2.

This seemed to be the easiest way to accommodate this quirk as once C* 4.0 is released, it should be available as 4.0.0 and Version.parse(String) and Version.toString() on that will work like it has been. I didn't want to mess with the Version class as making the patch version optional would cause all kinds of parsing headaches considering we already have a dsePatch version that is optional (i.e. 2.0.1.1 vs 2.0.1).

TL;DR; If the Version object is 4.0.0 and has at least 1 preRelease tag, CcmBridge will cut out the patch version for the String it uses to build the CCM command.

@adutra adutra Jan 23, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why didn't we have the problem so far with versions such as 3.0, 3.11, etc? Maybe the Jenkins images were called 3.0.0 and 3.11.0? I don't mind special-casing 4.0 but I would like to understand why we need to do this now.

@emerkle826 emerkle826 Jan 23, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is correct. For Cassandra versions, Jenkins images have access to the ones here: http://repomirror.datastax.lan/apache/cassandra/. If the version string doesn't match one of those, CCM will try to clone down a branch or tag that matches the version string. For whatever reason, Cassandra tags the 4.0 pre-releases without a patch version.

I suppose it is possible to store off 4.0-alpha2 as 4.0.0-alpha2 in that repository above, but I think the other drivers are doing something similar to this approach currently.

Going forward, I expect Cassandra 4.0 to resolve to 4.0.0, or 4.0.x where x is the latest available patch release, which is exactly what happens with our builds when the matrix specifies 3.0 or 3.11. And once Cassandra 4.0 releases have a patch version in the number scheme, this special casing won't be hit.

// truncate the patch version from the Version string
StringBuilder sb = new StringBuilder();
sb.append(version.getMajor()).append('.').append(version.getMinor());
for (String preReleaseString : version.getPreReleaseLabels()) {
sb.append('-').append(preReleaseString);
}
return sb.toString();
}
return version.toString();
}

public void create() {
if (created.compareAndSet(false, true)) {
if (INSTALL_DIRECTORY != null) {
Expand All @@ -194,7 +212,7 @@ public void create() {
createOptions.add("-v git:" + BRANCH.trim().replaceAll("\"", ""));

} else {
createOptions.add("-v " + VERSION.toString());
createOptions.add("-v " + getCcmVersionString(VERSION));
}
if (DSE_ENABLEMENT) {
createOptions.add("--dse");
Expand Down