Skip to content
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

Jump to bottom

Add production warning for pre-release builds #20674

Merged
merged 2 commits into from
Sep 28, 2016
Merged

Add production warning for pre-release builds #20674

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02b2170
Add production warning for pre-release builds
jasontedor Sep 27, 2016
0fce68d
Move displayVersion to Node
jasontedor Sep 27, 2016
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
9 changes: 9 additions & 0 deletions core/src/main/java/org/elasticsearch/Version.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ public String toString() {
return sb.toString();
}

public String displayVersion() {
Copy link
Member

Choose a reason for hiding this comment

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

I foresee being confused about whether we should use this or toString. Maybe we should just use toString in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

@nik9000 Version#toString does not display whether or not the build is a snapshot build; I'm happy to add Javadocs?

Copy link
Member

Choose a reason for hiding this comment

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

Ah! I see the static reference now. I think this method is fairly confusing then? Like, it looks like it is just about the version that you are calling it on but it does stuff with the current build.

Copy link
Member Author

@jasontedor jasontedor Sep 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I pushed 0fce68d, I think this will remove the confusion.

return this + (Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "");
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down Expand Up @@ -352,4 +356,9 @@ public boolean isAlpha() {
public boolean isRC() {
return build > 50 && build < 99;
}

public boolean isRelease() {
return build == 99;
}

}
16 changes: 12 additions & 4 deletions core/src/main/java/org/elasticsearch/node/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@
import org.elasticsearch.plugins.DiscoveryPlugin;
import org.elasticsearch.plugins.IngestPlugin;
import org.elasticsearch.plugins.MapperPlugin;
import org.elasticsearch.plugins.MetaDataUpgrader;
import org.elasticsearch.plugins.NetworkPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.MetaDataUpgrader;
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.plugins.RepositoryPlugin;
import org.elasticsearch.plugins.ScriptPlugin;
Expand Down Expand Up @@ -255,11 +255,10 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
NODE_NAME_SETTING.get(tmpSettings), NODE_NAME_SETTING.getKey());
}

final String displayVersion = Version.CURRENT + (Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "");
final JvmInfo jvmInfo = JvmInfo.jvmInfo();
logger.info(
"version[{}], pid[{}], build[{}/{}], OS[{}/{}/{}], JVM[{}/{}/{}/{}]",
displayVersion,
Version.CURRENT.displayVersion(),
jvmInfo.pid(),
Build.CURRENT.shortHash(),
Build.CURRENT.date(),
Expand All @@ -270,7 +269,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
Constants.JVM_NAME,
Constants.JAVA_VERSION,
Constants.JVM_VERSION);

warnIfPreRelease(Version.CURRENT, Build.CURRENT.isSnapshot(), logger);

if (logger.isDebugEnabled()) {
logger.debug("using config [{}], data [{}], logs [{}], plugins [{}]",
Expand Down Expand Up @@ -445,6 +444,15 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
}
}

// visible for testing
static void warnIfPreRelease(final Version version, final boolean isSnapshot, final Logger logger) {
if (!version.isRelease() || isSnapshot) {
logger.warn(
"version [{}] is a pre-release version of Elasticsearch and is not suitable for production",
version.displayVersion());
}
}

protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool,
TransportInterceptor interceptor) {
return new TransportService(settings, transport, threadPool, interceptor);
Expand Down
31 changes: 31 additions & 0 deletions test/framework/src/main/java/org/elasticsearch/node/NodeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.elasticsearch.node;

import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -30,6 +32,10 @@
import java.util.Collections;

import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

public class NodeTests extends ESTestCase {

Expand All @@ -55,4 +61,29 @@ public void testNodeName() throws IOException {
}
}
}

public void testWarnIfPreRelease() {
final Logger logger = mock(Logger.class);

final int id = randomIntBetween(1, 9) * 1000000;
final Version releaseVersion = Version.fromId(id + 99);
final Version preReleaseVersion = Version.fromId(id + randomIntBetween(0, 98));

Node.warnIfPreRelease(releaseVersion, false, logger);
verifyNoMoreInteractions(logger);

reset(logger);
Node.warnIfPreRelease(releaseVersion, true, logger);
verify(logger).warn(
"version [{}] is a pre-release version of Elasticsearch and is not suitable for production", releaseVersion + "-SNAPSHOT");

reset(logger);
final boolean isSnapshot = randomBoolean();
Node.warnIfPreRelease(preReleaseVersion, isSnapshot, logger);
verify(logger).warn(
"version [{}] is a pre-release version of Elasticsearch and is not suitable for production",
preReleaseVersion + (isSnapshot ? "-SNAPSHOT" : ""));

}

}