Skip to content

Commit

Permalink
fix(core.deployment): dpa.properties is written even if package is al…
Browse files Browse the repository at this point in the history 
…ready in persistence [backport release-5.3.0] (#4692)

fix(core.deployment): dpa.properties is written even if package is already in persistence (#4690)

* fix(core.deployment): dpa.properties is written even if package is already in persistence

Signed-off-by: Marcello Martina <[email protected]>

* fix(core.deployment.agent): use '-' as version separator, always try to update dpa.properties

Signed-off-by: Marcello Martina <[email protected]>

* fix: Delete downloaded dp only if exist, otherwise exception is thrown

Signed-off-by: Nicola Timeus <[email protected]>

* fix: Use underscore for dps and dash for scripts

Signed-off-by: Nicola Timeus <[email protected]>

* fix(tests): fixed tests

Signed-off-by: Marcello Martina <[email protected]>

* revert: stored name is constructed from DP info from deploymentAgent and not from metrics

Signed-off-by: Marcello Martina <[email protected]>

* fix: Attempt to fix test

Signed-off-by: Nicola Timeus <[email protected]>

---------

Signed-off-by: Marcello Martina <[email protected]>
Signed-off-by: Nicola Timeus <[email protected]>
Co-authored-by: Nicola Timeus <[email protected]>
(cherry picked from commit f952678)

Co-authored-by: Marcello Rinaldo Martina <[email protected]>
  • Loading branch information
@github-actions @marcellorinaldo
github-actions[bot] and marcellorinaldo authored May 29, 2023
1 parent 648c46a commit 4d9f510
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 33 deletions.
4 changes: 2 additions & 2 deletions ...oyment/src/main/java/org/eclipse/kura/core/deployment/download/DownloadFileUtilities.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ public static File getDpDownloadFile(DeploymentPackageInstallOptions options) th
String downloadDirectory = options.getDownloadDirectory();
String packageFilename;
if (!options.getSystemUpdate()) {
String dpName = FileUtilities.getFileName(options.getDpName(), options.getDpVersion(), ".dp");
String dpName = FileUtilities.getFileName(options.getDpName(), options.getDpVersion(), ".dp", "_");
packageFilename = new StringBuilder().append(downloadDirectory).append(File.separator).append(dpName)
.toString();
} else {
String shName = FileUtilities.getFileName(options.getDpName(), options.getDpVersion(), ".sh");
String shName = FileUtilities.getFileName(options.getDpName(), options.getDpVersion(), ".sh", "-");
packageFilename = new StringBuilder().append(downloadDirectory).append(File.separator).append(shName)
.toString();
}
Expand Down
5 changes: 3 additions & 2 deletions ...deployment/src/main/java/org/eclipse/kura/core/deployment/download/impl/DownloadImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ private void incrementalDownloadFromURL(File dpFile, String url, int downloadInd
}
s_logger.info("--> Going to verify hash signature!");
try {
// these things should be checked beforehand, so that hash() has a chance to succeed
// these things should be checked beforehand, so that hash() has a chance to
// succeed
if (hashAlgorithm == null || "".equals(hashAlgorithm) || hashValue == null || "".equals(hashValue)) {
throw new KuraException(KuraErrorCode.INTERNAL_ERROR, null,
"Failed to verify checksum with empty algorithm: " + hashAlgorithm);
Expand Down Expand Up @@ -278,7 +279,7 @@ private void downloadFailedAsync(int downloadIndex, Exception e) {

private File getDpVerifierFile(DeploymentPackageInstallOptions options) {

String shName = FileUtilities.getFileName(options.getDpName(), options.getDpVersion(), ".sh_verifier.sh");
String shName = FileUtilities.getFileName(options.getDpName(), options.getDpVersion(), ".sh_verifier.sh", "-");
String packageFilename = new StringBuilder().append(this.verificationDirectory).append(File.separator)
.append(shName).toString();

Expand Down
42 changes: 21 additions & 21 deletions ...a.core.deployment/src/main/java/org/eclipse/kura/core/deployment/install/InstallImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.eclipse.kura.core.deployment.DeploymentPackageOptions;
import org.eclipse.kura.core.deployment.InstallStatus;
import org.eclipse.kura.core.deployment.download.DeploymentPackageDownloadOptions;
import org.eclipse.kura.core.deployment.util.FileUtilities;
import org.eclipse.kura.core.linux.executor.LinuxSignal;
import org.eclipse.kura.executor.Command;
import org.eclipse.kura.executor.CommandExecutorService;
Expand Down Expand Up @@ -272,36 +273,35 @@ private DeploymentPackage installDeploymentPackageInternal(File fileReference)
throws DeploymentException, IOException {

DeploymentPackage dp = null;
File dpPersistentFile = null;
File downloadedFile = fileReference;
File packagesFolder = new File(this.packagesPath);
StringBuilder dpPersistentFilePath = new StringBuilder();

try (InputStream dpInputStream = new FileInputStream(downloadedFile);) {
StringBuilder pathSB = new StringBuilder();
pathSB.append(this.packagesPath);
pathSB.append(File.separator);

dp = this.deploymentAdmin.installDeploymentPackage(dpInputStream);

pathSB.append(dp.getName()).append("_");
pathSB.append(dp.getVersion()).append(".dp");
String dpPersistentFilePath = pathSB.toString();
dpPersistentFile = new File(pathSB.toString());

// Now we need to copy the deployment package file to the Kura
// packages directory unless it's already there.
if (!downloadedFile.getCanonicalPath().startsWith(packagesFolder.getCanonicalPath())) {
logger.debug("dpFile.getCanonicalPath(): {}", downloadedFile.getCanonicalPath());
logger.debug("dpPersistentFile.getCanonicalPath(): {}", dpPersistentFile.getCanonicalPath());
FileUtils.copyFile(downloadedFile, dpPersistentFile);
addPackageToConfFile(dp.getName(), "file:" + dpPersistentFilePath);
dpPersistentFilePath.append(this.packagesPath);
dpPersistentFilePath.append(File.separator);
dpPersistentFilePath
.append(FileUtilities.getFileName(dp.getName(), dp.getVersion().toString(), ".dp", "_"));
File dpPersistentFile = new File(dpPersistentFilePath.toString());

boolean isDownloadedInPersistentPath = downloadedFile.getCanonicalPath()
.equals(dpPersistentFile.getCanonicalPath());

if (!isDownloadedInPersistentPath) {
logger.info("Moving downloaded DP from '{}' to '{}'.", downloadedFile.getCanonicalPath(),
dpPersistentFile.getCanonicalPath());
Files.deleteIfExists(dpPersistentFile.toPath());
FileUtils.moveFile(downloadedFile, dpPersistentFile);
}
} catch (IOException ex) {

addPackageToConfFile(dp.getName(), "file:" + dpPersistentFilePath.toString());
} catch (IOException ex) {
logger.error("Unable to move downloaded DP from '" + downloadedFile.getCanonicalPath() + "' to '"
+ dpPersistentFilePath.toString() + "'.", ex);
} finally {
// The file from which we have installed the deployment package will be deleted
// unless it's a persistent deployment package file.
if (!downloadedFile.getCanonicalPath().startsWith(packagesFolder.getCanonicalPath())) {
if (!downloadedFile.getCanonicalPath().equals(dpPersistentFilePath.toString())) {
downloadedFile.delete();
}
}
Expand Down
8 changes: 5 additions & 3 deletions ...ra.core.deployment/src/main/java/org/eclipse/kura/core/deployment/util/FileUtilities.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
public class FileUtilities {

public static String getFileName(String dpName, String dpVersion, String extension) {
String packageFilename = new StringBuilder().append(dpName).append("-").append(dpVersion).append(extension)
.toString();
return packageFilename;
return getFileName(dpName, dpVersion, extension, "-");
}

public static String getFileName(String dpName, String dpVersion, String extension, String separator) {
return dpName + separator + dpVersion + extension;
}
}
8 changes: 5 additions & 3 deletions ...eployment.agent/src/main/java/org/eclipse/kura/deployment/agent/impl/DeploymentAgent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,15 @@ private DeploymentPackage installDeploymentPackageInternal(String urlSpec)
if (!dpFile.getCanonicalPath().equals(dpPersistentFile.getCanonicalPath())) {
logger.debug("dpFile.getCanonicalPath(): {}", dpFile.getCanonicalPath());
logger.debug("dpPersistentFile.getCanonicalPath(): {}", dpPersistentFile.getCanonicalPath());
FileUtils.copyFile(dpFile, dpPersistentFile);
addPackageToConfFile(dp.getName(), "file:" + dpPersistentFilePath);
Files.deleteIfExists(dpPersistentFile.toPath());
FileUtils.moveFile(dpFile, dpPersistentFile);
}

addPackageToConfFile(dp.getName(), "file:" + dpPersistentFilePath);
} finally {
// The file from which we have installed the deployment package will be deleted
// unless it's a persistent deployment package file.
if (dpPersistentFile != null && dpPersistentFile.exists()
if (dpPersistentFile != null && dpPersistentFile.exists() && dpFile.exists()
&& !dpFile.getCanonicalPath().equals(dpPersistentFile.getCanonicalPath())) {
Files.delete(dpFile.toPath());
logger.debug("Deleted file: {}", dpFile.getName());
Expand Down
17 changes: 15 additions & 2 deletions ...ployment.test/src/test/java/org/eclipse/kura/core/deployment/install/InstallImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.osgi.framework.Version;
import org.osgi.service.deploymentadmin.DeploymentAdmin;
import org.osgi.service.deploymentadmin.DeploymentException;
import org.osgi.service.deploymentadmin.DeploymentPackage;
Expand Down Expand Up @@ -89,12 +90,19 @@ public void testGetDeployedPackages() throws IOException {
}

@Test
public void testInstallDpSuccessMessage() throws KuraException {
public void testInstallDpSuccessMessage() throws KuraException, DeploymentException {
CloudDeploymentHandlerV2 callbackMock = mock(CloudDeploymentHandlerV2.class);
String kuraDataDir = "/tmp";
InstallImpl ii = new InstallImpl(callbackMock, kuraDataDir, null);
ii.setPackagesPath(kuraDataDir);

DeploymentAdmin mockDeploymentAdmin = mock(DeploymentAdmin.class);
DeploymentPackage dpMock = mock(DeploymentPackage.class);
when(dpMock.getName()).thenReturn("dpname");
when(mockDeploymentAdmin.installDeploymentPackage(Mockito.any())).thenReturn(dpMock);
ii.setDeploymentAdmin(mockDeploymentAdmin);
ii.setDpaConfPath("/tmp/dpa.properties");

final String clientId = "clientid";
final long jobid = 1234;

Expand All @@ -113,8 +121,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
KuraInstallPayload kp = (KuraInstallPayload) arguments[1];

assertEquals("", clientId, kp.getClientId());
assertEquals("", 100, kp.getInstallProgress());
assertEquals("", InstallStatus.COMPLETED.getStatusString(), kp.getInstallStatus());
assertEquals("", 100, kp.getInstallProgress());
assertEquals("", jobid, kp.getJobId().longValue());

return null;
Expand Down Expand Up @@ -195,6 +203,8 @@ public void testInstallDeploymentPackageInternal() throws Throwable {

ii.setPackagesPath(kuraDataDir);

ii.setDpaConfPath("/tmp/dpa.properties");

File persDir = new File(kuraDataDir, "persistance");
persDir.mkdirs();
File veriDir = new File(persDir, "verification");
Expand All @@ -205,6 +215,8 @@ public void testInstallDeploymentPackageInternal() throws Throwable {
dpFile.createNewFile();

DeploymentPackage dpMock = mock(DeploymentPackage.class);
when(dpMock.getName()).thenReturn("dp");
when(dpMock.getVersion()).thenReturn(new Version("1.0.0"));
when(deploymentAdminMock.installDeploymentPackage(any())).thenReturn(dpMock);

Object dp = TestUtil.invokePrivate(ii, "installDeploymentPackageInternal", dpFile);
Expand Down Expand Up @@ -246,6 +258,7 @@ public Properties getDeployedPackages() {
when(deploymentAdminMock.installDeploymentPackage(any())).thenReturn(dpMock);

when(dpMock.getName()).thenReturn("dpname");
when(dpMock.getVersion()).thenReturn(new Version("1.0.0"));

ii.setDpaConfPath(null); // make sure this is null, so that we don't test too much
ii.setPackagesPath(pkgDir.getCanonicalPath());
Expand Down

0 comments on commit 4d9f510

Please sign in to comment.