Introduce explicit API for configure test cluster feature flags #83876
Conversation
mark-vieira
added
>non-issue
:Delivery/Build
|
Pinging @elastic/es-delivery (Team:Delivery) |
|
While testing this I'm now discovering that our implementation of feature flags is woefully inconsistent. Feature flags that are automatically enabled in snapshot builds and fail if you attempt to set them for a snapshot build. Feature flags that are automatically enabled in snapshot builds but do not fail if you attempt to set them for a snapshot build. Feature flags that are never automatically enabled and must be explicitly set for all build types. Pinging @elastic/es-security and @elastic/es-analytics-geo for guidance on the last two here. Should we modify these to conform with the convention of the first set? We're trying to implement a consistent way to decoratively configure these things during testing and that becomes impossible when they don't follow a convention. |
| testClustersPlugin.setIsReleasedVersion( | ||
| version -> (version.equals(VersionProperties.getElasticsearchVersion()) && BuildParams.isSnapshotBuild() == false) | ||
| || BuildParams.getBwcVersions().unreleasedInfo(version) == null | ||
| ); |
There was a problem hiding this comment.
"either it is the current version and we're running a release build OR it's not unreleased". That's a lot of double negatives, but I get it.
| systemProperty 'es.random_sampler_feature_flag_registered', 'true' | ||
| systemProperty 'es.user_profile_feature_flag_enabled', 'true' | ||
| } | ||
| requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") |
There was a problem hiding this comment.
Should this be 8.1.0 as well, like the ones below?
There was a problem hiding this comment.
This flag was introduced in 8.0.0.
@mark-vieira For |
|
The conversation on what our convention should be stalled last year, and is waiting for a new owner. In the absence of a conclusion about that convention, my view was that the simplest option with Specifically, the primary reason for needing this PR is because the model used in the other feature flags means that the build needs specific logic to know whether it is OK to set the flag. It's simpler to allow the setting all the time. I don't mind changing |
I wanted to but the flags don't use a consistent convention. Some use the |
|
Ok, for now I've simply made |
|
Agreed, I'm confident that feature flags are going to become way more important in the future, so we want to get ahead of that in terms of support in the build and ES itself. |
|
@elasticmachine run elasticsearch-ci/full-bwc |
…tic#83876) (cherry picked from commit 64929dc)
…tic#83876) (cherry picked from commit 64929dc)
Now that elastic#83876 is in we can have our tests back. Closes elastic#83926
Introduce improved handling for enabling feature flags on external test clusters. We are increasingly developing feature behind feature flags. For testing purposes, that means we need to set these flags when testing non-snapshot builds. This is the case both when a) doing release builds and b) when backward compatibility testing against released versions which require the feature flag for certain tests. Until now this was done via convoluted logic in the build scripts themselves. This pull request moves this logic into the test cluster orchestration itself. This means in our build scripts we can simply do this:
This simply indicates that if we are attempting to start a non-snapshot release node with a version of 8.0.0 or later, we must pass the appropriate flag.
Closes #83822