Skip to content

Commit 7cf06ed

Browse files
nvazqueznvazquez
committed
Fix background task for uploading certificates to hosts
1 parent cd5f473 commit 7cf06ed

File tree

7 files changed

+118
-43
lines changed

7 files changed

+118
-43
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/RevokeTemplateDirectDownloadCertificateCmd.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.cloudstack.api.BaseCmd;
3131
import org.apache.cloudstack.api.Parameter;
3232
import org.apache.cloudstack.api.ServerApiException;
33+
import org.apache.cloudstack.api.response.HostResponse;
3334
import org.apache.cloudstack.api.response.SuccessResponse;
3435
import org.apache.cloudstack.api.response.ZoneResponse;
3536
import org.apache.cloudstack.context.CallContext;
@@ -58,13 +59,17 @@ public class RevokeTemplateDirectDownloadCertificateCmd extends BaseCmd {
5859
private String certificateAlias;
5960

6061
@Parameter(name = ApiConstants.HYPERVISOR, type = BaseCmd.CommandType.STRING, required = true,
61-
description = "Hypervisor type")
62+
description = "hypervisor type")
6263
private String hypervisor;
6364

6465
@Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class,
65-
description = "Zone to upload certificate", required = true)
66+
description = "zone to revoke certificate", required = true)
6667
private Long zoneId;
6768

