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

CODENVY-413: Make it possible to mark MachineNode as 'sheduled for maintenace' #1705

Merged
merged 1 commit into from
Jul 14, 2016
Merged

CODENVY-413: Make it possible to mark MachineNode as 'sheduled for maintenace' #1705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 13 additions & 11 deletions ...gin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.eclipse.che.plugin.docker.client.connection.DockerConnection;
import org.eclipse.che.plugin.docker.client.connection.DockerConnectionFactory;
import org.eclipse.che.plugin.docker.client.connection.DockerResponse;
import org.eclipse.che.plugin.docker.client.dto.AuthConfigs;
import org.eclipse.che.plugin.docker.client.exception.ContainerNotFoundException;
import org.eclipse.che.plugin.docker.client.exception.DockerException;
import org.eclipse.che.plugin.docker.client.exception.ImageNotFoundException;
Expand Down Expand Up @@ -758,25 +757,28 @@ public String buildImage(final BuildImageParams params,
private String buildImage(final DockerConnection dockerConnection,
final BuildImageParams params,
final ProgressMonitor progressMonitor) throws IOException {
final AuthConfigs authConfigs = params.getAuthConfigs();
final String repository = params.getRepository();
final String tag = params.getTag();

try (DockerConnection connection = dockerConnection.method("POST")
.path(apiVersionPathPrefix + "/build")
.query("rm", 1)
.query("forcerm", 1)
.header("X-Registry-Config",
authResolver.getXRegistryConfigHeaderValue(authConfigs))) {
if (tag == null) {
addQueryParamIfNotNull(connection, "t", repository);
} else {
addQueryParamIfNotNull(connection, "t", repository == null ? null : repository + ':' + tag);
}
authResolver.getXRegistryConfigHeaderValue(params.getAuthConfigs()))) {
addQueryParamIfNotNull(connection, "rm", params.isRemoveIntermediateContainer());
addQueryParamIfNotNull(connection, "forcerm", params.isForceRemoveIntermediateContainers());
addQueryParamIfNotNull(connection, "memory", params.getMemoryLimit());
addQueryParamIfNotNull(connection, "memswap", params.getMemorySwapLimit());
addQueryParamIfNotNull(connection, "pull", params.isDoForcePull());
addQueryParamIfNotNull(connection, "dockerfile", params.getDockerfile());
addQueryParamIfNotNull(connection, "nocache", params.isNoCache());
addQueryParamIfNotNull(connection, "q", params.isQuiet());
if (params.getTag() == null) {
addQueryParamIfNotNull(connection, "t", repository);
} else {
addQueryParamIfNotNull(connection, "t", repository == null ? null : repository + ':' + params.getTag());
}
if (params.getBuildArgs() != null) {
addQueryParamIfNotNull(connection, "buildargs", URLEncoder.encode(GSON.toJson(params.getBuildArgs()), "UTF-8"));
}

final DockerResponse response = connection.request();
if (OK.getStatusCode() != response.getStatus()) {
Expand Down
196 changes: 158 additions & 38 deletions ...er-client/src/main/java/org/eclipse/che/plugin/docker/client/params/BuildImageParams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
import javax.validation.constraints.NotNull;
import java.io.File;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import static java.util.Objects.requireNonNull;
Expand All @@ -30,16 +32,20 @@
* @author Alexander Garagatyi
*/
public class BuildImageParams {
// todo add next parameters q, nocache, rm, forcerm
private String repository;
private String tag;
private AuthConfigs authConfigs;
private Boolean doForcePull;
private Long memoryLimit;
private Long memorySwapLimit;
private List<File> files;
private String dockerfile;
private String remote;
private String repository;
private String tag;
private AuthConfigs authConfigs;
private Boolean doForcePull;
private Long memoryLimit;
private Long memorySwapLimit;
private List<File> files;
private String dockerfile;
private String remote;
private Boolean quiet;
private Boolean noCache;
private Boolean removeIntermediateContainer;
private Boolean forceRemoveIntermediateContainers;
private Map<String, String> buildArgs;

/**
* Creates arguments holder with required parameters.
Expand Down Expand Up @@ -182,6 +188,9 @@ public BuildImageParams withFiles(@NotNull File... files) {
* if {@code files} is null
*/
public BuildImageParams addFiles(@NotNull File... files) {
if (remote != null) {
Copy link

Choose a reason for hiding this comment

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

Useless since it is already checked in withFiles

Copy link
Contributor Author

@mmorhun mmorhun Jul 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

No, addFiles is public and can be used directly. If we created params from remote and try to add a file we get NPE. It is not clear as for me.

Copy link

Choose a reason for hiding this comment

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

withFiles will be called before becase it is used in create method. If create with remote is called then https://github.com/eclipse/che/pull/1705/files/46da34dd88373c9b7292e0d0d1aaecdf20fe4ace#diff-3b42f0904980953ecdfb9059553a7736R194 will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessarily. It is still possible to do this: BuildImageParams.create(<remote>).addFiles(<a file>). And NPE says nothing about root of error.

throw new IllegalStateException("Remote parameter is already set. Remote and files parameters are mutually exclusive.");
}
requireNonNull(files);
for (File file : files) {
requireNonNull(file);
Expand Down Expand Up @@ -213,6 +222,87 @@ public BuildImageParams withRemote(@NotNull String remote) {
return this;
}

/**
* Suppress verbose build output.
*
* @param quiet
* quiet flag
* @return this params instance
*/
public BuildImageParams withQuiet(boolean quiet) {
this.quiet = quiet;
return this;
}

/**
* Do not use the cache when building the image.
*
* @param noCache
* no cache flag
* @return this params instance
*/
public BuildImageParams withNoCache(boolean noCache) {
this.noCache = noCache;
return this;
}

/**
* Remove intermediate containers after a successful build.
*
* @param removeIntermediateContainer
* remove intermediate container flag
* @return this params instance
*/
public BuildImageParams withRemoveIntermediateContainers(boolean removeIntermediateContainer) {
this.removeIntermediateContainer = removeIntermediateContainer;
return this;
}

/**
* Always remove intermediate containers (includes removeIntermediateContainer).
*
* @param forceRemoveIntermediateContainers
* remove intermediate containers with force flag
* @return this params instance
*/
public BuildImageParams withForceRemoveIntermediateContainers(boolean forceRemoveIntermediateContainers) {
this.forceRemoveIntermediateContainers = forceRemoveIntermediateContainers;
return this;
}

/**
* Map of string pairs for build-time variables.
* Users pass these values at build-time.
* Docker uses the buildargs as the environment context for command(s) run via the Dockerfile’s RUN instruction
* or for variable expansion in other Dockerfile instructions.
*
* @param buildArgs
* map of build arguments
* @return this params instance
*/
public BuildImageParams withBuildArgs(Map<String, String> buildArgs) {
this.buildArgs = buildArgs;
return this;
}

/**
* Adds build variable to build args.
* See {@link #withBuildArgs(Map)}
*
* @param key
* variable name
* @param value
* variable value
* @return this params instance
*/
public BuildImageParams addBuildArg(String key, String value) {
if (buildArgs == null) {
buildArgs = new HashMap<>();
}
buildArgs.put(key, value);
return this;
}

/**
* Sets path to alternate dockerfile in build context
*
Expand Down Expand Up @@ -261,39 +351,63 @@ public String getRemote() {
return remote;
}

public Boolean isQuiet() {
return quiet;
}

public Boolean isNoCache() {
return noCache;
}

public Boolean isRemoveIntermediateContainer() {
return removeIntermediateContainer;
}

public Boolean isForceRemoveIntermediateContainers() {
return forceRemoveIntermediateContainers;
}

public Map<String, String> getBuildArgs() {
return buildArgs;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof BuildImageParams)) {
return false;
}
final BuildImageParams that = (BuildImageParams)obj;
return Objects.equals(repository, that.repository)
&& Objects.equals(tag, that.tag)
&& Objects.equals(authConfigs, that.authConfigs)
&& Objects.equals(doForcePull, that.doForcePull)
&& Objects.equals(memoryLimit, that.memoryLimit)
&& Objects.equals(memorySwapLimit, that.memorySwapLimit)
&& getFiles().equals(that.getFiles())
&& Objects.equals(dockerfile, that.dockerfile)
&& Objects.equals(remote, that.remote);
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
BuildImageParams that = (BuildImageParams)o;
return Objects.equals(repository, that.repository) &&
Objects.equals(tag, that.tag) &&
Objects.equals(authConfigs, that.authConfigs) &&
Objects.equals(doForcePull, that.doForcePull) &&
Objects.equals(memoryLimit, that.memoryLimit) &&
Objects.equals(memorySwapLimit, that.memorySwapLimit) &&
Objects.equals(files, that.files) &&
Objects.equals(dockerfile, that.dockerfile) &&
Objects.equals(remote, that.remote) &&
Objects.equals(quiet, that.quiet) &&
Objects.equals(noCache, that.noCache) &&
Objects.equals(removeIntermediateContainer, that.removeIntermediateContainer) &&
Objects.equals(forceRemoveIntermediateContainers, that.forceRemoveIntermediateContainers) &&
Objects.equals(buildArgs, that.buildArgs);
}

@Override
public int hashCode() {
int hash = 7;
hash = 31 * hash + Objects.hashCode(repository);
hash = 31 * hash + Objects.hashCode(tag);
hash = 31 * hash + Objects.hashCode(authConfigs);
hash = 31 * hash + Objects.hashCode(doForcePull);
hash = 31 * hash + Objects.hashCode(memoryLimit);
hash = 31 * hash + Objects.hashCode(memorySwapLimit);
hash = 31 * hash + getFiles().hashCode();
hash = 31 * hash + Objects.hashCode(dockerfile);
hash = 31 * hash + Objects.hashCode(remote);
return hash;
return Objects.hash(repository,
tag,
authConfigs,
doForcePull,
memoryLimit,
memorySwapLimit,
files,
dockerfile,
remote,
quiet,
noCache,
removeIntermediateContainer,
forceRemoveIntermediateContainers,
buildArgs);
}

@Override
Expand All @@ -308,6 +422,12 @@ public String toString() {
", files=" + files +
", dockerfile='" + dockerfile + '\'' +
", remote='" + remote + '\'' +
", quiet=" + quiet +
", noCache=" + noCache +
", removeIntermediateContainer=" + removeIntermediateContainer +
", forceRemoveIntermediateContainers=" + forceRemoveIntermediateContainers +
", buildArgs=" + buildArgs +
'}';
}

}
2 changes: 0 additions & 2 deletions ...docker-client/src/test/java/org/eclipse/che/plugin/docker/client/DockerConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,8 +974,6 @@ public void shouldBeAbleToBuildImage() throws IOException, InterruptedException
verify(dockerConnectionFactory).openConnection(any(URI.class));
verify(dockerConnection).method(REQUEST_METHOD_POST);
verify(dockerConnection).path("/build");
verify(dockerConnection).query("rm", 1);
verify(dockerConnection).query("forcerm", 1);
verify(dockerConnection).header("Content-Type", "application/x-compressed-tar");
verify(dockerConnection).header(eq("Content-Length"), anyInt());
verify(dockerConnection).header(eq("X-Registry-Config"), any(byte[].class));
Expand Down
Loading