Skip to content

Commit

Permalink
Added integration test + implemented logic inside #390
Browse files Browse the repository at this point in the history
- changed also cancel mechanism: ScanJobRunnableData
  has no two different setter method for threads. one
  for execution thread, one for cancel thread
  • Loading branch information
de-jcup committed Sep 24, 2024
1 parent d95ca38 commit dac8b7f
Show file tree
Hide file tree
Showing 63 changed files with 1,160 additions and 235 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ public class PDSAdapterDataConfigurator implements PDSAdapterConfigData, PDSAdap

private long resilienceTimeToWaitBeforeRetryInMilliseconds;

private UUID executorConfigurationUUID; // FIXME correct position?

public void setTargetType(String targetType) {
if (targetType == null) {
this.targetType = EMPTY_TARGET_TYPE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected AdapterExecutionResult execute(PDSAdapterConfig config, AdapterRuntime
AdapterExecutionResult result = null;

assertThreadNotInterrupted();

/* create and configure current PDS context object */
PDSContext pdsContext = contextFactory.create(config, this, runtimeContext);
handleResilienceConfiguration(pdsContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ protected String createAdapterId() {
@Override
public final AdapterExecutionResult start(C config, AdapterMetaDataCallback callback) throws AdapterException {
AdapterRuntimeContext runtimeContext = new AdapterRuntimeContext();
/* callback is from product executor and resolves adapter meta data for the product result (which is reused on soft restart of job*/
/*
* callback is from product executor and resolves adapter meta data for the
* product result (which is reused on soft restart of job
*/
runtimeContext.callback = callback;
runtimeContext.metaData = callback.getMetaDataOrNull();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@
import org.springframework.stereotype.Component;

import com.mercedesbenz.sechub.domain.administration.config.AdministrationConfigService;
import com.mercedesbenz.sechub.sharedkernel.Step;
import com.mercedesbenz.sechub.sharedkernel.messaging.AdministrationConfigMessage;
import com.mercedesbenz.sechub.sharedkernel.messaging.AsynchronMessageHandler;
import com.mercedesbenz.sechub.sharedkernel.messaging.DomainMessage;
import com.mercedesbenz.sechub.sharedkernel.messaging.IsReceivingAsyncMessage;
import com.mercedesbenz.sechub.sharedkernel.messaging.JobMessage;
import com.mercedesbenz.sechub.sharedkernel.messaging.MessageDataKeys;
import com.mercedesbenz.sechub.sharedkernel.messaging.MessageID;
import com.mercedesbenz.sechub.sharedkernel.usecases.other.UseCaseSystemHandlesSIGTERM;

@Component
public class JobAdministrationMessageHandler implements AsynchronMessageHandler {
Expand Down Expand Up @@ -44,6 +46,9 @@ public void receiveAsyncMessage(DomainMessage request) {
case JOB_FAILED:
handleJobFailed(request);
break;
case JOB_SUSPENDED:
handleJobSuspended(request);
break;
case JOB_CANCELLATION_RUNNING:
handleJobCancellationRunning(request);
break;
Expand Down Expand Up @@ -86,4 +91,13 @@ private void handleJobFailed(DomainMessage request) {
deleteService.delete(message.getJobUUID());
}

@IsReceivingAsyncMessage(MessageID.JOB_SUSPENDED)
@UseCaseSystemHandlesSIGTERM(@Step(number = 7, name = "Administration handles suspension", description = "Administration removes suspended listeners about job suspension"))
private void handleJobSuspended(DomainMessage request) {
JobMessage message = request.get(MessageDataKeys.JOB_SUSPENDED_DATA);
// we do drop job info - we only hold running and waiting jobs. The suspended
// job will be restarted and appear later again.
deleteService.delete(message.getJobUUID());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ private void updateSchedulerJobInformation(SchedulerMessage status) {
saveStatusEntry(SchedulerStatusEntryKeys.SCHEDULER_JOBS_STARTED, status.getAmountOfJobsStarted());
saveStatusEntry(SchedulerStatusEntryKeys.SCHEDULER_JOBS_CANCEL_REQUESTED, status.getAmountOfJobsCancelRequested());
saveStatusEntry(SchedulerStatusEntryKeys.SCHEDULER_JOBS_CANCELED, status.getAmountOfJobsCanceled());
saveStatusEntry(SchedulerStatusEntryKeys.SCHEDULER_JOBS_SUSPENDED, status.getAmountOfJobsSuspended());
saveStatusEntry(SchedulerStatusEntryKeys.SCHEDULER_JOBS_ENDED, status.getAmountOfJobsEnded());

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public enum SchedulerStatusEntryKeys implements StatusEntryKey {

SCHEDULER_JOBS_CANCEL_REQUESTED("status.scheduler.jobs.cancel_requested"),

SCHEDULER_JOBS_SUSPENDED("status.scheduler.jobs.suspended"),

SCHEDULER_JOBS_ENDED("status.scheduler.jobs.ended"),

;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ private int compareOnMessageIfTypeIsSame(SecHubMessage otherNotNull) {
if (text == null) {
return -1;
}
if (otherNotNull.text == null) {
return 1;
}
return text.compareTo(otherNotNull.text);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

/**
* Represents the execution state of a scheduled SecHub job.
*
*
* Attention: never change the enum values because they are used for persistence
* as identifiers!
*
*
* @author Albert Tregnaghi
*
*/
Expand All @@ -25,11 +25,7 @@ public enum ExecutionState {

CANCELED("The job has been canceled"),

/*
* TODO 2024-06-05 de-jcup, winzj: write an integration test with sechub client
* to check if go client can run with the new enum part
*/
PAUSED("The job has been paused and can be resumed by another SecHub instance where scheduler is running"),
SUSPENDED("The job has been suspended and can be resumed by another SecHub instance"),

ENDED("Has ended - with failure or success");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,38 @@

class SecHubMessageTest {

@Test
void compare_with_null_works() {
/* prepare */
SecHubMessage info1 = new SecHubMessage(SecHubMessageType.INFO, "info1");

/* execute */
int compareResult = info1.compareTo(null);

/* test */
assertEquals(1, compareResult);

}

@Test
void compare_with_message_without_text_works() {
/* prepare */
SecHubMessage info1 = new SecHubMessage(SecHubMessageType.INFO, "info1");
SecHubMessage info2 = new SecHubMessage(SecHubMessageType.INFO, null);

/* execute 1 */
int compareResult = info1.compareTo(info2);

/* test 1 */
assertEquals(1, compareResult);

/* execute 2 */
compareResult = info2.compareTo(info1);

/* test 1 */
assertEquals(-1, compareResult);
}

/* Why we need ordering? so results are always same listed when recreated */
@Test
void types_ordering_is_error_warn_than_info_in_a_treeset() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ package com.mercedesbenz.sechub.domain.schedule {
handleJobRestartRequested()
}

class SynchronSecHubJobExecutor {
}

class SchedulerTerminationService {
void terminate()
boolean isTerminating()
Expand All @@ -45,7 +48,7 @@ package com.mercedesbenz.sechub.domain.schedule {
}

SchedulerJobBatchTriggerService ..> SchedulerTerminationService
SchedulerTerminationService -> ScheduleSecHubJob : persists with execution state\n`PAUSED`
SchedulerTerminationService -> ScheduleSecHubJob : persists with execution state\n`SUSPENDED`

node EventBus {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
[[section-shared-concepts-sechub-deployment-without-scheduler-stop]]
*Why this concept is necessary*

Before this concept, for new deployments of {sechub} server instances administrators had to
stop the scheduler and to wait for running jobs. After no job were running any longer,
Before this concept, for new deployments of {sechub} server instances, administrators had to
stop the scheduler and wait for running jobs to finish. After no jobs were running any longer,
the deployment was triggered and after this the scheduler was enabled again and job processing
started again.

Expand All @@ -17,18 +17,22 @@ a bad user experience.

===== SIGTERM handling

plantuml::diagrams/diagram_sechub_sigterm_handling.puml[format=svg, title="Sigterm handling"]
plantuml::diagrams/diagram_sechub_sigterm_handling.puml[format=svg, title="SIGTERM handling"]

In {pds} we already have a SIGTERM handling where the instance which is running a {pds} job will
mark this job as PAUSED.

We use a similar approach in {sechub}:
K8s and other systems will send a `SIGTERM` signal to give application the possibility to shutdown
gracefully.

On a `SIGTERM` signal {sechub} temporarily suspends a job, allowing its {pds} instances to continue
processing it in the background. The next new SecHub server then reactivates the job and proceeds
with the results from the {pds} instances (or wait for them if still not already available).

All running {sechub} jobs on terminating instance are marked with execution state `SUSPENDED` +
(similar to cancel) which will also update `ENDED` timestamp on job.

Other servers instances will restart `SUSPENDED` jobs by existing
<<section-shared-concepts-sechub-job-restart-handling,restart mechanism>> (but `SUSPENDED` will
be handled before `READY_TO_START`).

. Stop new job processing on {sechub} instance where SIGTERM signal occurred
. Introduce a new {sechub} job execution state: `PAUSED`
. Mark all running {sechub} jobs on terminating instance as `PAUSED` (similar to cancel) which will also update `ENDED` timestamp on job
. Other servers instances will restart `PAUSED` jobs by existing restart mechanism (but `PAUSED` will be handled before `READY_TO_START`).
To prevent too fast restarts, the `ENDED` timestamp of {sechub} job will be inspected for paused jobs and only fetched as
next job when the time gap is greater than a defined (configurable) time period.
To prevent too fast restarts, the `ENDED` timestamp of {sechub} job will be inspected on suspended jobs
and only fetched as next job when the time gap is greater than a defined (configurable) time period.


Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: MIT
package com.mercedesbenz.sechub.integrationtest.api;

import static com.mercedesbenz.sechub.integrationtest.api.IntegrationTestMockMode.*;
import static com.mercedesbenz.sechub.integrationtest.api.TestAPI.*;
import static com.mercedesbenz.sechub.integrationtest.internal.IntegrationTestExampleConstants.*;
import static org.junit.Assert.*;

import java.io.File;
Expand Down Expand Up @@ -56,6 +58,7 @@
import com.mercedesbenz.sechub.test.executorconfig.TestExecutorSetupJobParam;

public class AsUser {
private static boolean warnedOnceAboutMissingConfigurationUUIDs;

private static final Logger LOG = LoggerFactory.getLogger(AsUser.class);
private JSONTestSupport jsonTestSupport = JSONTestSupport.DEFAULT;
Expand Down Expand Up @@ -962,6 +965,19 @@ public ProjectFalsePositivesDefinition startFalsePositiveDefinition(TestProject
return new ProjectFalsePositivesDefinition(project);
}

public UUID triggerAsyncPDSCodeScanWithDifferentDataSections(TestProject project) {
/* @formatter:off */
UUID jobUUID = createCodeScanWithTemplate(
IntegrationTestTemplateFile.CODE_SCAN_3_SOURCES_DATA_ONE_REFERENCE,
project, NOT_MOCKED,
TemplateData.builder().addReferenceId("files-a").build());

uploadSourcecode(project, jobUUID, PATH_TO_ZIPFILE_WITH_DIFFERENT_DATA_SECTIONS).
approveJob(project, jobUUID);
/* @formatter:on */
return jobUUID;
}

public UUID triggerAsyncCodeScanGreenSuperFastWithPseudoZipUpload(TestProject project) {
return triggerAsyncCodeScanApproveWithSourceUploadAndGetJobUUID(project, IntegrationTestMockMode.CODE_SCAN__CHECKMARX__GREEN__10_MS_WATING,
TestDataConstants.RESOURCE_PATH_ZIPFILE_ONLY_TEST1_TXT);
Expand Down Expand Up @@ -1034,8 +1050,13 @@ void ensureExecutorConfigUUIDs(TestExecutorConfig executorConfig) {
if (executorConfig.uuid != null) {
return;
}
LOG.warn("The executor config:" + executorConfig.name
+ " had no UUID - this can happen when starting an integration test again when executor config already exists and not recreated. So load list of executor configurations and try to resolve the config. But be aware! Ensure names of configurations are unique here so it's really the config you wanted");
if (!warnedOnceAboutMissingConfigurationUUIDs) {
LOG.warn(
"The executor config: {} had no UUID - this can happen when starting an integration test again when executor config already exists and not recreated. So load list of executor configurations and try to resolve the config. But be aware! Ensure names of configurations are unique here so it's really the config you wanted."
+ "This warning will appear only one time",
executorConfig.name);
warnedOnceAboutMissingConfigurationUUIDs = true;
}
TestExecutorConfigList list = fetchProductExecutorConfigList();
for (TestExecutorConfigListEntry entry : list.executorConfigurations) {
if (executorConfig.name.equals(entry.name)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ public enum TestExecutionState {

CANCEL_REQUESTED,

SUSPENDED,

ENDED;

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ public AssertUserJobInfoForJob withExecutionResult(String result) {
return this;
}

public AssertUserJobInfoForJob withEndedTimeStampNotNull() {
assertNotNull("Execution result has no ended timestamp!", info.ended);
return this;
}

public AssertUserJobInfo and() {
return AssertUserJobInfo.this;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.mercedesbenz.sechub.integrationtest.api;

/**
* Special exception which shall only be thrown when the tests are in a state
* where it is no longer possible to execute them correctly.
*
* For example: Via TestAPI it is possible to wait until no longer jobs are
* running. If this is not possible/time out is done, this is a critical
* behavior because the checks are very time consuming and after the timeout
* there are still jobs running. But the wait mechanism is used for every
* integration test and would fail of them but after an extreme long time
* period.
*
* @author Albert Tregnaghi
*
*/
public class CriticalTestProblemException extends IllegalStateException {

public CriticalTestProblemException(String s) {
super(s);
}

public CriticalTestProblemException(String message, Throwable cause) {
super(message, cause);
}

private static final long serialVersionUID = 1L;

}
Loading

0 comments on commit dac8b7f

Please sign in to comment.