-
Notifications
You must be signed in to change notification settings - Fork 306
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.
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
FISH-5799: access docker instances logs via docker rest #5544
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,7 +37,7 @@ | |
* only if the new code is made subject to such option by the copyright | ||
* holder. | ||
* | ||
* Portions Copyright [2017-2018] Payara Foundation and/or affiliates | ||
* Portions Copyright [2017-2021] Payara Foundation and/or affiliates | ||
*/ | ||
|
||
package com.sun.enterprise.server.logging.logviewer.backend; | ||
|
@@ -53,11 +53,17 @@ | |
import com.trilead.ssh2.SFTPv3DirectoryEntry; | ||
import com.trilead.ssh2.SFTPv3FileAttributes; | ||
|
||
import fish.payara.enterprise.server.logging.TrivialTarInputStream; | ||
import java.io.BufferedInputStream; | ||
import java.io.BufferedOutputStream; | ||
import java.io.File; | ||
import java.io.FileOutputStream; | ||
import java.io.IOException; | ||
import java.net.URL; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import java.nio.file.StandardCopyOption; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.logging.Logger; | ||
|
@@ -178,14 +184,60 @@ public File downloadGivenInstanceLogFile(ServiceLocator habitat, Server targetSe | |
|
||
wrf.copyTo(instanceLogFile); | ||
} catch (WindowsException ex) { | ||
throw new IOException("Unable to download instance log file from DCOM Instance Node"); | ||
throw new IOException("Unable to download instance log file from DCOM Instance Node", ex); | ||
} | ||
|
||
} else if (node.getType().equals("DOCKER") || node.getType().equals("TEMP")) { | ||
String dockerContainerId = targetServer.getDockerContainerId(); | ||
String nodeHost = node.getNodeHost(); | ||
int nodeDockerPort = Integer.valueOf(node.getDockerPort()); | ||
try { | ||
if (dockerContainerId == null && node.getType().equals("TEMP")) { | ||
// FIXME: it would be easies, if the TEMP server had docker container id set! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An instance on a TEMP node should have it set? From the docker-node entrypoint script, it should get set when creating the instance:
The script even makes efforts to ensure that it at least has something set right at the start to avoid failing that if check:
It also later on makes efforts to check that the container ID matches:
So I think the only way it couldn't be set is if you're not using the script? In which case I'd just exit out and reach for my "stop holding it wrong" bat |
||
// temporary docker node, we need to access the original node, cut the added generated name | ||
String originalInstanceName = instanceName.substring(0, instanceName.lastIndexOf('-')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is safe |
||
Server originalInstance = domain.getServerNamed(originalInstanceName); | ||
dockerContainerId = originalInstance.getDockerContainerId(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the originalInstance is null? If that is the case this will throw NullPointerException There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole part is a temporary solution. We need to get the docker host and port from the instance/node, not by searching other instance. So we need to fix creation of the running docker instance+node, fill the information and use it here. |
||
String originalNodeName = originalInstance.getNodeRef(); | ||
Node originalNode = nodes.getNode(originalNodeName); | ||
nodeHost = originalNode.getNodeHost(); | ||
nodeDockerPort = Integer.valueOf(originalNode.getDockerPort()); | ||
logFileName = "server.log"; | ||
} | ||
File logFileDirectoryOnServer = makingDirectory(domainRoot + File.separator + "logs" | ||
+ File.separator + instanceName); | ||
|
||
String loggingDir = getLoggingDirectoryForNode(instanceLogFileName, node, sNode, instanceName); | ||
|
||
if (logFileName == null || logFileName.equals("")) { | ||
logFileName = "server.log"; | ||
} | ||
//"/opt/payara/appserver/glassfish/nodes/Hilarious-Angelfish/docker-instance-LovelyChar/logs/server.log" | ||
String remotePath = loggingDir + "/" + logFileName; | ||
|
||
String prefix = "http" + (Boolean.valueOf(node.getUseTls()) ? "s" : ""); | ||
|
||
// store the log file to the given path | ||
URL logUrl = new URL(prefix, nodeHost, nodeDockerPort, "/containers/" | ||
+ dockerContainerId | ||
+ "/archive?path=" | ||
+ remotePath); | ||
BufferedInputStream downloadFile = new BufferedInputStream(logUrl.openStream()); | ||
// ArchiveInputStream tarStream = new ArchiveStreamFactory().createArchiveInputStream(downloadFile); | ||
TrivialTarInputStream tarStream = new TrivialTarInputStream(downloadFile); | ||
Path instanceLogPath = Paths.get(logFileDirectoryOnServer.getAbsolutePath(), logFileName); | ||
//Path instanceLogPathTar = Paths.get(logFileDirectoryOnServer.getAbsolutePath() + ".tar", logFileName); | ||
Files.copy(tarStream, instanceLogPath, StandardCopyOption.REPLACE_EXISTING); | ||
// Files.copy(downloadFile, instanceLogPath, StandardCopyOption.REPLACE_EXISTING); | ||
instanceLogFile = instanceLogPath.toFile(); | ||
} catch (IOException ex) { | ||
// } catch (IOException | ArchiveException ex) { | ||
throw new IOException("Unable to download file from docker node at " + nodeHost + ":" + nodeDockerPort + ", instance " + instanceName + ", container " + dockerContainerId, ex); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. definitely increasing complexity with this addition 😉 |
||
|
||
} | ||
|
||
return instanceLogFile; | ||
|
||
} | ||
|
||
public void downloadAllInstanceLogFiles(ServiceLocator habitat, Server targetServer, Domain domain, Logger logger, | ||
|
@@ -402,7 +454,7 @@ private File makingDirectory(String path) { | |
} | ||
|
||
} | ||
created = targetDir.mkdir(); | ||
created = targetDir.mkdirs(); | ||
if (!created) { | ||
return null; | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
/* | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. | ||
* | ||
* Copyright (c) [2021] Payara Foundation and/or its affiliates. All rights reserved. | ||
* | ||
* The contents of this file are subject to the terms of either the GNU | ||
* General Public License Version 2 only ("GPL") or the Common Development | ||
* and Distribution License("CDDL") (collectively, the "License"). You | ||
* may not use this file except in compliance with the License. You can | ||
* obtain a copy of the License at | ||
* https://github.com/payara/Payara/blob/master/LICENSE.txt | ||
* See the License for the specific | ||
* language governing permissions and limitations under the License. | ||
* | ||
* When distributing the software, include this License Header Notice in each | ||
* file and include the License file at glassfish/legal/LICENSE.txt. | ||
* | ||
* GPL Classpath Exception: | ||
* The Payara Foundation designates this particular file as subject to the "Classpath" | ||
* exception as provided by the Payara Foundation in the GPL Version 2 section of the License | ||
* file that accompanied this code. | ||
* | ||
* Modifications: | ||
* If applicable, add the following below the License Header, with the fields | ||
* enclosed by brackets [] replaced by your own identifying information: | ||
* "Portions Copyright [year] [name of copyright owner]" | ||
* | ||
* Contributor(s): | ||
* If you wish your version of this file to be governed by only the CDDL or | ||
* only the GPL Version 2, indicate your decision by adding "[Contributor] | ||
* elects to include this software in this distribution under the [CDDL or GPL | ||
* Version 2] license." If you don't indicate a single choice of license, a | ||
* recipient has the option to distribute your version of this file under | ||
* either the CDDL, the GPL Version 2 or to extend the choice of license to | ||
* its licensees as provided above. However, if you add GPL Version 2 code | ||
* and therefore, elected the GPL Version 2 license, then the option applies | ||
* only if the new code is made subject to such option by the copyright | ||
* holder. | ||
*/ | ||
package fish.payara.enterprise.server.logging; | ||
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
|
||
/** | ||
* Trivial implementation for TAR file downloaded from Docker, expecting regular file, ignoring filename. | ||
* | ||
* @author Petr Aubrecht <aubrecht@asoftware.cz> | ||
*/ | ||
public class TrivialTarInputStream extends InputStream { | ||
|
||
private final InputStream sourceStream; | ||
private byte[] header = null; | ||
private long size; | ||
private long readSoFar = 0; | ||
|
||
public TrivialTarInputStream(InputStream sourceStream) { | ||
this.sourceStream = sourceStream; | ||
} | ||
|
||
@Override | ||
public int read() throws IOException { | ||
if (header == null) { | ||
header = new byte[512]; | ||
sourceStream.read(header); | ||
byte fileLinkType = header[156]; | ||
if (fileLinkType != 0 && fileLinkType != 48 /*'0'*/) { | ||
throw new IOException("Tar file is not a normal file, type is " + fileLinkType); | ||
} | ||
String sizeStr = new String(header, 124, 11, "ASCII"); | ||
size = Integer.valueOf(sizeStr, 8); | ||
} | ||
if (readSoFar >= size) { | ||
// we are at the end of file | ||
return -1; | ||
} | ||
readSoFar++; | ||
return sourceStream.read(); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly a bit blanket - I don't think there's strictly anything preventing you creating a temp node outside of a container and on localhost.
The intent is definitely that TEMP nodes should only really exist in cloud environments, so I guess it depends how defensive you want to be - it's probably fine to just update the comment to something like "Assume TEMP instances are within a container"