Skip to content
This repository has been archived by the owner on Sep 29, 2021. It is now read-only.

Commit

Permalink
Support all host selector operators in host cmd
Browse files Browse the repository at this point in the history 
We previously only supported the "=" host selector operator in
`helios hosts --labels`. This change supports using "!=", "in",
and "notin" operators.
  • Loading branch information
@davidxia
davidxia committed Jul 24, 2016
1 parent 66c345c commit f74e84f
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 95 deletions.
149 changes: 94 additions & 55 deletions helios-system-tests/src/main/java/com/spotify/helios/system/LabelTest.java
View file
Expand Up @@ -24,83 +24,122 @@
import com.spotify.helios.common.Json;
import com.spotify.helios.common.descriptors.HostStatus;

import com.google.common.collect.ImmutableSet;

import org.hamcrest.CustomTypeSafeMatcher;
import org.junit.Test;

import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;

import static com.spotify.helios.common.descriptors.HostStatus.Status.UP;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;

public class LabelTest extends SystemTestBase {

/**
* @param hosts Hostnames for which to get labels.
* @return a map whose keys are hostnames and values are maps of label key-vals.
*/
private Callable<Map<String, Map<String, String>>> hostLabels(final Set<String> hosts) {
return () -> {
final ImmutableMap.Builder<String, Map<String, String>> hostLabels = ImmutableMap.builder();

for (final String host : hosts) {
final Map<String, HostStatus> status = Json.read(
cli("hosts", host, "--json"),
new TypeReference<Map<String, HostStatus>>() {
});

final HostStatus hostStatus = status.get(host);
if (hostStatus == null) {
return null;
}

final Map<String, String> labels = hostStatus.getLabels();
if (labels != null && !labels.isEmpty()) {
hostLabels.put(host, labels);
} else {
return null;
}

}
return hostLabels.build();
};
}

@Test
public void testHostStatus() throws Exception {
public void testHostLabels() throws Exception {
startDefaultMaster();
startDefaultAgent(testHost(), "--labels", "role=foo", "xyz=123");
awaitHostStatus(testHost(), UP, LONG_WAIT_SECONDS, SECONDS);

// Wait for the agent to report labels
final Map<String, String> labels = Polling.await(LONG_WAIT_SECONDS, SECONDS,
new Callable<Map<String, String>>() {
@Override
public Map<String, String> call()
throws Exception {
final Map<String, HostStatus> status = Json.read(
cli("hosts", testHost(), "--json"),
new TypeReference<Map<String, HostStatus>>() {
});
final Map<String, String>
labels =
status.get(testHost()).getLabels();
if (labels != null && !labels.isEmpty()) {
return labels;
} else {
return null;
}
}
});

assertEquals(ImmutableMap.of("role", "foo", "xyz", "123"), labels);
final Map<String, Map<String, String>> labels = Polling.await(
LONG_WAIT_SECONDS, SECONDS, hostLabels(ImmutableSet.of(testHost())));

final ImmutableMap<String, ImmutableMap<String, String>> expectedLabels =
ImmutableMap.of(testHost(), ImmutableMap.of("role", "foo", "xyz", "123"));
assertThat(labels, equalTo(expectedLabels));
}

@Test
public void testCliHosts() throws Exception {
final String testHost1 = testHost() + "1";
final String testHost2 = testHost() + "2";

startDefaultMaster();
startDefaultAgent(testHost(), "--labels", "role=foo", "xyz=123");
awaitHostStatus(testHost(), UP, LONG_WAIT_SECONDS, SECONDS);
startDefaultAgent(testHost1, "--labels", "role=foo", "xyz=123");
startDefaultAgent(testHost2, "--labels", "role=bar", "xyz=123");

// Wait for the agent to report labels
Polling.await(LONG_WAIT_SECONDS, SECONDS,
new Callable<Map<String, String>>() {
@Override
public Map<String, String> call()
throws Exception {
final Map<String, HostStatus> status = Json.read(
cli("hosts", testHost(), "--json"),
new TypeReference<Map<String, HostStatus>>() {
});
final Map<String, String>
labels =
status.get(testHost()).getLabels();
if (labels != null && !labels.isEmpty()) {
return labels;
} else {
return null;
}
}
});

assertThat(cli("hosts", "--labels", "role=foo"), containsString(testHost()));
assertThat(cli("hosts", "--labels", "xyz=123"), containsString(testHost()));
assertThat(cli("hosts", "--labels", "role=foo", "xyz=123"), containsString(testHost()));

assertThat(cli("hosts", "--labels", "role=doesnt_exist"), not(containsString(testHost())));
assertThat(cli("hosts", "--labels", "role=doesnt_exist", "xyz=123"),
not(containsString(testHost())));
// Wait for the agents to report labels
Polling.await(LONG_WAIT_SECONDS, SECONDS, hostLabels(ImmutableSet.of(testHost1, testHost2)));

// Test all these host selectors in one test method to reduce testing time.
// We can't start masters and agents in @BeforeClass because startDefaultMaster() isn't static
final String fooHosts = cli("hosts", "--labels", "role=foo");
assertThat(fooHosts, containsString(testHost1));
assertThat(fooHosts, not(containsString(testHost2)));

final String xyzHosts = cli("hosts", "--labels", "xyz=123");
assertThat(xyzHosts, containsStrings(ImmutableSet.of(testHost1, testHost2)));

final String fooAndXyzHosts = cli("hosts", "--labels", "role=foo", "xyz=123");
assertThat(fooAndXyzHosts, containsString(testHost1));
assertThat(fooAndXyzHosts, not(containsString(testHost2)));

final String notFooHosts = cli("hosts", "--labels", "role!=foo");
assertThat(notFooHosts, not(containsString(testHost1)));
assertThat(notFooHosts, containsString(testHost2));

final String inFooBarHosts = cli("hosts", "--labels", "role in (foo, bar)");
assertThat(inFooBarHosts, containsStrings(ImmutableSet.of(testHost1, testHost2)));

final String notInFooBarHosts = cli("hosts", "--labels", "role notin (foo, bar)");
assertThat(notInFooBarHosts, not(containsStrings(ImmutableSet.of(testHost1, testHost2))));

final String nonHosts = cli("hosts", "--labels", "role=doesnt_exist");
assertThat(nonHosts, not(containsStrings(ImmutableSet.of(testHost1, testHost2))));

final String nonHosts2 = cli("hosts", "--labels", "role=doesnt_exist", "xyz=123");
assertThat(nonHosts2, not(containsStrings(ImmutableSet.of(testHost1, testHost2))));
}

private CustomTypeSafeMatcher<String> containsStrings(final Set<String> substrings) {
return new CustomTypeSafeMatcher<String>(
"A string containing in any order all the substrings " + substrings) {
@Override
protected boolean matchesSafely(final String string) {
for (final String s : substrings) {
if (!string.contains(s)) {
return false;
}
}
return true;
}
};
}
}
2 changes: 1 addition & 1 deletion helios-tools/src/main/java/com/spotify/helios/cli/command/DeploymentGroupCreateCommand.java
View file
Expand Up @@ -60,7 +60,7 @@ public DeploymentGroupCreateCommand(final Subparser parser) {
.help("Host selector expression. Hosts matching this expression will be part of the " +
"deployment-group. Multiple conditions can be specified, separated by spaces (as " +
"separate arguments). If multiple conditions are given, all must be fulfilled. " +
"Operators supported are '=' and '!='. Example: foo=bar baz!=qux");
"Operators supported are =, !=, in and notin. Example: foo=bar baz!=qux");

quietArg = parser.addArgument("-q")
.action(storeTrue())
Expand Down
44 changes: 16 additions & 28 deletions helios-tools/src/main/java/com/spotify/helios/cli/command/HostListCommand.java
View file
Expand Up @@ -28,10 +28,12 @@
import com.google.common.util.concurrent.ListenableFuture;

import com.spotify.helios.cli.Table;
import com.spotify.helios.cli.Utils;
import com.spotify.helios.client.HeliosClient;
import com.spotify.helios.common.Json;
import com.spotify.helios.common.descriptors.DockerVersion;
import com.spotify.helios.common.descriptors.HostInfo;
import com.spotify.helios.common.descriptors.HostSelector;
import com.spotify.helios.common.descriptors.HostStatus;
import com.spotify.helios.common.descriptors.JobId;
import com.spotify.helios.common.descriptors.TaskStatus;
Expand All @@ -58,7 +60,6 @@
import static com.spotify.helios.cli.Output.humanDuration;
import static com.spotify.helios.cli.Output.table;
import static com.spotify.helios.cli.Utils.allAsMap;
import static com.spotify.helios.cli.Utils.argToStringMap;
import static com.spotify.helios.common.descriptors.HostStatus.Status.UP;
import static java.lang.String.format;
import static java.lang.System.currentTimeMillis;
Expand All @@ -71,7 +72,7 @@ public class HostListCommand extends ControlCommand {
private final Argument patternArg;
private final Argument fullArg;
private final Argument statusArg;
private final Argument labelsArg;
private final Argument hostSelectorsArg;

private final String statusChoicesString;

Expand Down Expand Up @@ -108,12 +109,14 @@ public String apply(final HostStatus.Status input) {
.choices(statusChoices.toArray(new String[statusChoices.size()]))
.help("Filter hosts by its status. Valid statuses are: " + statusChoicesString);

labelsArg = parser.addArgument("-l", "--labels")
hostSelectorsArg = parser.addArgument("-l", "--labels")
.action(append())
.setDefault(new ArrayList<String>())
.nargs("+")
.help("Only include hosts that match all of these labels. Labels need to be in the format "
+ "key=value.");
.help("Host selector expression. Hosts matching this expression will be part of the " +
"deployment-group. Multiple conditions can be specified, separated by spaces (as " +
"separate arguments). If multiple conditions are given, all must be fulfilled. " +
"Operators supported are =, !=, in and notin. Example: foo=bar baz!=qux");
}

@Override
Expand Down Expand Up @@ -152,17 +155,11 @@ int run(final Namespace options, final HeliosClient client, final PrintStream ou

final List<String> sortedHosts = natural().sortedCopy(hosts);

final Map<String, String> selectedLabels;
try {
selectedLabels = argToStringMap(options, labelsArg);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(e.getMessage() +
"\nLabels need to be in the format key=value.");
}
final List<HostSelector> hostSelectors = Utils.parseHostSelectors(options, hostSelectorsArg);

if (selectedLabels != null && !selectedLabels.isEmpty() && json) {
System.err.println("Warning: filtering by label is not supported for JSON output. Not doing"
+ " any filtering by label.");
if (hostSelectors != null && !hostSelectors.isEmpty() && json) {
System.err.println("Warning: host selectors are not supported for JSON output. Not applying"
+ " any host selectors.");
}

if (quiet) {
Expand Down Expand Up @@ -206,20 +203,11 @@ int run(final Namespace options, final HeliosClient client, final PrintStream ou
}

boolean skipHost = false;
if (selectedLabels != null && !selectedLabels.isEmpty()) {
if (hostSelectors != null && !hostSelectors.isEmpty()) {
final Map<String, String> hostLabels = s.getLabels();
for (final Entry<String, String> label : selectedLabels.entrySet()) {
final String key = label.getKey();
final String value = label.getValue();

if (!hostLabels.containsKey(key)) {
skipHost = true;
break;
}

final String hostValue = hostLabels.get(key);

if (isNullOrEmpty(value) ? !isNullOrEmpty(hostValue) : !value.equals(hostValue)) {
for (final HostSelector selector : hostSelectors) {
if (!hostLabels.containsKey(selector.getLabel()) ||
!selector.matches(hostLabels.get(selector.getLabel()))) {
skipHost = true;
break;
}
Expand Down

0 comments on commit f74e84f

Please sign in to comment.