69+
@Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class,
70+
description = "(optional) the host ID to revoke certificate")
71+
private Long hostId;
72+
6873
@Override
6974
public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
7075
if (!hypervisor.equalsIgnoreCase("kvm")) {
@@ -73,7 +78,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
7378
SuccessResponse response = new SuccessResponse(getCommandName());
7479
try {
7580
LOG.debug("Revoking certificate " + certificateAlias + " from " + hypervisor + " hosts");
76-
boolean result = directDownloadManager.revokeCertificateAlias(certificateAlias, hypervisor, zoneId);
81+
boolean result = directDownloadManager.revokeCertificateAlias(certificateAlias, hypervisor, zoneId, hostId);
7782
response.setSuccess(result);
7883
setResponseObject(response);
7984
} catch (Exception e) {

‎api/src/main/java/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.cloudstack.api.Parameter;
2424
import org.apache.cloudstack.api.ServerApiException;
2525
import org.apache.cloudstack.api.ApiErrorCode;
26+
import org.apache.cloudstack.api.response.HostResponse;
2627
import org.apache.cloudstack.api.response.SuccessResponse;
2728
import org.apache.cloudstack.api.response.ZoneResponse;
2829
import org.apache.cloudstack.context.CallContext;
@@ -61,6 +62,10 @@ public class UploadTemplateDirectDownloadCertificateCmd extends BaseCmd {
6162
description = "Zone to upload certificate", required = true)
6263
private Long zoneId;
6364

65+
@Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class,
66+
description = "(optional) the host ID to revoke certificate")
67+
private Long hostId;
68+
6469
@Override
6570
public void execute() {
6671
if (!hypervisor.equalsIgnoreCase("kvm")) {
@@ -69,7 +74,7 @@ public void execute() {
6974

7075
try {
7176
LOG.debug("Uploading certificate " + name + " to agents for Direct Download");
72-
boolean result = directDownloadManager.uploadCertificateToHosts(certificate, name, hypervisor, zoneId);
77+
boolean result = directDownloadManager.uploadCertificateToHosts(certificate, name, hypervisor, zoneId, hostId);
7378
SuccessResponse response = new SuccessResponse(getCommandName());
7479
response.setSuccess(result);
7580
setResponseObject(response);

‎api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ public interface DirectDownloadManager extends DirectDownloadService, PluggableS
2626

2727
ConfigKey<Long> DirectDownloadCertificateUploadInterval = new ConfigKey<>("Advanced", Long.class,
2828
"direct.download.certificate.background.task.interval",
29-
"24",
30-
"The Direct Download framework background interval in hours.",
31-
true);
29+
"0",
30+
"This interval (in hours) controls a background task to sync hosts within enabled zones " +
31+
"missing uploaded certificates for direct download." +
32+
"Only certificates which have not been revoked from hosts are uploaded",
33+
false);
3234

3335
/**
3436
* Revoke direct download certificate with alias 'alias' from hosts of hypervisor type 'hypervisor'
3537
*/
36-
boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId);
38+
boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId, Long hostId);
3739
}

‎engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapVO.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,16 @@ public class DirectDownloadCertificateHostMapVO {
3838
@Column(name = "certificate_id")
3939
private Long certificateId;
4040

41+
@Column(name = "revoked")
42+
private Boolean revoked;
43+
4144
public DirectDownloadCertificateHostMapVO() {
4245
}
4346

4447
public DirectDownloadCertificateHostMapVO(Long certificateId, Long hostId) {
4548
this.certificateId = certificateId;
4649
this.hostId = hostId;
50+
this.revoked = false;
4751
}
4852

4953
public Long getId() {
@@ -69,4 +73,12 @@ public Long getCertificateId() {
6973
public void setCertificateId(Long certificateId) {
7074
this.certificateId = certificateId;
7175
}
76+
77+
public Boolean isRevoked() {
78+
return revoked;
79+
}
80+
81+
public void setRevoked(Boolean revoked) {
82+
this.revoked = revoked;
83+
}
7284
}

‎engine/schema/src/main/resources/META-INF/db/schema-41200to41300.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ CREATE TABLE `cloud`.`direct_download_certificate_host_map` (
378378
`id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
379379
`certificate_id` bigint(20) unsigned NOT NULL,
380380
`host_id` bigint(20) unsigned NOT NULL,
381+
`revoked` int(1) NOT NULL DEFAULT 0,
381382
PRIMARY KEY (`id`),
382383
KEY `fk_direct_download_certificate_host_map__host_id` (`host_id`),
383384
KEY `fk_direct_download_certificate_host_map__certificate_id` (`certificate_id`),

‎framework/direct-download/src/main/java/org/apache/cloudstack/framework/agent/direct/download/DirectDownloadService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public interface DirectDownloadService {
2727
/**
2828
* Upload client certificate to each running host
2929
*/
30-
boolean uploadCertificateToHosts(String certificateCer, String certificateName, String hypervisor, Long zoneId);
30+
boolean uploadCertificateToHosts(String certificateCer, String certificateName, String hypervisor, Long zoneId, Long hostId);
3131

3232
/**
3333
* Upload a stored certificate on database with id 'certificateId' to host with id 'hostId'

‎server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java‎

Lines changed: 84 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.cloud.storage.dao.VMTemplateDao;
4242
import com.cloud.storage.dao.VMTemplatePoolDao;
4343
import com.cloud.utils.component.ManagerBase;
44+
import com.cloud.utils.concurrency.NamedThreadFactory;
4445
import com.cloud.utils.exception.CloudRuntimeException;
4546

4647
import java.net.URI;
@@ -55,8 +56,12 @@
5556
import java.util.Map;
5657
import java.util.Arrays;
5758
import java.util.Collections;
59+
import java.util.concurrent.ScheduledExecutorService;
60+
import java.util.concurrent.ScheduledThreadPoolExecutor;
61+
import java.util.concurrent.TimeUnit;
5862
import java.util.stream.Collectors;
5963
import javax.inject.Inject;
64+
import javax.naming.ConfigurationException;
6065

6166
import com.cloud.utils.security.CertificateHelper;
6267
import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand;
@@ -76,7 +81,6 @@
7681
import org.apache.cloudstack.framework.config.ConfigKey;
7782
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
7883
import org.apache.cloudstack.poll.BackgroundPollManager;
79-
import org.apache.cloudstack.poll.BackgroundPollTask;
8084
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
8185
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
8286
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
@@ -116,6 +120,8 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown
116120
@Inject
117121
private DataCenterDao dataCenterDao;
118122

123+
protected ScheduledExecutorService executorService;
124+
119125
@Override
120126
public List<Class<?>> getCommands() {
121127
final List<Class<?>> cmdList = new ArrayList<Class<?>>();
@@ -380,23 +386,36 @@ protected void certificateSanity(String certificatePem) {
380386
}
381387

382388
@Override
383-
public boolean uploadCertificateToHosts(String certificateCer, String alias, String hypervisor, Long zoneId) {
389+
public boolean uploadCertificateToHosts(String certificateCer, String alias, String hypervisor, Long zoneId, Long hostId) {
384390
if (alias != null && (alias.equalsIgnoreCase("cloud") || alias.startsWith("cloudca"))) {
385391
throw new CloudRuntimeException("Please provide a different alias name for the certificate");
386392
}
387393

394+
List<HostVO> hosts;
395+
DirectDownloadCertificateVO certificateVO;
388396
HypervisorType hypervisorType = HypervisorType.getType(hypervisor);
389-
List<HostVO> hosts = getRunningHostsToUploadCertificate(zoneId, hypervisorType);
390397

391-
String certificatePem = getPretifiedCertificate(certificateCer);
392-
certificateSanity(certificatePem);
398+
if (hostId == null) {
399+
hosts = getRunningHostsToUploadCertificate(zoneId, hypervisorType);
400+
401+
String certificatePem = getPretifiedCertificate(certificateCer);
402+
certificateSanity(certificatePem);
393403

394-
DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findByAlias(alias, hypervisorType, zoneId);
395-
if (certificateVO != null) {
396-
throw new CloudRuntimeException("Certificate alias " + alias + " has been already created");
404+
certificateVO = directDownloadCertificateDao.findByAlias(alias, hypervisorType, zoneId);
405+
if (certificateVO != null) {
406+
throw new CloudRuntimeException("Certificate alias " + alias + " has been already created");
407+
}
408+
certificateVO = new DirectDownloadCertificateVO(alias, certificatePem, hypervisorType, zoneId);
409+
directDownloadCertificateDao.persist(certificateVO);
410+
} else {
411+
HostVO host = hostDao.findById(hostId);
412+
hosts = Collections.singletonList(host);
413+
certificateVO = directDownloadCertificateDao.findByAlias(alias, hypervisorType, zoneId);
414+
if (certificateVO == null) {
415+
s_logger.info("Certificate must be uploaded on zone " + zoneId);
416+
return false;
417+
}
397418
}
398-
certificateVO = new DirectDownloadCertificateVO(alias, certificatePem, hypervisorType, zoneId);
399-
directDownloadCertificateDao.persist(certificateVO);
400419

401420
s_logger.info("Attempting to upload certificate: " + alias + " to " + hosts.size() + " hosts on zone " + zoneId);
402421
int hostCount = 0;
@@ -418,12 +437,6 @@ public boolean uploadCertificateToHosts(String certificateCer, String alias, Str
418437
* Upload and import certificate to hostId on keystore
419438
*/
420439
public boolean uploadCertificate(long certificateId, long hostId) {
421-
DirectDownloadCertificateHostMapVO map = directDownloadCertificateHostMapDao.findByCertificateAndHost(certificateId, hostId);
422-
if (map != null) {
423-
s_logger.debug("Certificate " + certificateId + " is already uploaded on host " + hostId);
424-
return true;
425-
}
426-
427440
DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findById(certificateId);
428441
if (certificateVO == null) {
429442
throw new CloudRuntimeException("Could not find certificate with id " + certificateId + " to upload to host: " + hostId);
@@ -432,7 +445,7 @@ public boolean uploadCertificate(long certificateId, long hostId) {
432445
String certificate = certificateVO.getCertificate();
433446
String alias = certificateVO.getAlias();
434447

435-
s_logger.debug("Uploading certificate: " + certificateVO.getAlias() + " to host " + hostId);
448+
s_logger.debug("Uploading certificate: " + certificateVO.getAlias() + " to host " + hostId);
436449
SetupDirectDownloadCertificateCommand cmd = new SetupDirectDownloadCertificateCommand(certificate, alias);
437450
Answer answer = agentManager.easySend(hostId, cmd);
438451
if (answer == null || !answer.getResult()) {
@@ -444,31 +457,51 @@ public boolean uploadCertificate(long certificateId, long hostId) {
444457
return false;
445458
}
446459

447-
DirectDownloadCertificateHostMapVO mapVO = new DirectDownloadCertificateHostMapVO(certificateId, hostId);
448-
directDownloadCertificateHostMapDao.persist(mapVO);
449460
s_logger.info("Certificate " + alias + " successfully uploaded to host: " + hostId);
461+
DirectDownloadCertificateHostMapVO map = directDownloadCertificateHostMapDao.findByCertificateAndHost(certificateId, hostId);
462+
if (map != null) {
463+
map.setRevoked(false);
464+
directDownloadCertificateHostMapDao.update(map.getId(), map);
465+
} else {
466+
DirectDownloadCertificateHostMapVO mapVO = new DirectDownloadCertificateHostMapVO(certificateId, hostId);
467+
directDownloadCertificateHostMapDao.persist(mapVO);
468+
}
469+
450470
return true;
451471
}
452472

453473
@Override
454-
public boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId) {
474+
public boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId, Long hostId) {
455475
HypervisorType hypervisorType = HypervisorType.getType(hypervisor);
456476
DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findByAlias(certificateAlias, hypervisorType, zoneId);
457477
if (certificateVO == null) {
458478
throw new CloudRuntimeException("Certificate alias " + certificateAlias + " does not exist");
459479
}
460480

461-
List<DirectDownloadCertificateHostMapVO> maps = directDownloadCertificateHostMapDao.listByCertificateId(certificateVO.getId());
481+
List<DirectDownloadCertificateHostMapVO> maps = null;
482+
if (hostId == null) {
483+
maps = directDownloadCertificateHostMapDao.listByCertificateId(certificateVO.getId());
484+
} else {
485+
DirectDownloadCertificateHostMapVO hostMap = directDownloadCertificateHostMapDao.findByCertificateAndHost(certificateVO.getId(), hostId);
486+
if (hostMap == null) {
487+
s_logger.info("Certificate " + certificateAlias + " cannot be revoked from host " + hostId + " as it is not available on the host");
488+
return false;
489+
}
490+
maps = Collections.singletonList(hostMap);
491+
}
492+
462493
s_logger.info("Attempting to revoke certificate alias: " + certificateAlias + " from " + maps.size() + " hosts");
463494
if (CollectionUtils.isNotEmpty(maps)) {
464495
for (DirectDownloadCertificateHostMapVO map : maps) {
465-
Long hostId = map.getHostId();
466-
if (!revokeCertificateAliasFromHost(certificateAlias, hostId)) {
467-
String msg = "Could not revoke certificate from host: " + hostId;
496+
Long mappingHostId = map.getHostId();
497+
if (!revokeCertificateAliasFromHost(certificateAlias, mappingHostId)) {
498+
String msg = "Could not revoke certificate from host: " + mappingHostId;
468499
s_logger.error(msg);
469500
throw new CloudRuntimeException(msg);
470501
}
471-
directDownloadCertificateHostMapDao.remove(map.getId());
502+
s_logger.info("Certificate " + certificateAlias + " revoked from host " + mappingHostId);
503+
map.setRevoked(true);
504+
directDownloadCertificateHostMapDao.update(map.getId(), map);
472505
}
473506
}
474507
return true;
@@ -485,6 +518,29 @@ protected boolean revokeCertificateAliasFromHost(String alias, Long hostId) {
485518
return false;
486519
}
487520

521+
@Override
522+
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
523+
executorService = new ScheduledThreadPoolExecutor(1, new NamedThreadFactory("DirectDownloadCertificateMonitor"));
524+
return true;
525+
}
526+
527+
@Override
528+
public boolean stop() {
529+
executorService.shutdownNow();
530+
return true;
531+
}
532+
533+
@Override
534+
public boolean start() {
535+
if (DirectDownloadCertificateUploadInterval.value() > 0L) {
536+
executorService.scheduleWithFixedDelay(
537+
new DirectDownloadCertificateUploadBackgroundTask(this, hostDao, dataCenterDao,
538+
directDownloadCertificateDao, directDownloadCertificateHostMapDao),
539+
60L, DirectDownloadCertificateUploadInterval.value(), TimeUnit.HOURS);
540+
}
541+
return true;
542+
}
543+
488544
@Override
489545
public String getConfigComponentName() {
490546
return DirectDownloadManager.class.getSimpleName();
@@ -497,7 +553,7 @@ public ConfigKey<?>[] getConfigKeys() {
497553
};
498554
}
499555

500-
public static final class DirectDownloadCertificateUploadBackgroundTask extends ManagedContextRunnable implements BackgroundPollTask {
556+
public static final class DirectDownloadCertificateUploadBackgroundTask extends ManagedContextRunnable {
501557

502558
private DirectDownloadManager directDownloadManager;
503559
private HostDao hostDao;
@@ -506,12 +562,12 @@ public static final class DirectDownloadCertificateUploadBackgroundTask extends
506562
private DataCenterDao dataCenterDao;
507563

508564
public DirectDownloadCertificateUploadBackgroundTask(
509-
final DirectDownloadManager caManager,
565+
final DirectDownloadManager manager,
510566
final HostDao hostDao,
511567
final DataCenterDao dataCenterDao,
512568
final DirectDownloadCertificateDao directDownloadCertificateDao,
513569
final DirectDownloadCertificateHostMapDao directDownloadCertificateHostMapDao) {
514-
this.directDownloadManager = caManager;
570+
this.directDownloadManager = manager;
515571
this.hostDao = hostDao;
516572
this.dataCenterDao = dataCenterDao;
517573
this.directDownloadCertificateDao = directDownloadCertificateDao;
@@ -552,11 +608,5 @@ protected void runInContext() {
552608
s_logger.error("Error trying to run Direct Download background task", t);
553609
}
554610
}
555-
556-
@Override
557-
public Long getDelay() {
558-
return DirectDownloadCertificateUploadInterval.value()
559-
* 60L * 60L * 1000L; //From hours to milliseconds
560-
}
561611
}
562612
}

0 commit comments

Comments
 (0)