[CI] TrialLicenseVersionTests testNewTrialAllowed failing

[kkrik-es]

Build scan:
https://gradle-enterprise.elastic.co/s/chgmd2eenbvwc/tests/:x-pack:plugin:core:test/org.elasticsearch.license.internal.TrialLicenseVersionTests/testNewTrialAllowed
Reproduction line:

./gradlew ':x-pack:plugin:core:test' --tests "org.elasticsearch.license.internal.TrialLicenseVersionTests.testNewTrialAllowed" -Dtests.seed=BAFF06A03CD00ED4 -Dtests.locale=fi-FI -Dtests.timezone=America/Miquelon -Druntime.java=21

Applicable branches:
main

Reproduces locally?:
Yes

Failure history:
https://gradle-enterprise.elastic.co/scans/tests?tests.container=org.elasticsearch.license.internal.TrialLicenseVersionTests&tests.test=testNewTrialAllowed

Failure excerpt:

java.lang.AssertionError: (No message provided)

  at __randomizedtesting.SeedInfo.seed([BAFF06A03CD00ED4:AECF3107111E0BDC]:0)
  at org.junit.Assert.fail(Assert.java:86)
  at org.junit.Assert.assertTrue(Assert.java:41)
  at org.junit.Assert.assertFalse(Assert.java:64)
  at org.junit.Assert.assertFalse(Assert.java:74)
  at org.elasticsearch.license.internal.TrialLicenseVersionTests.testNewTrialAllowed(TrialLicenseVersionTests.java:40)
  at jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
  at java.lang.reflect.Method.invoke(Method.java:580)
  at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1758)
  at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:946)
  at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:982)
  at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:996)
  at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
  at org.apache.lucene.tests.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:48)
  at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
  at org.apache.lucene.tests.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:45)
  at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
  at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
  at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
  at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:390)
  at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:843)
  at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:490)
  at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:955)
  at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:840)
  at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:891)
  at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:902)
  at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
  at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
  at org.apache.lucene.tests.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:38)
  at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
  at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
  at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
  at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
  at org.apache.lucene.tests.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
  at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
  at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
  at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
  at org.apache.lucene.tests.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:47)
  at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
  at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:390)
  at com.carrotsearch.randomizedtesting.ThreadLeakControl.lambda$forkTimeoutingTask$0(ThreadLeakControl.java:850)
  at java.lang.Thread.run(Thread.java:1583)

[elasticsearchmachine]

Pinging @elastic/es-security (Team:Security)

[n1v0lg]

Can't tell yet if this is an actual bug, or a test conditions issue:

This fails when we pick a version that comes before TRIAL_VERSION_CUTOVER due to the special handling in ableToStartNewTrialSince. For example, new Version(7_000_000).ableToStartNewTrialSince(new Version(7_000_000)) evaluates to true which feels bogus. I'll dig into the PR that introduced the relevant changes.

Either way, muting this since this will reliably continue failing.

[n1v0lg]

The problem is in handling two versions that both fall before TRIAL_VERSION_CUTOVER, e.g., new TrialLicenseVersion(7_00_00_00).ableToStartNewTrialSince(new TrialLicenseVersion(7_00_00_01)) will incorrectly evaluate to true.

This can't happen in practice, since the only place we call this in production code is via trialLicenseVersion.ableToStartNewTrialSince(TrialLicenseVersion.CURRENT) and TrialLicenseVersion.CURRENT is guaranteed to be equal to the cutover.

I'll put up a PR to fix this since Athena is on PTO for another week.

I'm currently leaning towards simplifying this by using s.th. like:

public boolean ableToStartNewTrial() {
    return CURRENT.asInt() < trialVersion;
}

Instead of the current method that accepts an explicit version as argument. I don't see a good reason for having the more general and complex method in the first place, though I gotta stare at the code a bit more to confirm.