Skip to content

Commit

Permalink
VBV-1105: imageReference can not be set in templates
Browse files Browse the repository at this point in the history 
ContainerDescription.imageReference could not be set in yaml templates because
the property was ignored, so it used to be imported as customProperty.
This fix allows to set imageReference in yaml templates. Additionally, it allows
to use a file reference (file:///path_to_file). In case the scheme is file,
DockerAdapterService loads the file without downloading it to a temporary
file. The docker compose format is not affected by this change.

Finally, the fix adds a unit test for importing templates with imageReference,
and removed and old workaround for an issue loading a file which was fixed
long ago.

Change-Id: I72a44ce24cff3d1d8ff1a462a5998ef1a44d1046
Reviewed-on: http://bellevue-ci.eng.vmware.com:8080/7945
Compute-Verified: jenkins <[email protected]>
Upgrade-Verified: jenkins <[email protected]>
Bellevue-Verified: jenkins <[email protected]>
Reviewed-by: Tony Georgiev <[email protected]>
CS-Verified: jenkins <[email protected]>
  • Loading branch information
@jdillet
jdillet committed Mar 20, 2017
1 parent 35da927 commit 9c055f8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 42 deletions.
79 changes: 38 additions & 41 deletions .../docker/src/main/java/com/vmware/admiral/adapter/docker/service/DockerAdapterService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ public class DockerAdapterService extends AbstractDockerAdapterService {
*/
private static final String DOWNLOAD_TEMPFILE_PREFIX = "admiral";

private static final String FILE_SCHEME = "file";

public static final String SELF_LINK = ManagementUriParts.ADAPTER_DOCKER;

public static final String PROVISION_CONTAINER_RETRIES_COUNT_PARAM_NAME = "provision.container.retries.count";
Expand Down Expand Up @@ -545,49 +547,44 @@ private void processContainerDescription(RequestContext context) {
getHost().log(Level.INFO, "Downloading image from: %s %s", imageReference,
context.request.getRequestTrackingLog());
try {
File tempFile = File.createTempFile(DOWNLOAD_TEMPFILE_PREFIX, null);
tempFile.deleteOnExit();

Operation fetchOp = Operation.createGet(imageReference);

fetchOp.setExpiration(ServiceUtils.getExpirationTimeFromNowInMicros(
getHost().getOperationTimeoutMicros()))
.setReferer(UriUtils.buildUri(getHost(), SELF_LINK))
.setContextId(context.request.getRequestId())
.setCompletion((o, ex) -> {
if (ex != null) {
if (!tempFile.delete()) {
this.logWarning("Failed to delete temp file: %s %s", tempFile,
context.request.getRequestTrackingLog());
}
fail(context.request, ex);

} else {
// Hack: until issue:
// https://www.pivotaltracker.com/projects/1471320/stories/111849709
// tempFile is not ready by the time it gets here.
for (int i = 0; i < 200; i++) {
if (tempFile.length() > 0) {
break;
if (FILE_SCHEME.equals(imageReference.getScheme())) {
// for file scheme use the file and do not delete it (it is not a temp copy)
processDownloadedImage(context, new File(imageReference),
imageCompletionHandler, false);
} else {
// for not file scheme, download it to a temp file
File tempFile = File.createTempFile(DOWNLOAD_TEMPFILE_PREFIX, null);
tempFile.deleteOnExit();

Operation fetchOp = Operation.createGet(imageReference);

fetchOp.setExpiration(ServiceUtils.getExpirationTimeFromNowInMicros(
getHost().getOperationTimeoutMicros()))
.setReferer(UriUtils.buildUri(getHost(), SELF_LINK))
.setContextId(context.request.getRequestId())
.setCompletion((o, ex) -> {
if (ex != null) {
if (!tempFile.delete()) {
this.logWarning("Failed to delete temp file: %s %s",
tempFile, context.request.getRequestTrackingLog());
}
fail(context.request, ex);

try {
Thread.sleep(50);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
getHost().log(Level.INFO,
"Finished download of %d bytes from %s to %s %s",
tempFile.length(), o.getUri(), tempFile.getAbsolutePath(),
context.request.getRequestTrackingLog());
} else {
getHost().log(Level.INFO,
"Finished download of %d bytes from %s to %s %s",
tempFile.length(), o.getUri(),
tempFile.getAbsolutePath(),
context.request.getRequestTrackingLog());

processDownloadedImage(context, tempFile, imageCompletionHandler);
}
});
processDownloadedImage(context, tempFile,
imageCompletionHandler, true);
}
});

// TODO ssl trust / credentials for the image server
FileUtils.getFile(getHost().getClient(), fetchOp, tempFile);
// TODO ssl trust / credentials for the image server
FileUtils.getFile(getHost().getClient(), fetchOp, tempFile);
}

} catch (IOException x) {
throw new RuntimeException("Failure downloading image from: " + imageReference
Expand All @@ -605,7 +602,7 @@ private void processContainerDescription(RequestContext context) {
* @param imageCompletionHandler
*/
private void processDownloadedImage(RequestContext context, File tempFile,
CompletionHandler imageCompletionHandler) {
CompletionHandler imageCompletionHandler, boolean isTempFile) {

Operation fileReadOp = Operation.createPatch(null)
.setContextId(context.request.getRequestId())
Expand All @@ -616,7 +613,7 @@ private void processDownloadedImage(RequestContext context, File tempFile,
}

byte[] imageData = o.getBody(byte[].class);
if (!tempFile.delete()) {
if (isTempFile && !tempFile.delete()) {
this.logWarning("Failed to delete temp file: %s %s", tempFile,
context.request.getRequestTrackingLog());
}
Expand Down
3 changes: 2 additions & 1 deletion compute/src/main/java/com/vmware/admiral/compute/container/ContainerDescriptionService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,13 @@ public static class ContainerDescription extends ResourceState implements Clonea
@Documentation(description = "Link to the parent container description.")
@UsageOption(option = PropertyUsageOption.OPTIONAL)
public String parentDescriptionLink;

/**
* (Optional) An image reference to a docker image in .tgz format to be downloaded to the
* server and pushed to the local host repository.
*/
@JsonProperty(value = "image_reference")
@Documentation(description = "An image reference to a docker image in .tgz format to be downloaded to the server and pushed to the local host repository.")
@JsonIgnore
@UsageOption(option = PropertyUsageOption.OPTIONAL)
public URI imageReference;

Expand Down
9 changes: 9 additions & 0 deletions .../test/java/com/vmware/admiral/compute/content/CompositeDescriptionContentServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.vmware.admiral.compute.container.CompositeDescriptionService.CompositeDescription;
import com.vmware.admiral.compute.container.CompositeDescriptionService.CompositeDescriptionExpanded;
import com.vmware.admiral.compute.container.ComputeBaseTest;
import com.vmware.admiral.compute.container.ContainerDescriptionService.ContainerDescription;
import com.vmware.admiral.compute.kubernetes.entities.common.BaseKubernetesObject;
import com.vmware.admiral.compute.network.ComputeNetworkDescriptionService.ComputeNetworkDescription;
import com.vmware.photon.controller.model.Constraint;
Expand Down Expand Up @@ -97,6 +98,14 @@ public CompositeDescriptionContentServiceTest(String templateFileName,
cd.customProperties.get("_leaseDays"));

descLinks.addAll(cd.descriptionLinks);

CompositeDescriptionExpanded cdExpanded = o.getBody(CompositeDescriptionExpanded.class);
for (ComponentDescription component : cdExpanded.componentDescriptions) {
ContainerDescription containerDescription = Utils
.fromJson(component.componentJson, ContainerDescription.class);
assertNotNull(containerDescription.image);
assertNotNull(containerDescription.imageReference);
}
};

private static BiConsumer<Operation, List<String>> verifyKubernetesTemplate = (o, descLinks)
Expand Down
2 changes: 2 additions & 0 deletions compute/src/test/resources/WordPress_with_MySQL_containers.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ components:
data:
name: "wordpress"
image: "centurylink/wordpress:3.9.1"
image_reference: "http://registry.hub.docker.com/centurylink/wordpress.tgz"
_cluster: 2
env:
- var: "DB_PASSWORD"
Expand Down Expand Up @@ -35,6 +36,7 @@ components:
data:
name: "mysql"
image: "centurylink/mysql:5.5"
image_reference: "http://registry.hub.docker.com/centurylink/mysql.tgz"
env:
- var: "MYSQL_ROOT_PASSWORD"
value: "pass@word01"
Expand Down

0 comments on commit 9c055f8

Please sign in to comment.